Tracer avoids allocating Optional for requestId (#948)

Tracer avoids allocating Optional for requestId

changelog/@unreleased/pr-948.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Tracer avoids allocating Optional for requestId
+  links:
+  - https://github.com/palantir/tracing-java/pull/948

tracing/src/main/java/com/palantir/tracing/Trace.java

@@ -30,6 +30,7 @@
 import java.util.ArrayDeque;
 import java.util.Deque;
 import java.util.Optional;
+import javax.annotation.Nullable;
 
 /**
  * Represents a trace as an ordered list of non-completed spans. Supports adding and removing of spans. This class is
@@ -73,7 +74,7 @@ final OpenSpan startSpan(String operation, String parentSpanId, SpanType type) {
     final OpenSpan startSpan(String operation, SpanType type) {
         Optional<OpenSpan> prevState = top();
         final OpenSpan span;
-        // Avoid lambda allocation in hot paths
+        //noinspection OptionalIsPresent - Avoid lambda allocation in hot paths
         if (prevState.isPresent()) {
             span = OpenSpan.of(
                     operation,
@@ -119,14 +120,15 @@ final String getTraceId() {
     }
 
     /**
-     * The request identifier of this trace.
-     *
+     * The request identifier of this trace, or null if undefined.
+     * <p>
      * The request identifier is an implementation detail of this tracing library. A new identifier is generated
      * each time a new trace is created with a SERVER_INCOMING root span. This is a convenience in order to
      * distinguish between requests with the same traceId.
      */
-    final Optional<String> getRequestId() {
-        return Optional.ofNullable(traceState.requestId());
+    @Nullable
+    final String maybeGetRequestId() {
+        return traceState.requestId();
     }
 
     /**

tracing/src/main/java/com/palantir/tracing/TraceState.java

@@ -61,7 +61,7 @@ String traceId() {
 
     /**
      * The request identifier of this trace.
-     *
+     * <p>
      * The request identifier is an implementation detail of this tracing library. A new identifier is generated
      * each time a new trace is created with a SERVER_INCOMING root span. This is a convenience in order to
      * distinguish between requests with the same traceId.

tracing/src/main/java/com/palantir/tracing/Tracer.java

@@ -112,21 +112,19 @@ public static Optional<TraceMetadata> maybeGetTraceMetadata() {
             return Optional.empty();
         }
 
-        String traceId = trace.getTraceId();
-        Optional<String> requestId = trace.getRequestId();
+        TraceMetadata.Builder builder = TraceMetadata.builder().traceId(trace.getTraceId());
+        String requestId = trace.maybeGetRequestId();
+        if (requestId != null) {
+            builder.requestId(requestId);
+        }
+
         if (trace.isObservable()) {
-            return trace.top().map(openSpan -> TraceMetadata.builder()
-                    .spanId(openSpan.getSpanId())
+            return trace.top().map(openSpan -> builder.spanId(openSpan.getSpanId())
                     .parentSpanId(openSpan.getParentSpanId())
-                    .traceId(traceId)
-                    .requestId(requestId)
                     .build());
         } else {
-            return Optional.of(TraceMetadata.builder()
-                    .spanId(Tracers.randomId())
+            return Optional.of(builder.spanId(Tracers.randomId())
                     .parentSpanId(Optional.empty())
-                    .traceId(traceId)
-                    .requestId(requestId)
                     .build());
         }
     }
@@ -636,6 +634,7 @@ public static <T> void fastCompleteSpan(TagTranslator<? super T> tag, T state) {
 
     private static <T> void completeSpanAndNotifyObservers(
             Optional<OpenSpan> openSpan, TagTranslator<? super T> tag, T state, String traceId) {
+        //noinspection OptionalIsPresent - Avoid lambda allocation in hot paths
         if (openSpan.isPresent()) {
             Tracer.notifyObservers(toSpan(openSpan.get(), tag, state, traceId));
         }
@@ -720,7 +719,7 @@ public void tag(Span.Builder target, Map<String, String> tags) {
 
     /**
      * Subscribes the given (named) span observer to all "span completed" events. Observers are expected to be "cheap",
-     * i.e., do all non-trivial work (logging, sending network messages, etc) asynchronously. If an observer is already
+     * i.e., do all non-trivial work (logging, sending network messages, etc.) asynchronously. If an observer is already
      * registered for the given name, then it gets overwritten by this call. Returns the observer previously associated
      * with the given name, or null if there is no such observer.
      */
@@ -870,7 +869,7 @@ static void setTrace(Trace trace) {
         // Give log appenders access to the trace id and whether the trace is being sampled
         MDC.put(Tracers.TRACE_ID_KEY, trace.getTraceId());
         setTraceSampledMdcIfObservable(trace.isObservable());
-        setTraceRequestId(trace.getRequestId());
+        setTraceRequestId(trace.maybeGetRequestId());
 
         logSettingTrace();
     }
@@ -885,12 +884,12 @@ private static void setTraceSampledMdcIfObservable(boolean observable) {
         }
     }
 
-    private static void setTraceRequestId(Optional<String> requestId) {
-        if (requestId.isPresent()) {
-            MDC.put(Tracers.REQUEST_ID_KEY, requestId.get());
-        } else {
+    private static void setTraceRequestId(@Nullable String requestId) {
+        if (requestId == null) {
             // Ensure MDC state is cleared when there is no request identifier
             MDC.remove(Tracers.REQUEST_ID_KEY);
+        } else {
+            MDC.put(Tracers.REQUEST_ID_KEY, requestId);
         }
     }