Clean up some Tracer helper methods (#1494)

Author: pkoenig10

tracing-api/src/main/java/com/palantir/tracing/api/OpenSpan.java

@@ -16,7 +16,6 @@
 
 package com.palantir.tracing.api;
 
-import java.time.Clock;
 import java.time.Instant;
 import java.util.Optional;
 import org.immutables.value.Value;
@@ -28,7 +27,6 @@
 @Value.Immutable
 @ImmutablesStyle
 public abstract class OpenSpan {
-    private static final Clock CLOCK = Clock.systemUTC();
 
     /** Returns a description of the operation for this event. */
     @Value.Parameter
@@ -109,7 +107,7 @@ public static OpenSpan of(String operation, String spanId, SpanType type, Option
     }
 
     private static long getNowInMicroSeconds() {
-        Instant now = CLOCK.instant();
+        Instant now = Instant.now();
         return (1000000 * now.getEpochSecond()) + (now.getNano() / 1000);
     }
 

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

@@ -92,7 +92,7 @@ public DeferredTracer(@Safe Optional<String> operation) {
     public DeferredTracer(@Safe String operation, @Safe Map<String, String> metadata) {
         Trace trace = Tracer.getTrace();
         if (trace != null) {
-            OpenSpan span = trace.top();
+            OpenSpan span = trace.current();
             this.traceState = trace.traceState();
             this.parentSpanId = span != null ? span.getSpanId() : null;
             this.operation = operation;

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

@@ -16,12 +16,8 @@
 
 package com.palantir.tracing;
 
-import static com.palantir.logsafe.Preconditions.checkArgument;
 import static com.palantir.logsafe.Preconditions.checkNotNull;
-import static com.palantir.logsafe.Preconditions.checkState;
 
-import com.google.common.base.Strings;
-import com.google.errorprone.annotations.CheckReturnValue;
 import com.palantir.logsafe.SafeArg;
 import com.palantir.logsafe.exceptions.SafeIllegalStateException;
 import com.palantir.tracing.api.OpenSpan;
@@ -51,49 +47,15 @@ private Trace(TraceState traceState) {
         this.traceState = traceState;
     }
 
-    /**
-     * Opens a new span for this thread's call trace, labeled with the provided operation and parent span. Only allowed
-     * when the current trace is empty. If the return value is not used, prefer {@link #fastStartSpan(String, String,
-     * SpanType)}}.
-     */
-    @CheckReturnValue
-    final OpenSpan startSpan(String operation, String parentSpanId, SpanType type) {
-        checkState(isEmpty(), "Cannot start a span with explicit parent if the current thread's trace is non-empty");
-        checkArgument(!Strings.isNullOrEmpty(parentSpanId), "parentSpanId must be non-empty");
-
-        return startSpan(operation, Optional.of(parentSpanId), type);
-    }
-
-    /**
-     * Opens a new span for this thread's call trace, labeled with the provided operation. If the return value is not
-     * used, prefer {@link #fastStartSpan(String, SpanType)}}.
-     */
-    @CheckReturnValue
-    final OpenSpan startSpan(String operation, SpanType type) {
-        OpenSpan span = top();
+    abstract void fastStartSpan(String operation, Optional<String> parentSpanId, SpanType type);
 
-        return startSpan(operation, span != null ? Optional.of(span.getSpanId()) : Optional.empty(), type);
-    }
-
-    private OpenSpan startSpan(String operation, Optional<String> parentSpanId, SpanType type) {
-        OpenSpan span = OpenSpan.of(operation, Tracers.randomId(), type, parentSpanId);
-        push(span);
-        return span;
-    }
-
-    /** Like {@link #startSpan(String, String, SpanType)}, but does not return an {@link OpenSpan}. */
-    abstract void fastStartSpan(String operation, String parentSpanId, SpanType type);
-
-    /** Like {@link #startSpan(String, SpanType)}, but does not return an {@link OpenSpan}. */
-    abstract void fastStartSpan(String operation, SpanType type);
+    @Nullable
+    abstract OpenSpan fastCompleteSpan();
 
     abstract void push(OpenSpan span);
 
     @Nullable
-    abstract OpenSpan top();
-
-    @Nullable
-    abstract OpenSpan pop();
+    abstract OpenSpan current();
 
     abstract boolean isEmpty();
 
@@ -120,15 +82,15 @@ private Sampled(TraceState traceState) {
         }
 
         @Override
-        @SuppressWarnings("ResultOfMethodCallIgnored") // Sampled traces cannot optimize this path
-        void fastStartSpan(String operation, String parentSpanId, SpanType type) {
-            startSpan(operation, parentSpanId, type);
+        void fastStartSpan(String operation, Optional<String> parentSpanId, SpanType type) {
+            OpenSpan span = OpenSpan.of(operation, Tracers.randomId(), type, parentSpanId);
+            push(span);
         }
 
         @Override
-        @SuppressWarnings("ResultOfMethodCallIgnored") // Sampled traces cannot optimize this path
-        void fastStartSpan(String operation, SpanType type) {
-            startSpan(operation, type);
+        @Nullable
+        OpenSpan fastCompleteSpan() {
+            return stack.pollFirst();
         }
 
         @Override
@@ -138,16 +100,10 @@ protected void push(OpenSpan span) {
 
         @Override
         @Nullable
-        OpenSpan top() {
+        OpenSpan current() {
             return stack.peekFirst();
         }
 
-        @Override
-        @Nullable
-        OpenSpan pop() {
-            return stack.pollFirst();
-        }
-
         @Override
         boolean isEmpty() {
             return stack.isEmpty();
@@ -177,13 +133,18 @@ private Unsampled(TraceState traceState) {
         }
 
         @Override
-        void fastStartSpan(String _operation, String _parentSpanId, SpanType _type) {
+        void fastStartSpan(String _operation, Optional<String> _parentSpanId, SpanType _type) {
             numberOfSpans++;
         }
 
         @Override
-        void fastStartSpan(String _operation, SpanType _type) {
-            numberOfSpans++;
+        @Nullable
+        OpenSpan fastCompleteSpan() {
+            validateNumberOfSpans();
+            if (numberOfSpans > 0) {
+                numberOfSpans--;
+            }
+            return null;
         }
 
         @Override
@@ -193,17 +154,7 @@ protected void push(OpenSpan _span) {
 
         @Override
         @Nullable
-        OpenSpan top() {
-            return null;
-        }
-
-        @Override
-        @Nullable
-        OpenSpan pop() {
-            validateNumberOfSpans();
-            if (numberOfSpans > 0) {
-                numberOfSpans--;
-            }
+        OpenSpan current() {
             return null;
         }
 
@@ -213,7 +164,7 @@ boolean isEmpty() {
             return numberOfSpans <= 0;
         }
 
-        /** Internal validation, this should never fail because {@link #pop()} only decrements positive values. */
+        /** Internal validation, this should never fail because {@link #fastCompleteSpan()} only decrements positive values. */
         private void validateNumberOfSpans() {
             if (numberOfSpans < 0) {
                 throw new SafeIllegalStateException(