Set OT Shim's SpanBuilder attributes before the Span is created. (open-telemetry#3984)

carlosalberto · shawnsong · web-flow · commit 3940dd254e28 · 2021-12-13T14:45:33.000+09:00
* Set OT Shim's SpanBuilder attributes before the Span is created.

* Update opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java

Co-authored-by: Shawn Song &lt;mr.song.shawn@gmail.com&gt;

Co-authored-by: Shawn Song &lt;mr.song.shawn@gmail.com&gt;
diff --git a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java
@@ -214,13 +214,16 @@ public Span start() {
       builder.setStartTimestamp(startTimestampMicros, TimeUnit.MICROSECONDS);
     }
 
-    io.opentelemetry.api.trace.Span span = builder.startSpan();
-
+    // Attributes passed to the OT SpanBuilder MUST
+    // be set before the OTel Span is created,
+    // so those attributes are available to the Sampling API.
     for (int i = 0; i < this.spanBuilderAttributeKeys.size(); i++) {
       AttributeKey key = this.spanBuilderAttributeKeys.get(i);
       Object value = this.spanBuilderAttributeValues.get(i);
-      span.setAttribute(key, value);
+      builder.setAttribute(key, value);
     }
+
+    io.opentelemetry.api.trace.Span span = builder.startSpan();
     if (error) {
       span.setStatus(StatusCode.ERROR);
     }
diff --git a/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/SpanBuilderShimTest.java b/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/SpanBuilderShimTest.java
@@ -9,10 +9,18 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.Tracer;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.trace.ReadableSpan;
 import io.opentelemetry.sdk.trace.SdkTracerProvider;
+import io.opentelemetry.sdk.trace.data.LinkData;
 import io.opentelemetry.sdk.trace.data.SpanData;
+import io.opentelemetry.sdk.trace.samplers.Sampler;
+import io.opentelemetry.sdk.trace.samplers.SamplingResult;
+import java.util.List;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -97,4 +105,62 @@ void withStartTimestamp() {
     SpanData spanData = ((ReadableSpan) spanShim.getSpan()).toSpanData();
     assertThat(spanData.getStartEpochNanos()).isEqualTo(micros * 1000L);
   }
+
+  @Test
+  void setAttributes_beforeSpanStart() {
+    SdkTracerProvider tracerSdkFactory =
+        SdkTracerProvider.builder().setSampler(SamplingPrioritySampler.INSTANCE).build();
+    Tracer tracer = tracerSdkFactory.get("SpanShimTest");
+    TelemetryInfo telemetryInfo =
+        new TelemetryInfo(tracer, OpenTracingPropagators.builder().build());
+
+    SpanBuilderShim spanBuilder1 = new SpanBuilderShim(telemetryInfo, SPAN_NAME);
+    SpanBuilderShim spanBuilder2 = new SpanBuilderShim(telemetryInfo, SPAN_NAME);
+    SpanShim span1 =
+        (SpanShim)
+            spanBuilder1
+                .withTag(
+                    SamplingPrioritySampler.SAMPLING_PRIORITY_TAG,
+                    SamplingPrioritySampler.UNSAMPLED_VALUE)
+                .start();
+    SpanShim span2 =
+        (SpanShim)
+            spanBuilder2
+                .withTag(
+                    SamplingPrioritySampler.SAMPLING_PRIORITY_TAG,
+                    SamplingPrioritySampler.SAMPLED_VALUE)
+                .start();
+    assertThat(span1.getSpan().isRecording()).isFalse();
+    assertThat(span2.getSpan().isRecording()).isTrue();
+    assertThat(span1.getSpan().getSpanContext().isSampled()).isFalse();
+    assertThat(span2.getSpan().getSpanContext().isSampled()).isTrue();
+  }
+
+  static final class SamplingPrioritySampler implements Sampler {
+    static final SamplingPrioritySampler INSTANCE = new SamplingPrioritySampler();
+    static final String SAMPLING_PRIORITY_TAG = "sampling.priority";
+    static final long UNSAMPLED_VALUE = 0;
+    static final long SAMPLED_VALUE = 1;
+
+    @Override
+    public String getDescription() {
+      return "SamplingPrioritySampler";
+    }
+
+    @Override
+    public SamplingResult shouldSample(
+        Context parentContext,
+        String traceId,
+        String name,
+        SpanKind spanKind,
+        Attributes attributes,
+        List<LinkData> parentLinks) {
+
+      if (attributes.get(AttributeKey.longKey(SAMPLING_PRIORITY_TAG)) == UNSAMPLED_VALUE) {
+        return SamplingResult.drop();
+      }
+
+      return SamplingResult.recordAndSample();
+    }
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -214,13 +214,16 @@ public Span start() {`
`214`	`214`	`builder.setStartTimestamp(startTimestampMicros, TimeUnit.MICROSECONDS);`
`215`	`215`	`}`
`216`	`216`
`217`		`- io.opentelemetry.api.trace.Span span = builder.startSpan();`
`218`		`-`
	`217`	`+ // Attributes passed to the OT SpanBuilder MUST`
	`218`	`+ // be set before the OTel Span is created,`
	`219`	`+ // so those attributes are available to the Sampling API.`
`219`	`220`	`for (int i = 0; i < this.spanBuilderAttributeKeys.size(); i++) {`
`220`	`221`	`AttributeKey key = this.spanBuilderAttributeKeys.get(i);`
`221`	`222`	`Object value = this.spanBuilderAttributeValues.get(i);`
`222`		`- span.setAttribute(key, value);`
	`223`	`+ builder.setAttribute(key, value);`
`223`	`224`	`}`
	`225`	`+`
	`226`	`+ io.opentelemetry.api.trace.Span span = builder.startSpan();`
`224`	`227`	`if (error) {`
`225`	`228`	`span.setStatus(StatusCode.ERROR);`
`226`	`229`	`}`