Replace span.ended with span.started, add parent.origin attrib

Author: JonasKunz

sdk/common/src/main/java/io/opentelemetry/sdk/internal/SemConvAttributes.java

@@ -28,6 +28,10 @@ private SemConvAttributes() {}
   public static final AttributeKey<String> OTEL_SPAN_SAMPLING_RESULT =
       AttributeKey.stringKey("otel.span.sampling_result");
 
+  // TODO: Add tests verifying correctness when included in the semconv-incubating release
+  public static final AttributeKey<String> OTEL_SPAN_PARENT_ORIGIN =
+      AttributeKey.stringKey("otel.span.parent.origin");
+
   public static final AttributeKey<String> SERVER_ADDRESS =
       AttributeKey.stringKey("server.address");
   public static final AttributeKey<Long> SERVER_PORT = AttributeKey.longKey("server.port");

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/NonRecordingSpan.java

@@ -206,9 +206,13 @@ public Span startSpan() {
             tracerSharedState.isIdGeneratorSafeToSkipIdValidation());
 
     SpanInstrumentation.Recording metricsRecording =
-        tracerSharedState.getSpanMetrics().recordSpanStart(samplingResult);
+        tracerSharedState.getSpanMetrics().recordSpanStart(samplingResult, parentSpanContext);
     if (!isRecording(samplingDecision)) {
-      return new NonRecordingSpan(spanContext, metricsRecording);
+      if (!metricsRecording.isNoop()) {
+        throw new IllegalStateException(
+            "instrumentation ending is not supported for non-recording spans.");
+      }
+      return Span.wrap(spanContext);
     }
     Attributes samplingAttributes = samplingResult.getAttributes();
     if (!samplingAttributes.isEmpty()) {

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java

@@ -5,20 +5,27 @@
 
 package io.opentelemetry.sdk.trace.internal;
 
+import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.sdk.trace.samplers.SamplingResult;
 
 class NoopSpanInstrumentation implements SpanInstrumentation {
 
   static final NoopSpanInstrumentation INSTANCE = new NoopSpanInstrumentation();
 
+  static final SpanInstrumentation.Recording RECORDING_INSTANCE = new NoopRecording();
+
   @Override
-  public SpanInstrumentation.Recording recordSpanStart(SamplingResult samplingResult) {
-    return NoopRecording.INSTANCE;
+  public SpanInstrumentation.Recording recordSpanStart(
+      SamplingResult samplingResult, SpanContext parentContext) {
+    return RECORDING_INSTANCE;
   }
 
   private static class NoopRecording implements SpanInstrumentation.Recording {
 
-    private static final NoopRecording INSTANCE = new NoopRecording();
+    @Override
+    public boolean isNoop() {
+      return true;
+    }
 
     @Override
     public void recordSpanEnd() {}

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/NoopSpanInstrumentation.java

@@ -11,6 +11,7 @@
 import io.opentelemetry.api.metrics.LongUpDownCounter;
 import io.opentelemetry.api.metrics.Meter;
 import io.opentelemetry.api.metrics.MeterProvider;
+import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.sdk.internal.SemConvAttributes;
 import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
 import io.opentelemetry.sdk.trace.samplers.SamplingResult;
@@ -29,7 +30,7 @@ class SemConvSpanInstrumentation implements SpanInstrumentation {
       Attributes.of(SemConvAttributes.OTEL_SPAN_SAMPLING_RESULT, "RECORD_AND_SAMPLE");
 
   @Nullable private volatile LongUpDownCounter live = null;
-  @Nullable private volatile LongCounter ended = null;
+  @Nullable private volatile LongCounter started = null;
 
   SemConvSpanInstrumentation(Supplier<MeterProvider> meterProviderSupplier) {
     this.meterProviderSupplier = meterProviderSupplier;
@@ -58,18 +59,18 @@ private LongUpDownCounter live() {
     return live;
   }
 
-  private LongCounter ended() {
-    LongCounter ended = this.ended;
-    if (ended == null) {
-      ended =
+  private LongCounter started() {
+    LongCounter started = this.started;
+    if (started == null) {
+      started =
           meter()
-              .counterBuilder("otel.sdk.span.ended")
+              .counterBuilder("otel.sdk.span.started")
               .setUnit("{span}")
-              .setDescription("The number of created spans for which the end operation was called")
+              .setDescription("The number of created spans")
               .build();
-      this.ended = ended;
+      this.started = started;
     }
-    return ended;
+    return started;
   }
 
   static Attributes getAttributesForSamplingDecisions(SamplingDecision decision) {
@@ -84,11 +85,35 @@ static Attributes getAttributesForSamplingDecisions(SamplingDecision decision) {
     throw new IllegalStateException("Unhandled SamplingDecision case: " + decision);
   }
 
+  // TODO: Add test verifying attribute values when released in semantic conventions
+  static String getParentOriginAttributeValue(SpanContext parentSpanContext) {
+    if (!parentSpanContext.isValid()) {
+      return "none";
+    } else if (parentSpanContext.isRemote()) {
+      return "remote";
+    } else {
+      return "local";
+    }
+  }
+
   @Override
-  public SpanInstrumentation.Recording recordSpanStart(SamplingResult samplingResult) {
-    Attributes attribs = getAttributesForSamplingDecisions(samplingResult.getDecision());
-    live().add(1, attribs);
-    return new Recording(attribs);
+  public SpanInstrumentation.Recording recordSpanStart(
+      SamplingResult samplingResult, SpanContext parentSpanContext) {
+    Attributes samplingResultAttribs =
+        getAttributesForSamplingDecisions(samplingResult.getDecision());
+    Attributes startAttributes =
+        samplingResultAttribs.toBuilder()
+            .put(
+                SemConvAttributes.OTEL_SPAN_PARENT_ORIGIN,
+                getParentOriginAttributeValue(parentSpanContext))
+            .build();
+    started().add(1L, startAttributes);
+    if (samplingResult.getDecision() == SamplingDecision.DROP) {
+      // Per semconv, otel.sdk.span.live is NOT collected for non-recording spans
+      return NoopSpanInstrumentation.RECORDING_INSTANCE;
+    }
+    live().add(1, samplingResultAttribs);
+    return new Recording(samplingResultAttribs);
   }
 
   private class Recording implements SpanInstrumentation.Recording {
@@ -102,14 +127,18 @@ private Recording(Attributes attributes) {
       this.attributes = attributes;
     }
 
+    @Override
+    public boolean isNoop() {
+      return false;
+    }
+
     @Override
     public synchronized void recordSpanEnd() {
       if (endAlreadyReported) {
         return;
       }
       endAlreadyReported = true;
       live().add(-1, attributes);
-      ended().add(1, attributes);
     }
   }
 }

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/SemConvSpanInstrumentation.java

@@ -6,6 +6,7 @@
 package io.opentelemetry.sdk.trace.internal;
 
 import io.opentelemetry.api.metrics.MeterProvider;
+import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.sdk.common.InternalTelemetryVersion;
 import io.opentelemetry.sdk.trace.samplers.SamplingResult;
 import java.util.function.Supplier;
@@ -29,14 +30,16 @@ static SpanInstrumentation create(
         "Unhandled telemetry schema version: " + internalTelemetryVersion);
   }
 
-  Recording recordSpanStart(SamplingResult samplingResult);
+  Recording recordSpanStart(SamplingResult samplingResult, SpanContext parentSpanContext);
 
   /**
    * This class is internal and is hence not for public use. Its APIs are unstable and can change at
    * any time.
    */
   interface Recording {
 
+    boolean isNoop();
+
     void recordSpanEnd();
   }
 }