Fix opencensus shim spanBuilderWithRemoteParent behavior (#6415)

open-telemetry · May 1, 2024 · e1f707e · e1f707e
1 parent 35bc345
commit e1f707e
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 14 deletions.
diff --git a/...nsus-shim/src/main/java/io/opentelemetry/opencensusshim/OpenTelemetrySpanBuilderImpl.java b/...nsus-shim/src/main/java/io/opentelemetry/opencensusshim/OpenTelemetrySpanBuilderImpl.java
@@ -164,7 +164,11 @@ public Span startSpan() {
       otelSpanBuilder.setParent(Context.current().with((OpenTelemetrySpanImpl) ocParent));
     }
     if (ocRemoteParentSpanContext != null) {
-      otelSpanBuilder.addLink(SpanConverter.mapSpanContext(ocRemoteParentSpanContext));
+      io.opentelemetry.api.trace.SpanContext spanContext =
+          SpanConverter.mapSpanContext(ocRemoteParentSpanContext, /* isRemoteParent= */ true);
+      otelSpanBuilder.setParent(
+          Context.current().with(io.opentelemetry.api.trace.Span.wrap(spanContext)));
+      otelSpanBuilder.addLink(spanContext);
     }
     if (otelKind != null) {
       otelSpanBuilder.setSpanKind(otelKind);

diff --git a/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/SpanConverter.java b/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/SpanConverter.java
@@ -57,13 +57,22 @@ static SpanContext mapSpanContext(io.opentelemetry.api.trace.SpanContext otelSpa
   }
 
   static io.opentelemetry.api.trace.SpanContext mapSpanContext(SpanContext ocSpanContext) {
-    return io.opentelemetry.api.trace.SpanContext.create(
-        ocSpanContext.getTraceId().toLowerBase16(),
-        ocSpanContext.getSpanId().toLowerBase16(),
+    return mapSpanContext(ocSpanContext, /* isRemoteParent= */ false);
+  }
+
+  static io.opentelemetry.api.trace.SpanContext mapSpanContext(
+      SpanContext ocSpanContext, boolean isRemoteParent) {
+    String traceId = ocSpanContext.getTraceId().toLowerBase16();
+    String spanId = ocSpanContext.getSpanId().toLowerBase16();
+    TraceFlags traceFlags =
         ocSpanContext.getTraceOptions().isSampled()
             ? TraceFlags.getSampled()
-            : TraceFlags.getDefault(),
-        mapTracestate(ocSpanContext.getTracestate()));
+            : TraceFlags.getDefault();
+    TraceState traceState = mapTracestate(ocSpanContext.getTracestate());
+    return isRemoteParent
+        ? io.opentelemetry.api.trace.SpanContext.createFromRemoteParent(
+            traceId, spanId, traceFlags, traceState)
+        : io.opentelemetry.api.trace.SpanContext.create(traceId, spanId, traceFlags, traceState);
   }
 
   private static TraceState mapTracestate(Tracestate tracestate) {

diff --git a/opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/InteroperabilityTest.java b/opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/InteroperabilityTest.java
@@ -44,6 +44,7 @@
 import io.opentelemetry.sdk.trace.data.StatusData;
 import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
 import io.opentelemetry.sdk.trace.export.SpanExporter;
+import io.opentelemetry.sdk.trace.samplers.Sampler;
 import java.util.Collection;
 import java.util.concurrent.TimeUnit;
 import org.junit.jupiter.api.BeforeEach;
@@ -71,7 +72,11 @@ class InteroperabilityTest {
     SpanProcessor spanProcessor = SimpleSpanProcessor.create(spanExporter);
     openTelemetry =
         OpenTelemetrySdk.builder()
-            .setTracerProvider(SdkTracerProvider.builder().addSpanProcessor(spanProcessor).build())
+            .setTracerProvider(
+                SdkTracerProvider.builder()
+                    .setSampler(Sampler.alwaysOn())
+                    .addSpanProcessor(spanProcessor)
+                    .build())
             .buildAndRegisterGlobal();
   }
 
@@ -84,7 +89,7 @@ void resetMocks() {
   }
 
   @Test
-  void testParentChildRelationshipsAreExportedCorrectly() {
+  void parentChildRelationshipsAreExportedCorrectly() {
     Tracer tracer = openTelemetry.getTracer("io.opentelemetry.test.scoped.span.1");
     Span span = tracer.spanBuilder("OpenTelemetrySpan").startSpan();
     try (Scope scope = Context.current().with(span).makeCurrent()) {
@@ -128,7 +133,7 @@ void testParentChildRelationshipsAreExportedCorrectly() {
   }
 
   @Test
-  void testRemoteParent() {
+  void remoteParent() {
     io.opencensus.trace.Tracer tracer = Tracing.getTracer();
     io.opencensus.trace.Span remoteParentSpan =
         tracer.spanBuilder("remote parent span").startSpan();
@@ -148,14 +153,23 @@ void testRemoteParent() {
 
     assertThat(export1.size()).isEqualTo(1);
     SpanData spanData1 = export1.iterator().next();
+
+    // Remote parent should be set to parent span context
+    assertThat(spanData1.getParentSpanContext().isRemote()).isTrue();
+    assertThat(spanData1.getParentSpanContext().getTraceId())
+        .isEqualTo(remoteParentSpan.getContext().getTraceId().toLowerBase16());
+    assertThat(spanData1.getParentSpanContext().getSpanId())
+        .isEqualTo(remoteParentSpan.getContext().getSpanId().toLowerBase16());
+
+    // Remote parent should be added as link
     assertThat(spanData1.getName()).isEqualTo("OpenCensusSpan");
     assertThat(spanData1.getLinks().get(0).getSpanContext().getSpanId())
         .isEqualTo(remoteParentSpan.getContext().getSpanId().toLowerBase16());
   }
 
   @Test
   @SuppressLogger(OpenTelemetrySpanImpl.class)
-  void testParentChildRelationshipsAreExportedCorrectlyForOpenCensusOnly() {
+  void parentChildRelationshipsAreExportedCorrectlyForOpenCensusOnly() {
     io.opencensus.trace.Tracer tracer = Tracing.getTracer();
     io.opencensus.trace.Span parentLinkSpan = tracer.spanBuilder("parent link span").startSpan();
     try (io.opencensus.common.Scope scope =
@@ -261,7 +275,7 @@ void testParentChildRelationshipsAreExportedCorrectlyForOpenCensusOnly() {
   }
 
   @Test
-  void testOpenTelemetryMethodsOnOpenCensusSpans() {
+  void openTelemetryMethodsOnOpenCensusSpans() {
     io.opencensus.trace.Tracer tracer = Tracing.getTracer();
     try (io.opencensus.common.Scope scope =
         tracer
@@ -308,7 +322,7 @@ void testOpenTelemetryMethodsOnOpenCensusSpans() {
   }
 
   @Test
-  public void testNoSampleDoesNotExport() {
+  public void noSampleDoesNotExport() {
     io.opencensus.trace.Tracer tracer = Tracing.getTracer();
     try (io.opencensus.common.Scope scope =
         tracer.spanBuilder("OpenCensusSpan").setSampler(Samplers.neverSample()).startScopedSpan()) {
@@ -324,14 +338,14 @@ public void testNoSampleDoesNotExport() {
   }
 
   @Test
-  public void testOpenCensusSamplerIsAlwaysOn() {
+  public void openCensusSamplerIsAlwaysOn() {
     // OpenTelemetryTraceComponentImpl provides this behavior
     assertThat(Tracing.getTraceConfig().getActiveTraceParams().getSampler())
         .isEqualTo(Samplers.alwaysSample());
   }
 
   @Test
-  public void testByDefaultDoesExport() {
+  public void byDefaultDoesExport() {
     io.opencensus.trace.Tracer tracer = Tracing.getTracer();
     try (io.opencensus.common.Scope scope =
         tracer.spanBuilder("OpenCensusSpan").setRecordEvents(false).startScopedSpan()) {