Promote Span addLink to stable API (#6317)

open-telemetry · Apr 4, 2024 · d7370f3 · d7370f3
1 parent b416ace
commit d7370f3
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 78 deletions.
diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/Span.java b/api/all/src/main/java/io/opentelemetry/api/trace/Span.java
@@ -359,6 +359,47 @@ default Span recordException(Throwable exception) {
    */
   Span updateName(String name);
 
+  /**
+   * Adds a link to this {@code Span}.
+   *
+   * <p>Links are used to link {@link Span}s in different traces. Used (for example) in batching
+   * operations, where a single batch handler processes multiple requests from different traces or
+   * the same trace.
+   *
+   * <p>Implementations may ignore calls with an {@linkplain SpanContext#isValid() invalid span
+   * context}.
+   *
+   * <p>Callers should prefer to add links before starting the span via {@link
+   * SpanBuilder#addLink(SpanContext)} if possible.
+   *
+   * @param spanContext the context of the linked {@code Span}.
+   * @return this.
+   */
+  default Span addLink(SpanContext spanContext) {
+    return addLink(spanContext, Attributes.empty());
+  }
+
+  /**
+   * Adds a link to this {@code Span}.
+   *
+   * <p>Links are used to link {@link Span}s in different traces. Used (for example) in batching
+   * operations, where a single batch handler processes multiple requests from different traces or
+   * the same trace.
+   *
+   * <p>Implementations may ignore calls with an {@linkplain SpanContext#isValid() invalid span
+   * context}.
+   *
+   * <p>Callers should prefer to add links before starting the span via {@link
+   * SpanBuilder#addLink(SpanContext, Attributes)} if possible.
+   *
+   * @param spanContext the context of the linked {@code Span}.
+   * @param attributes the attributes of the {@code Link}.
+   * @return this.
+   */
+  default Span addLink(SpanContext spanContext, Attributes attributes) {
+    return this;
+  }
+
   /**
    * Marks the end of {@code Span} execution.
    *

diff --git a/api/all/src/test/java/io/opentelemetry/api/trace/DefaultTracerTest.java b/api/all/src/test/java/io/opentelemetry/api/trace/DefaultTracerTest.java
@@ -6,7 +6,9 @@
 package io.opentelemetry.api.trace;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
+import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.context.Context;
 import org.junit.jupiter.api.Test;
 
@@ -78,4 +80,14 @@ void testSpanContextPropagation_fromContextThenNoParent() {
     Span span = defaultTracer.spanBuilder(SPAN_NAME).setParent(context).setNoParent().startSpan();
     assertThat(span.getSpanContext()).isEqualTo(SpanContext.getInvalid());
   }
+
+  @Test
+  void addLink() {
+    Span span = Span.fromContext(Context.root());
+    assertThatCode(() -> span.addLink(null)).doesNotThrowAnyException();
+    assertThatCode(() -> span.addLink(SpanContext.getInvalid())).doesNotThrowAnyException();
+    assertThatCode(() -> span.addLink(null, null)).doesNotThrowAnyException();
+    assertThatCode(() -> span.addLink(SpanContext.getInvalid(), Attributes.empty()))
+        .doesNotThrowAnyException();
+  }
 }
diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/trace/ExtendedSpan.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/trace/ExtendedSpan.java
diff --git a/docs/apidiffs/current_vs_latest/opentelemetry-api.txt b/docs/apidiffs/current_vs_latest/opentelemetry-api.txt
@@ -1,2 +1,5 @@
 Comparing source compatibility of  against 
-No changes.
+***  MODIFIED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.api.trace.Span  (not serializable)
+	===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+	+++  NEW METHOD: PUBLIC(+) io.opentelemetry.api.trace.Span addLink(io.opentelemetry.api.trace.SpanContext)
+	+++  NEW METHOD: PUBLIC(+) io.opentelemetry.api.trace.Span addLink(io.opentelemetry.api.trace.SpanContext, io.opentelemetry.api.common.Attributes)
diff --git a/opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/DelegatingSpanTest.java b/opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/DelegatingSpanTest.java
@@ -11,6 +11,7 @@
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.api.trace.StatusCode;
 import io.opentelemetry.context.Context;
 import java.lang.reflect.InvocationTargetException;
@@ -120,7 +121,10 @@ static Stream<Arguments> delegateMethodsProvider() {
         // `true`
         Arguments.of("isRecording", new Class<?>[] {}, times(0)),
         // called twice: once in constructor, then once during delegation
-        Arguments.of("getSpanContext", new Class<?>[] {}, times(2)));
+        Arguments.of("getSpanContext", new Class<?>[] {}, times(2)),
+        // addLink is never called
+        Arguments.of("addLink", new Class<?>[] {SpanContext.class}, times(0)),
+        Arguments.of("addLink", new Class<?>[] {SpanContext.class, Attributes.class}, times(0)));
   }
 
   // gets default values for all cases, as mockito can't mock wrappers or primitives, including

diff --git a/sdk/trace/build.gradle.kts b/sdk/trace/build.gradle.kts
@@ -22,8 +22,6 @@ dependencies {
   api(project(":api:all"))
   api(project(":sdk:common"))
 
-  implementation(project(":api:incubator"))
-
   compileOnly(project(":sdk:trace-shaded-deps"))
 
   annotationProcessor("com.google.auto.value:auto-value")

diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java
@@ -7,7 +7,6 @@
 
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.incubator.trace.ExtendedSpan;
 import io.opentelemetry.api.internal.GuardedBy;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanContext;
@@ -36,7 +35,7 @@
 
 /** Implementation for the {@link Span} class that records trace events. */
 @ThreadSafe
-final class SdkSpan implements ReadWriteSpan, ExtendedSpan {
+final class SdkSpan implements ReadWriteSpan {
 
   private static final Logger logger = Logger.getLogger(SdkSpan.class.getName());
 

diff --git a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanTest.java b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanTest.java
@@ -15,7 +15,6 @@
 import static io.opentelemetry.api.common.AttributeKey.stringKey;
 import static java.util.stream.Collectors.joining;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.never;
@@ -25,7 +24,6 @@
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.common.AttributesBuilder;
-import io.opentelemetry.api.incubator.trace.ExtendedSpan;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.api.trace.SpanId;
@@ -821,10 +819,10 @@ void addLink() {
             null,
             null);
     try {
-      ExtendedSpan span1 = createTestSpan(SpanKind.INTERNAL);
-      ExtendedSpan span2 = createTestSpan(SpanKind.INTERNAL);
-      ExtendedSpan span3 = createTestSpan(SpanKind.INTERNAL);
-      ExtendedSpan span4 = createTestSpan(SpanKind.INTERNAL);
+      Span span1 = createTestSpan(SpanKind.INTERNAL);
+      Span span2 = createTestSpan(SpanKind.INTERNAL);
+      Span span3 = createTestSpan(SpanKind.INTERNAL);
+      Span span4 = createTestSpan(SpanKind.INTERNAL);
 
       span.addLink(span1.getSpanContext());
 
@@ -887,16 +885,6 @@ void addLink() {
     }
   }
 
-  @Test
-  void addLink_InvalidArgs() {
-    ExtendedSpan span = createTestSpan(SpanKind.INTERNAL);
-    assertThatCode(() -> span.addLink(null)).doesNotThrowAnyException();
-    assertThatCode(() -> span.addLink(SpanContext.getInvalid())).doesNotThrowAnyException();
-    assertThatCode(() -> span.addLink(null, null)).doesNotThrowAnyException();
-    assertThatCode(() -> span.addLink(SpanContext.getInvalid(), Attributes.empty()))
-        .doesNotThrowAnyException();
-  }
-
   @Test
   void droppingAttributes() {
     int maxNumberOfAttributes = 8;