Allow clients to provide ID for tracing

This makes it easier for users to communicate the trace ID to the
Gerrit support team. E.g. users can simply use a bug ID as trace ID and
then there is no need to find the generated trace ID and post it in the
bug. This is nice because finding the generated trace ID can be tricky,
e.g. for REST calls it is returned as a response header that is only
visible with curl -v or in the browser network tooling.

This slightly changes the way how tracing is enabled:

1. REST API:
   * old syntax:
     a) ?trace -> enabled trace
     b) ?trace=true -> enabled trace
     c) ?trace=false -> disabled trace
   * new syntax:
     a) ?trace -> enabled trace with generated trace ID
     b) ?trace=foo -> enabled trace with trace ID ‘foo’

2. Git API:
   * old syntax:
     a) -o trace -> enabled trace
     b) -o trace=true -> enabled trace
     c) -o trace=false -> disabled trace
   * new syntax:
     a) -o trace -> enabled trace with generated trace ID
     b) -o trace=foo -> enabled trace with trace ID ‘foo’

3. SSH API:
   * old syntax:
     a) --trace -> enabled trace
   * new syntax:
     a) --trace -> enabled trace with generated trace ID
     b) --trace --traceId=foo -> enabled trace with trace ID ‘foo’
     c) --traceId=foo -> error, cannot use --traceId without --trace

Change-Id: I4f2c3b5748678bbdf8820463d8374b854200743a
Signed-off-by: Edwin Kempin <ekempin@google.com>

Documentation/cmd-index.txt

@@ -212,7 +212,15 @@ link:cmd-suexec.html[suexec]::
 === Trace
 
 For executing SSH commands tracing can be enabled by setting the
-`--trace` option.
+`--trace` and `--trace-id <trace-id>` options. It is recommended to use
+the ID of the issue that is being investigated as trace ID.
+
+----
+  $ ssh -p 29418 review.example.com gerrit create-project --trace --trace-id issue/123 foo/bar
+----
+
+It is also possible to omit the trace ID and get a unique trace ID
+generated.
 
 ----
   $ ssh -p 29418 review.example.com gerrit create-project --trace foo/bar
@@ -220,8 +228,8 @@ For executing SSH commands tracing can be enabled by setting the
 
 Enabling tracing results in additional logs with debug information that
 are written to the `error_log`. All logs that correspond to the traced
-request are associated with a unique trace ID. This trace ID is printed
+request are associated with the trace ID. The trace ID is printed to
-to the stderr command output:
+the stderr command output:
 
 ----
   TRACE_ID: 1534174322774-7edf2a7b

Documentation/rest-api.txt

@@ -193,8 +193,17 @@ specified in the request body cannot be resolved.
 
 [[tracing]]
 === Request Tracing
-For each REST endpoint tracing can be enabled by setting the `trace`
+For each REST endpoint tracing can be enabled by setting the
-request parameter.
+`trace=<trace-id>` request parameter. It is recommended to use the ID
+of the issue that is being investigated as trace ID.
+
+.Example Request
+----
+  GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/suggest_reviewers?trace=issue/123&q=J
+----
+
+It is also possible to omit the trace ID and get a unique trace ID
+generated.
 
 .Example Request
 ----
@@ -203,8 +212,8 @@ request parameter.
 
 Enabling tracing results in additional logs with debug information that
 are written to the `error_log`. All logs that correspond to the traced
-request are associated with a unique trace ID. This trace ID is
+request are associated with the trace ID. The trace ID is returned with
-returned with the REST response in the `X-Gerrit-Trace` header.
+the REST response in the `X-Gerrit-Trace` header.
 
 .Example Response
 ----

Documentation/user-request-tracing.txt

@@ -26,6 +26,16 @@ request type:
   link:user-upload.html[upload documentation]. Tracing for Git requests
   other than Git push is not supported.
 
+When request tracing is enabled it is possible to provide an ID that
+should be used as trace ID. If a trace ID is not provided a trace ID is
+automatically generated. The trace ID must be provided to the support
+team so that they can find the trace.
+
+When doing traces it is recommended to specify the ID of the issue
+that is being investigated as trace ID so that the traces of the issue
+can be found more easily. When the issue ID is used as trace ID there
+is no need to find the generated trace ID and report it in the issue.
+
 Since tracing consumes additional server resources tracing should only
 be enabled for single requests if there is a concrete need for
 debugging. In particular bots should never enable tracing for all their

Documentation/user-upload.txt

@@ -424,17 +424,26 @@ branches, consider adding a custom remote block to your project's
 [[trace]]
 ==== Trace
 
-When pushing to Gerrit tracing can be enabled by setting the `trace`
+When pushing to Gerrit tracing can be enabled by setting the
-push option.
+`trace=<trace-id>` push option. It is recommended to use the ID of the
+issue that is being investigated as trace ID.
 
+----
+  git push -o trace=issue/123 ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master
+----
+
+It is also possible to omit the trace ID and get a unique trace ID
+generated.
+
+.Example Request
 ----
   git push -o trace ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master
 ----
 
 Enabling tracing results in additional logs with debug information that
 are written to the `error_log`. All logs that correspond to the traced
-request are associated with a unique trace ID. This trace ID is
+request are associated with the trace ID. This trace ID is returned in
-returned in the command output:
+the command output:
 
 ----
   remote: TRACE_ID: 1534174322774-7edf2a7b

java/com/google/gerrit/httpd/restapi/RestApiServlet.java

@@ -1319,9 +1319,10 @@ public class RestApiServlet extends HttpServlet {
   }
 
   private TraceContext enableTracing(HttpServletRequest req, HttpServletResponse res) {
-    String v = req.getParameter(ParameterParser.TRACE_PARAMETER);
+    String traceValue = req.getParameter(ParameterParser.TRACE_PARAMETER);
     return TraceContext.newTrace(
-        v != null && (v.isEmpty() || Boolean.parseBoolean(v)),
+        traceValue != null,
+        traceValue,
         (tagName, traceId) -> res.setHeader(X_GERRIT_TRACE, traceId.toString()));
   }
 

java/com/google/gerrit/server/git/receive/ReceiveCommits.java

@@ -397,7 +397,7 @@ class ReceiveCommits {
   private String setFullNameTo;
   private boolean setChangeAsPrivate;
   private Optional<NoteDbPushOption> noteDbPushOption;
-  private Optional<Boolean> tracePushOption;
+  private Optional<String> tracePushOption;
 
   private MessageSender messageSender;
 
@@ -578,7 +578,8 @@ class ReceiveCommits {
     parsePushOptions();
     try (TraceContext traceContext =
         TraceContext.newTrace(
-            tracePushOption.orElse(false),
+            tracePushOption.isPresent(),
+            tracePushOption.orElse(null),
             (tagName, traceId) -> addMessage(tagName + ": " + traceId))) {
       traceContext.addTag(RequestId.Type.RECEIVE_ID, RequestId.forProject(project.getNameKey()));
       try {
@@ -951,7 +952,7 @@ class ReceiveCommits {
     List<String> traceValues = pushOptions.get("trace");
     if (!traceValues.isEmpty()) {
       String value = traceValues.get(traceValues.size() - 1);
-      tracePushOption = Optional.of(value.isEmpty() || Boolean.parseBoolean(value));
+      tracePushOption = Optional.of(value);
     } else {
       tracePushOption = Optional.empty();
     }
@@ -1371,7 +1372,7 @@ class ReceiveCommits {
     Set<String> hashtags = new HashSet<>();
 
     @Option(name = "--trace", metaVar = "NAME", usage = "enable tracing")
-    boolean trace;
+    String trace;
 
     @Option(name = "--base", metaVar = "BASE", usage = "merge base of changes")
     List<ObjectId> base;

java/com/google/gerrit/server/logging/TraceContext.java

@@ -16,8 +16,10 @@ package com.google.gerrit.server.logging;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.HashBasedTable;
 import com.google.common.collect.Table;
+import com.google.gerrit.common.Nullable;
 import java.util.Optional;
 
 public class TraceContext implements AutoCloseable {
@@ -29,36 +31,44 @@ public class TraceContext implements AutoCloseable {
    * Opens a new trace context for request tracing.
    *
    * <ul>
-   *   <li>sets a tag with a generated trace ID
+   *   <li>sets a tag with a trace ID
    *   <li>enables force logging
    * </ul>
    *
-   * <p>A new trace ID is only generated if request tracing was not started yet. If request tracing
+   * <p>if no trace ID is provided a new trace ID is only generated if request tracing was not
-   * was already started the given {@code traceIdConsumer} is invoked with the existing trace ID and
+   * started yet. If request tracing was already started the given {@code traceIdConsumer} is
-   * no new logging tag is set.
+   * invoked with the existing trace ID and no new logging tag is set.
    *
    * <p>No-op if {@code trace} is {@code false}.
    *
    * @param trace whether tracing should be started
+   * @param traceId trace ID that should be used for tracing, if {@code null} a trace ID is
+   *     generated
    * @param traceIdConsumer consumer for the trace ID, should be used to return the generated trace
    *     ID to the client, not invoked if {@code trace} is {@code false}
    * @return the trace context
    */
-  public static TraceContext newTrace(boolean trace, TraceIdConsumer traceIdConsumer) {
+  public static TraceContext newTrace(
+      boolean trace, @Nullable String traceId, TraceIdConsumer traceIdConsumer) {
     if (!trace) {
       // Create an empty trace context.
       return open();
     }
 
-    Optional<String> traceId =
+    if (!Strings.isNullOrEmpty(traceId)) {
+      traceIdConsumer.accept(RequestId.Type.TRACE_ID.name(), traceId);
+      return open().addTag(RequestId.Type.TRACE_ID, traceId).forceLogging();
+    }
+
+    Optional<String> existingTraceId =
         LoggingContext.getInstance()
             .getTagsAsMap()
             .get(RequestId.Type.TRACE_ID.name())
             .stream()
             .findAny();
-    if (traceId.isPresent()) {
+    if (existingTraceId.isPresent()) {
       // request tracing was already started, no need to generate a new trace ID
-      traceIdConsumer.accept(RequestId.Type.TRACE_ID.name(), traceId.get());
+      traceIdConsumer.accept(RequestId.Type.TRACE_ID.name(), existingTraceId.get());
       return open();
     }
 

java/com/google/gerrit/sshd/SshCommand.java

@@ -24,6 +24,9 @@ public abstract class SshCommand extends BaseCommand {
   @Option(name = "--trace", usage = "enable request tracing")
   private boolean trace;
 
+  @Option(name = "--trace-id", usage = "trace ID (can only be set if --trace was set too)")
+  private String traceId;
+
   protected PrintWriter stdout;
   protected PrintWriter stderr;
 
@@ -48,8 +51,13 @@ public abstract class SshCommand extends BaseCommand {
 
   protected abstract void run() throws UnloggedFailure, Failure, Exception;
 
-  private TraceContext enableTracing() {
+  private TraceContext enableTracing() throws UnloggedFailure {
+    if (!trace && traceId != null) {
+      throw die("A trace ID can only be set if --trace was specified.");
+    }
     return TraceContext.newTrace(
-        trace, (tagName, traceId) -> stderr.println(String.format("%s: %s", tagName, traceId)));
+        trace,
+        traceId,
+        (tagName, traceId) -> stderr.println(String.format("%s: %s", tagName, traceId)));
   }
 }

javatests/com/google/gerrit/acceptance/rest/TraceIT.java

@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static org.apache.http.HttpStatus.SC_CREATED;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.RestResponse;
@@ -68,6 +69,7 @@ public class TraceIT extends AbstractDaemonTest {
     RestResponse response = adminRestSession.put("/projects/new1");
     assertThat(response.getStatusCode()).isEqualTo(SC_CREATED);
     assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
+    assertThat(projectCreationListener.traceId).isNull();
     assertThat(projectCreationListener.foundTraceId).isFalse();
     assertThat(projectCreationListener.isLoggingForced).isFalse();
   }
@@ -78,35 +80,28 @@ public class TraceIT extends AbstractDaemonTest {
         adminRestSession.put("/projects/new2?" + ParameterParser.TRACE_PARAMETER);
     assertThat(response.getStatusCode()).isEqualTo(SC_CREATED);
     assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNotNull();
+    assertThat(projectCreationListener.traceId).isNotNull();
     assertThat(projectCreationListener.foundTraceId).isTrue();
     assertThat(projectCreationListener.isLoggingForced).isTrue();
   }
 
   @Test
-  public void restCallWithTraceTrue() throws Exception {
+  public void restCallWithTraceAndProvidedTraceId() throws Exception {
     RestResponse response =
-        adminRestSession.put("/projects/new3?" + ParameterParser.TRACE_PARAMETER + "=true");
+        adminRestSession.put("/projects/new3?" + ParameterParser.TRACE_PARAMETER + "=issue/123");
     assertThat(response.getStatusCode()).isEqualTo(SC_CREATED);
     assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNotNull();
+    assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
     assertThat(projectCreationListener.foundTraceId).isTrue();
     assertThat(projectCreationListener.isLoggingForced).isTrue();
   }
 
-  @Test
-  public void restCallWithTraceFalse() throws Exception {
-    RestResponse response =
-        adminRestSession.put("/projects/new4?" + ParameterParser.TRACE_PARAMETER + "=false");
-    assertThat(response.getStatusCode()).isEqualTo(SC_CREATED);
-    assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
-    assertThat(projectCreationListener.foundTraceId).isFalse();
-    assertThat(projectCreationListener.isLoggingForced).isFalse();
-  }
-
   @Test
   public void pushWithoutTrace() throws Exception {
     PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
     PushOneCommit.Result r = push.to("refs/heads/master");
     r.assertOkStatus();
+    assertThat(commitValidationListener.traceId).isNull();
     assertThat(commitValidationListener.foundTraceId).isFalse();
     assertThat(commitValidationListener.isLoggingForced).isFalse();
   }
@@ -117,35 +112,28 @@ public class TraceIT extends AbstractDaemonTest {
     push.setPushOptions(ImmutableList.of("trace"));
     PushOneCommit.Result r = push.to("refs/heads/master");
     r.assertOkStatus();
+    assertThat(commitValidationListener.traceId).isNotNull();
     assertThat(commitValidationListener.foundTraceId).isTrue();
     assertThat(commitValidationListener.isLoggingForced).isTrue();
   }
 
   @Test
-  public void pushWithTraceTrue() throws Exception {
+  public void pushWithTraceAndProvidedTraceId() throws Exception {
     PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
-    push.setPushOptions(ImmutableList.of("trace=true"));
+    push.setPushOptions(ImmutableList.of("trace=issue/123"));
     PushOneCommit.Result r = push.to("refs/heads/master");
     r.assertOkStatus();
+    assertThat(commitValidationListener.traceId).isEqualTo("issue/123");
     assertThat(commitValidationListener.foundTraceId).isTrue();
     assertThat(commitValidationListener.isLoggingForced).isTrue();
   }
 
-  @Test
-  public void pushWithTraceFalse() throws Exception {
-    PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
-    push.setPushOptions(ImmutableList.of("trace=false"));
-    PushOneCommit.Result r = push.to("refs/heads/master");
-    r.assertOkStatus();
-    assertThat(commitValidationListener.foundTraceId).isFalse();
-    assertThat(commitValidationListener.isLoggingForced).isFalse();
-  }
-
   @Test
   public void pushForReviewWithoutTrace() throws Exception {
     PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
     PushOneCommit.Result r = push.to("refs/for/master");
     r.assertOkStatus();
+    assertThat(commitValidationListener.traceId).isNull();
     assertThat(commitValidationListener.foundTraceId).isFalse();
     assertThat(commitValidationListener.isLoggingForced).isFalse();
   }
@@ -156,50 +144,48 @@ public class TraceIT extends AbstractDaemonTest {
     push.setPushOptions(ImmutableList.of("trace"));
     PushOneCommit.Result r = push.to("refs/for/master");
     r.assertOkStatus();
+    assertThat(commitValidationListener.traceId).isNotNull();
     assertThat(commitValidationListener.foundTraceId).isTrue();
     assertThat(commitValidationListener.isLoggingForced).isTrue();
   }
 
   @Test
-  public void pushForReviewWithTraceTrue() throws Exception {
+  public void pushForReviewWithTraceAndProvidedTraceId() throws Exception {
     PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
-    push.setPushOptions(ImmutableList.of("trace=true"));
+    push.setPushOptions(ImmutableList.of("trace=issue/123"));
     PushOneCommit.Result r = push.to("refs/for/master");
     r.assertOkStatus();
+    assertThat(commitValidationListener.traceId).isEqualTo("issue/123");
     assertThat(commitValidationListener.foundTraceId).isTrue();
     assertThat(commitValidationListener.isLoggingForced).isTrue();
   }
 
-  @Test
-  public void pushForReviewWithTraceFalse() throws Exception {
-    PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
-    push.setPushOptions(ImmutableList.of("trace=false"));
-    PushOneCommit.Result r = push.to("refs/for/master");
-    r.assertOkStatus();
-    assertThat(commitValidationListener.foundTraceId).isFalse();
-    assertThat(commitValidationListener.isLoggingForced).isFalse();
-  }
-
   private static class TraceValidatingProjectCreationValidationListener
       implements ProjectCreationValidationListener {
+    String traceId;
     Boolean foundTraceId;
     Boolean isLoggingForced;
 
     @Override
     public void validateNewProject(CreateProjectArgs args) throws ValidationException {
-      this.foundTraceId = LoggingContext.getInstance().getTagsAsMap().containsKey("TRACE_ID");
+      this.traceId =
+          Iterables.getFirst(LoggingContext.getInstance().getTagsAsMap().get("TRACE_ID"), null);
+      this.foundTraceId = traceId != null;
       this.isLoggingForced = LoggingContext.getInstance().shouldForceLogging(null, null, false);
     }
   }
 
   private static class TraceValidatingCommitValidationListener implements CommitValidationListener {
+    String traceId;
     Boolean foundTraceId;
     Boolean isLoggingForced;
 
     @Override
     public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
         throws CommitValidationException {
-      this.foundTraceId = LoggingContext.getInstance().getTagsAsMap().containsKey("TRACE_ID");
+      this.traceId =
+          Iterables.getFirst(LoggingContext.getInstance().getTagsAsMap().get("TRACE_ID"), null);
+      this.foundTraceId = traceId != null;
       this.isLoggingForced = LoggingContext.getInstance().shouldForceLogging(null, null, false);
       return ImmutableList.of();
     }

javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java

@@ -2,6 +2,7 @@ package com.google.gerrit.acceptance.ssh;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.UseSsh;
 import com.google.gerrit.extensions.registration.DynamicSet;
@@ -39,6 +40,7 @@ public class SshTraceIT extends AbstractDaemonTest {
   public void sshCallWithoutTrace() throws Exception {
     adminSshSession.exec("gerrit create-project new1");
     adminSshSession.assertSuccess();
+    assertThat(projectCreationListener.traceId).isNull();
     assertThat(projectCreationListener.foundTraceId).isFalse();
     assertThat(projectCreationListener.isLoggingForced).isFalse();
   }
@@ -50,18 +52,40 @@ public class SshTraceIT extends AbstractDaemonTest {
     // The trace ID is written to stderr.
     adminSshSession.assertFailure(RequestId.Type.TRACE_ID.name());
 
+    assertThat(projectCreationListener.traceId).isNotNull();
     assertThat(projectCreationListener.foundTraceId).isTrue();
     assertThat(projectCreationListener.isLoggingForced).isTrue();
   }
 
+  @Test
+  public void sshCallWithTraceAndProvidedTraceId() throws Exception {
+    adminSshSession.exec("gerrit create-project --trace --trace-id issue/123 new3");
+
+    // The trace ID is written to stderr.
+    adminSshSession.assertFailure(RequestId.Type.TRACE_ID.name());
+
+    assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
+    assertThat(projectCreationListener.foundTraceId).isTrue();
+    assertThat(projectCreationListener.isLoggingForced).isTrue();
+  }
+
+  @Test
+  public void sshCallWithTraceIdAndWithoutTraceFails() throws Exception {
+    adminSshSession.exec("gerrit create-project --trace-id issue/123 new3");
+    adminSshSession.assertFailure("A trace ID can only be set if --trace was specified.");
+  }
+
   private static class TraceValidatingProjectCreationValidationListener
       implements ProjectCreationValidationListener {
+    String traceId;
     Boolean foundTraceId;
     Boolean isLoggingForced;
 
     @Override
     public void validateNewProject(CreateProjectArgs args) throws ValidationException {
-      this.foundTraceId = LoggingContext.getInstance().getTagsAsMap().containsKey("TRACE_ID");
+      this.traceId =
+          Iterables.getFirst(LoggingContext.getInstance().getTagsAsMap().get("TRACE_ID"), null);
+      this.foundTraceId = traceId != null;
       this.isLoggingForced = LoggingContext.getInstance().shouldForceLogging(null, null, false);
     }
   }

javatests/com/google/gerrit/server/logging/TraceContextTest.java

@@ -158,7 +158,7 @@ public class TraceContextTest {
   @Test
   public void newTrace() {
     TestTraceIdConsumer traceIdConsumer = new TestTraceIdConsumer();
-    try (TraceContext traceContext = TraceContext.newTrace(true, traceIdConsumer)) {
+    try (TraceContext traceContext = TraceContext.newTrace(true, null, traceIdConsumer)) {
       assertForceLogging(true);
       assertThat(LoggingContext.getInstance().getTagsAsMap().keySet())
           .containsExactly(RequestId.Type.TRACE_ID.name());
@@ -167,10 +167,33 @@ public class TraceContextTest {
     assertThat(traceIdConsumer.traceId).isNotNull();
   }
 
+  @Test
+  public void newTraceWithProvidedTraceId() {
+    TestTraceIdConsumer traceIdConsumer = new TestTraceIdConsumer();
+    String traceId = "foo";
+    try (TraceContext traceContext = TraceContext.newTrace(true, traceId, traceIdConsumer)) {
+      assertForceLogging(true);
+      assertTags(ImmutableMap.of(RequestId.Type.TRACE_ID.name(), ImmutableSet.of(traceId)));
+    }
+    assertThat(traceIdConsumer.tagName).isEqualTo(RequestId.Type.TRACE_ID.name());
+    assertThat(traceIdConsumer.traceId).isEqualTo(traceId);
+  }
+
   @Test
   public void newTraceDisabled() {
     TestTraceIdConsumer traceIdConsumer = new TestTraceIdConsumer();
-    try (TraceContext traceContext = TraceContext.newTrace(false, traceIdConsumer)) {
+    try (TraceContext traceContext = TraceContext.newTrace(false, null, traceIdConsumer)) {
+      assertForceLogging(false);
+      assertTags(ImmutableMap.of());
+    }
+    assertThat(traceIdConsumer.tagName).isNull();
+    assertThat(traceIdConsumer.traceId).isNull();
+  }
+
+  @Test
+  public void newTraceDisabledWithProvidedTraceId() {
+    TestTraceIdConsumer traceIdConsumer = new TestTraceIdConsumer();
+    try (TraceContext traceContext = TraceContext.newTrace(false, "foo", traceIdConsumer)) {
       assertForceLogging(false);
       assertTags(ImmutableMap.of());
     }
@@ -181,12 +204,12 @@ public class TraceContextTest {
   @Test
   public void onlyOneTraceId() {
     TestTraceIdConsumer traceIdConsumer1 = new TestTraceIdConsumer();
-    try (TraceContext traceContext1 = TraceContext.newTrace(true, traceIdConsumer1)) {
+    try (TraceContext traceContext1 = TraceContext.newTrace(true, null, traceIdConsumer1)) {
       String expectedTraceId = traceIdConsumer1.traceId;
       assertThat(expectedTraceId).isNotNull();
 
       TestTraceIdConsumer traceIdConsumer2 = new TestTraceIdConsumer();
-      try (TraceContext traceContext2 = TraceContext.newTrace(true, traceIdConsumer2)) {
+      try (TraceContext traceContext2 = TraceContext.newTrace(true, null, traceIdConsumer2)) {
         assertForceLogging(true);
         assertTags(
             ImmutableMap.of(RequestId.Type.TRACE_ID.name(), ImmutableSet.of(expectedTraceId)));
@@ -196,6 +219,23 @@ public class TraceContextTest {
     }
   }
 
+  @Test
+  public void multipleTraceIdsIfTraceIdProvided() {
+    String traceId1 = "foo";
+    try (TraceContext traceContext1 =
+        TraceContext.newTrace(true, traceId1, (tagName, traceId) -> {})) {
+      TestTraceIdConsumer traceIdConsumer = new TestTraceIdConsumer();
+      String traceId2 = "bar";
+      try (TraceContext traceContext2 = TraceContext.newTrace(true, traceId2, traceIdConsumer)) {
+        assertForceLogging(true);
+        assertTags(
+            ImmutableMap.of(RequestId.Type.TRACE_ID.name(), ImmutableSet.of(traceId1, traceId2)));
+      }
+      assertThat(traceIdConsumer.tagName).isEqualTo(RequestId.Type.TRACE_ID.name());
+      assertThat(traceIdConsumer.traceId).isEqualTo(traceId2);
+    }
+  }
+
   private void assertTags(ImmutableMap<String, ImmutableSet<String>> expectedTagMap) {
     SortedMap<String, SortedSet<Object>> actualTagMap =
         LoggingContext.getInstance().getTags().asMap();