Standardize trace propagation

Previously the Parent and Span ids were manipulated based on what they "should" be once they got to the child. That's not really how it is supposed to work.

Now the exact span context is sent from the parent to the child and the child can do what they need with it.

genie-agent/src/main/java/com/netflix/genie/agent/cli/GenieAgentRunner.java

@@ -18,7 +18,6 @@
 package com.netflix.genie.agent.cli;
 
 import brave.ScopedSpan;
-import brave.Span;
 import brave.SpanCustomizer;
 import brave.Tracer;
 import brave.propagation.TraceContext;
@@ -148,17 +147,15 @@ public int getExitCode() {
     private ScopedSpan initializeTracing() {
         // Attempt to extract any existing trace information from the environment
         final Optional<TraceContext> existingTraceContext = this.tracePropagator.extract(System.getenv());
-        final Span initSpan = existingTraceContext.isPresent()
-            ? this.tracer.joinSpan(existingTraceContext.get())
-            : this.tracer.nextSpan();
+        final ScopedSpan initSpan = existingTraceContext.isPresent()
+            ? this.tracer.startScopedSpanWithParent(INIT_SPAN_NAME, existingTraceContext.get())
+            : this.tracer.startScopedSpan(INIT_SPAN_NAME);
         // Quickly create and report an initial span
         final TraceContext initContext = initSpan.context();
-        final String traceId = initContext.traceIdString();
-        log.info("Trace ID: {}", traceId);
-        ConsoleLog.getLogger().info("Trace ID: {}", traceId);
         try {
-            initSpan.name(INIT_SPAN_NAME);
-            initSpan.start();
+            final String traceId = initContext.traceIdString();
+            log.info("Trace ID: {}", traceId);
+            ConsoleLog.getLogger().info("Trace ID: {}", traceId);
         } finally {
             initSpan.finish();
         }

genie-agent/src/test/groovy/com/netflix/genie/agent/cli/GenieAgentRunnerSpec.groovy

@@ -18,7 +18,6 @@
 package com.netflix.genie.agent.cli
 
 import brave.ScopedSpan
-import brave.Span
 import brave.Tracer
 import brave.propagation.TraceContext
 import com.beust.jcommander.ParameterException
@@ -41,7 +40,7 @@ class GenieAgentRunnerSpec extends Specification {
     BraveTracePropagator tracePropagator
     BraveTracingCleanup traceCleaner
     BraveTagAdapter tagAdapter
-    Span initSpan
+    ScopedSpan initSpan
     ScopedSpan runSpan
     TraceContext initContext
 
@@ -55,7 +54,7 @@ class GenieAgentRunnerSpec extends Specification {
         this.traceCleaner = Mock(BraveTracingCleanup)
         this.tagAdapter = Mock(BraveTagAdapter)
         this.args = new String[0]
-        this.initSpan = Mock(Span)
+        this.initSpan = Mock(ScopedSpan)
         this.runSpan = Mock(ScopedSpan)
         this.runner = new GenieAgentRunner(
             this.argsParser,
@@ -77,11 +76,9 @@ class GenieAgentRunnerSpec extends Specification {
 
         then:
         1 * this.tracePropagator.extract(_ as Map) >> Optional.empty()
-        0 * this.tracer.joinSpan(_ as TraceContext)
-        1 * this.tracer.nextSpan() >> this.initSpan
+        0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext)
+        1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan
         1 * this.initSpan.context() >> this.initContext
-        1 * this.initSpan.name(GenieAgentRunner.INIT_SPAN_NAME)
-        1 * this.initSpan.start()
         1 * this.initSpan.finish()
         1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan
         1 * this.runSpan.finish()
@@ -106,11 +103,9 @@ class GenieAgentRunnerSpec extends Specification {
 
         then:
         1 * this.tracePropagator.extract(_ as Map) >> Optional.empty()
-        0 * this.tracer.joinSpan(_ as TraceContext)
-        1 * this.tracer.nextSpan() >> this.initSpan
+        0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext)
+        1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan
         1 * this.initSpan.context() >> this.initContext
-        1 * this.initSpan.name(GenieAgentRunner.INIT_SPAN_NAME)
-        1 * this.initSpan.start()
         1 * this.initSpan.finish()
         1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan
         1 * this.runSpan.error(_ as Throwable)
@@ -134,11 +129,9 @@ class GenieAgentRunnerSpec extends Specification {
 
         then:
         1 * this.tracePropagator.extract(_ as Map) >> Optional.empty()
-        0 * this.tracer.joinSpan(_ as TraceContext)
-        1 * this.tracer.nextSpan() >> this.initSpan
+        0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext)
+        1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan
         1 * this.initSpan.context() >> this.initContext
-        1 * this.initSpan.name(GenieAgentRunner.INIT_SPAN_NAME)
-        1 * this.initSpan.start()
         1 * this.initSpan.finish()
         1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan
         1 * this.runSpan.error(_ as Throwable)
@@ -162,11 +155,9 @@ class GenieAgentRunnerSpec extends Specification {
 
         then:
         1 * this.tracePropagator.extract(_ as Map) >> Optional.empty()
-        0 * this.tracer.joinSpan(_ as TraceContext)
-        1 * this.tracer.nextSpan() >> this.initSpan
+        0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext)
+        1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan
         1 * this.initSpan.context() >> this.initContext
-        1 * this.initSpan.name(GenieAgentRunner.INIT_SPAN_NAME)
-        1 * this.initSpan.start()
         1 * this.initSpan.finish()
         1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan
         1 * this.runSpan.error(_ as Throwable)
@@ -190,11 +181,9 @@ class GenieAgentRunnerSpec extends Specification {
 
         then:
         1 * this.tracePropagator.extract(_ as Map) >> Optional.empty()
-        0 * this.tracer.joinSpan(_ as TraceContext)
-        1 * this.tracer.nextSpan() >> this.initSpan
+        0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext)
+        1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan
         1 * this.initSpan.context() >> this.initContext
-        1 * this.initSpan.name(GenieAgentRunner.INIT_SPAN_NAME)
-        1 * this.initSpan.start()
         1 * this.initSpan.finish()
         1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan
         1 * this.runSpan.error(_ as Throwable)
@@ -218,11 +207,9 @@ class GenieAgentRunnerSpec extends Specification {
 
         then:
         1 * this.tracePropagator.extract(_ as Map) >> Optional.empty()
-        0 * this.tracer.joinSpan(_ as TraceContext)
-        1 * this.tracer.nextSpan() >> this.initSpan
+        0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext)
+        1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan
         1 * this.initSpan.context() >> this.initContext
-        1 * this.initSpan.name(GenieAgentRunner.INIT_SPAN_NAME)
-        1 * this.initSpan.start()
         1 * this.initSpan.finish()
         1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan
         1 * this.runSpan.error(_ as Throwable)

genie-common-internal/src/main/java/com/netflix/genie/common/internal/tracing/brave/impl/EnvVarBraveTracePropagatorImpl.java

@@ -27,7 +27,6 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
-import java.util.UUID;
 
 /**
  * Implementation of {@link TracePropagator} based on <a href="https://github.com/openzipkin/brave">Brave</a> and
@@ -55,6 +54,13 @@ public class EnvVarBraveTracePropagatorImpl implements BraveTracePropagator {
      */
     static final String GENIE_AGENT_B3_TRACE_ID_HIGH_KEY = "GENIE_AGENT_B3_TRACE_ID_HIGH";
 
+    /**
+     * The key for the span id.
+     *
+     * @see TraceContext.Builder#spanId(long)
+     */
+    static final String GENIE_AGENT_B3_SPAN_ID_KEY = "GENIE_AGENT_B3_SPAN_ID";
+
     /**
      * The key for the parent span.
      *
@@ -83,6 +89,13 @@ public class EnvVarBraveTracePropagatorImpl implements BraveTracePropagator {
      */
     static final String GENIE_JOB_B3_TRACE_ID_HIGH_KEY = "GENIE_B3_TRACE_ID_HIGH";
 
+    /**
+     * The key for the span id.
+     *
+     * @see TraceContext.Builder#spanId(long)
+     */
+    static final String GENIE_JOB_B3_SPAN_ID_KEY = "GENIE_B3_SPAN_ID";
+
     /**
      * The key for the parent span of the job.
      *
@@ -117,31 +130,33 @@ public Optional<TraceContext> extract(final Map<String, String> environment) {
                 LOG.debug("No trace id low found in the supplied set of key value pairs. Can't extract trace context");
                 return Optional.empty();
             }
-            final String parentSpanId = environment.get(GENIE_AGENT_B3_PARENT_SPAN_ID_KEY);
-            if (StringUtils.isBlank(parentSpanId)) {
+            final String spanId = environment.get(GENIE_AGENT_B3_SPAN_ID_KEY);
+            if (StringUtils.isBlank(spanId)) {
                 LOG.debug(
-                    "No parent span id found in the supplied set of key value pairs. Can't extract trace context"
+                    "No span id found in the supplied set of key value pairs. Can't extract trace context"
                 );
                 return Optional.empty();
             }
 
             final TraceContext.Builder builder = TraceContext.newBuilder();
             builder.traceId(Long.parseLong(traceIdLow));
-            builder.parentId(Long.parseLong(parentSpanId));
+            builder.spanId(Long.parseLong(spanId));
 
             final String traceIdHigh = environment.get(GENIE_AGENT_B3_TRACE_ID_HIGH_KEY);
             if (StringUtils.isNotBlank(traceIdHigh)) {
                 builder.traceIdHigh(Long.parseLong(traceIdHigh));
             }
 
+            final String parentSpanId = environment.get(GENIE_AGENT_B3_PARENT_SPAN_ID_KEY);
+            if (StringUtils.isNotBlank(parentSpanId)) {
+                builder.parentId(Long.parseLong(parentSpanId));
+            }
+
             final String sampled = environment.get(GENIE_AGENT_B3_SAMPLED_KEY);
             if (StringUtils.isNotBlank(sampled)) {
                 builder.sampled(Boolean.parseBoolean(sampled));
             }
 
-            final long spanId = UUID.randomUUID().getMostSignificantBits();
-            builder.spanId(spanId);
-
             LOG.debug(
                 "Extracted trace context: "
                     + "Trace Id Low = {}, "
@@ -191,12 +206,12 @@ private Map<String, String> inject(final TraceContext traceContext, final boolea
                 Long.toString(traceContext.traceId())
             );
             propagationContext.put(
-                isForAgent ? GENIE_AGENT_B3_SAMPLED_KEY : GENIE_JOB_B3_SAMPLED_KEY,
-                traceContext.sampled().toString()
+                isForAgent ? GENIE_AGENT_B3_SPAN_ID_KEY : GENIE_JOB_B3_SPAN_ID_KEY,
+                Long.toString(traceContext.spanId())
             );
             propagationContext.put(
-                isForAgent ? GENIE_AGENT_B3_PARENT_SPAN_ID_KEY : GENIE_JOB_B3_PARENT_SPAN_ID_KEY,
-                Long.toString(traceContext.spanId())
+                isForAgent ? GENIE_AGENT_B3_SAMPLED_KEY : GENIE_JOB_B3_SAMPLED_KEY,
+                traceContext.sampled().toString()
             );
 
             // only propagate high bits of trace id if they're non-zero
@@ -208,6 +223,14 @@ private Map<String, String> inject(final TraceContext traceContext, final boolea
                 );
             }
 
+            final Long parentSpanId = traceContext.parentId();
+            if (parentSpanId != null) {
+                propagationContext.put(
+                    isForAgent ? GENIE_AGENT_B3_PARENT_SPAN_ID_KEY : GENIE_JOB_B3_PARENT_SPAN_ID_KEY,
+                    Long.toString(parentSpanId)
+                );
+            }
+
             return propagationContext;
         } catch (final Throwable t) {
             LOG.warn(

genie-common-internal/src/test/groovy/com/netflix/genie/common/internal/tracing/brave/impl/EnvVarBraveTracePropagatorImplSpec.groovy

@@ -54,7 +54,7 @@ class EnvVarBraveTracePropagatorImplSpec extends Specification {
         noExceptionThrown()
         !context.isPresent()
 
-        when: "Trace id is present but no parent span"
+        when: "Trace id is present but no span id"
         environment.put(
             EnvVarBraveTracePropagatorImpl.GENIE_AGENT_B3_TRACE_ID_LOW_KEY,
             Long.toString(UUID.randomUUID().getMostSignificantBits())
@@ -69,15 +69,16 @@ class EnvVarBraveTracePropagatorImplSpec extends Specification {
     def "Can inject and extract for agent propagation"() {
         def traceIdLow = UUID.randomUUID().getMostSignificantBits()
         def traceIdHigh = UUID.randomUUID().getLeastSignificantBits()
+        def spanId = UUID.randomUUID().getLeastSignificantBits()
         def sampled = true
         def parentSpanId = UUID.randomUUID().getMostSignificantBits()
 
         def parentTraceContext = TraceContext.newBuilder()
             .traceId(traceIdLow)
             .traceIdHigh(traceIdHigh)
             .sampled(sampled)
-            .parentId(UUID.randomUUID().getMostSignificantBits())
-            .spanId(parentSpanId)
+            .parentId(parentSpanId)
+            .spanId(spanId)
             .build()
 
         when:
@@ -86,6 +87,7 @@ class EnvVarBraveTracePropagatorImplSpec extends Specification {
         then:
         environment.get(EnvVarBraveTracePropagatorImpl.GENIE_AGENT_B3_TRACE_ID_LOW_KEY) == Long.toString(traceIdLow)
         environment.get(EnvVarBraveTracePropagatorImpl.GENIE_AGENT_B3_TRACE_ID_HIGH_KEY) == Long.toString(traceIdHigh)
+        environment.get(EnvVarBraveTracePropagatorImpl.GENIE_AGENT_B3_SPAN_ID_KEY) == Long.toString(spanId)
         environment.get(EnvVarBraveTracePropagatorImpl.GENIE_AGENT_B3_PARENT_SPAN_ID_KEY) == Long.toString(parentSpanId)
         environment.get(EnvVarBraveTracePropagatorImpl.GENIE_AGENT_B3_SAMPLED_KEY) == Boolean.toString(sampled)
         environment.get(EnvVarBraveTracePropagatorImpl.GENIE_JOB_B3_TRACE_ID_LOW_KEY) == null
@@ -99,20 +101,20 @@ class EnvVarBraveTracePropagatorImplSpec extends Specification {
         then:
         childTraceContext != null
         childTraceContext.traceIdString() == parentTraceContext.traceIdString()
-        childTraceContext.parentIdString() == parentTraceContext.spanIdString()
+        childTraceContext.spanIdString() == parentTraceContext.spanIdString()
+        childTraceContext.parentIdString() == parentTraceContext.parentIdString()
         childTraceContext.sampled() == parentTraceContext.sampled()
     }
 
     def "Can inject for job propagation"() {
         def traceIdLow = UUID.randomUUID().getMostSignificantBits()
         def sampled = false
-        def parentSpanId = UUID.randomUUID().getMostSignificantBits()
+        def spanId = UUID.randomUUID().getLeastSignificantBits()
 
         def parentTraceContext = TraceContext.newBuilder()
             .traceId(traceIdLow)
             .sampled(sampled)
-            .parentId(UUID.randomUUID().getMostSignificantBits())
-            .spanId(parentSpanId)
+            .spanId(spanId)
             .build()
 
         when:
@@ -125,7 +127,8 @@ class EnvVarBraveTracePropagatorImplSpec extends Specification {
         environment.get(EnvVarBraveTracePropagatorImpl.GENIE_AGENT_B3_SAMPLED_KEY) == null
         environment.get(EnvVarBraveTracePropagatorImpl.GENIE_JOB_B3_TRACE_ID_LOW_KEY) == Long.toString(traceIdLow)
         environment.get(EnvVarBraveTracePropagatorImpl.GENIE_JOB_B3_TRACE_ID_HIGH_KEY) == null
-        environment.get(EnvVarBraveTracePropagatorImpl.GENIE_JOB_B3_PARENT_SPAN_ID_KEY) == Long.toString(parentSpanId)
+        environment.get(EnvVarBraveTracePropagatorImpl.GENIE_JOB_B3_SPAN_ID_KEY) == Long.toString(spanId)
+        environment.get(EnvVarBraveTracePropagatorImpl.GENIE_JOB_B3_PARENT_SPAN_ID_KEY) == null
         environment.get(EnvVarBraveTracePropagatorImpl.GENIE_JOB_B3_SAMPLED_KEY) == Boolean.toString(sampled)
     }
 }