Don't start a new trace if request tracing was already started
If request tracing was started twice we would generate 2 trace IDs and
set the TRACE_ID logging tag for each of the trace IDs. This is
unneeded. Instead just use the existing trace ID if request tracing is
started multiple times.
This is useful if a custom servlet filter is used to start requesting.
Change-Id: Id5536fb9cef30b96e0e74df2d8d3127f8afdd356
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/server/logging/TraceContext.java b/java/com/google/gerrit/server/logging/TraceContext.java
index 7d73303..5b8d8e1 100644
--- a/java/com/google/gerrit/server/logging/TraceContext.java
+++ b/java/com/google/gerrit/server/logging/TraceContext.java
@@ -18,6 +18,7 @@
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.Table;
+import java.util.Optional;
public class TraceContext implements AutoCloseable {
public static TraceContext open() {
@@ -32,6 +33,10 @@
* <li>enables force logging
* </ul>
*
+ * <p>A new trace ID is only generated if request tracing was not started yet. If request tracing
+ * was already started the given {@code traceIdConsumer} is 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
@@ -45,9 +50,21 @@
return open();
}
- RequestId traceId = new RequestId();
- traceIdConsumer.accept(RequestId.Type.TRACE_ID.name(), traceId.toString());
- return open().addTag(RequestId.Type.TRACE_ID, traceId).forceLogging();
+ Optional<String> traceId =
+ LoggingContext.getInstance()
+ .getTagsAsMap()
+ .get(RequestId.Type.TRACE_ID.name())
+ .stream()
+ .findAny();
+ if (traceId.isPresent()) {
+ // request tracing was already started, no need to generate a new trace ID
+ traceIdConsumer.accept(RequestId.Type.TRACE_ID.name(), traceId.get());
+ return open();
+ }
+
+ RequestId newTraceId = new RequestId();
+ traceIdConsumer.accept(RequestId.Type.TRACE_ID.name(), newTraceId.toString());
+ return open().addTag(RequestId.Type.TRACE_ID, newTraceId).forceLogging();
}
@FunctionalInterface
diff --git a/javatests/com/google/gerrit/server/logging/TraceContextTest.java b/javatests/com/google/gerrit/server/logging/TraceContextTest.java
index cd3ad4c..7969545 100644
--- a/javatests/com/google/gerrit/server/logging/TraceContextTest.java
+++ b/javatests/com/google/gerrit/server/logging/TraceContextTest.java
@@ -178,6 +178,24 @@
assertThat(traceIdConsumer.traceId).isNull();
}
+ @Test
+ public void onlyOneTraceId() {
+ TestTraceIdConsumer traceIdConsumer1 = new TestTraceIdConsumer();
+ try (TraceContext traceContext1 = TraceContext.newTrace(true, traceIdConsumer1)) {
+ String expectedTraceId = traceIdConsumer1.traceId;
+ assertThat(expectedTraceId).isNotNull();
+
+ TestTraceIdConsumer traceIdConsumer2 = new TestTraceIdConsumer();
+ try (TraceContext traceContext2 = TraceContext.newTrace(true, traceIdConsumer2)) {
+ assertForceLogging(true);
+ assertTags(
+ ImmutableMap.of(RequestId.Type.TRACE_ID.name(), ImmutableSet.of(expectedTraceId)));
+ }
+ assertThat(traceIdConsumer2.tagName).isEqualTo(RequestId.Type.TRACE_ID.name());
+ assertThat(traceIdConsumer2.traceId).isEqualTo(expectedTraceId);
+ }
+ }
+
private void assertTags(ImmutableMap<String, ImmutableSet<String>> expectedTagMap) {
SortedMap<String, SortedSet<Object>> actualTagMap =
LoggingContext.getInstance().getTags().asMap();
