Add Span#addLink, for adding link after span start (#6084)

Co-authored-by: John Watson <jkwatson@gmail.com>

extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/trace/ExtendedSpan.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.extension.incubator.trace;
+
+import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.SpanBuilder;
+import io.opentelemetry.api.trace.SpanContext;
+
+/** Extended {@link Span} with experimental APIs. */
+public interface ExtendedSpan extends Span {
+
+  /**
+   * 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;
+  }
+}

opencensus-shim/src/test/java/io/opentelemetry/opencensusshim/DelegatingSpanTest.java

@@ -8,12 +8,18 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.times;
 
+import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.StatusCode;
+import io.opentelemetry.context.Context;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.time.Instant;
 import java.util.Arrays;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
@@ -80,64 +86,33 @@ static List<Method> allInterfaceMethods(Class<?> clazz) {
   static Stream<Arguments> delegateMethodsProvider() {
     return Stream.of(
         Arguments.of("end", new Class<?>[] {}, times(1)),
-        Arguments.of(
-            "end", new Class<?>[] {long.class, java.util.concurrent.TimeUnit.class}, times(1)),
-        Arguments.of("end", new Class<?>[] {java.time.Instant.class}, times(1)),
+        Arguments.of("end", new Class<?>[] {long.class, TimeUnit.class}, times(1)),
+        Arguments.of("end", new Class<?>[] {Instant.class}, times(1)),
         Arguments.of("setAttribute", new Class<?>[] {String.class, String.class}, times(1)),
-        Arguments.of(
-            "setAttribute",
-            new Class<?>[] {io.opentelemetry.api.common.AttributeKey.class, int.class},
-            times(1)),
-        Arguments.of(
-            "setAttribute",
-            new Class<?>[] {io.opentelemetry.api.common.AttributeKey.class, Object.class},
-            times(1)),
+        Arguments.of("setAttribute", new Class<?>[] {AttributeKey.class, int.class}, times(1)),
+        Arguments.of("setAttribute", new Class<?>[] {AttributeKey.class, Object.class}, times(1)),
         Arguments.of("setAttribute", new Class<?>[] {String.class, long.class}, times(1)),
         Arguments.of("setAttribute", new Class<?>[] {String.class, double.class}, times(1)),
         Arguments.of("setAttribute", new Class<?>[] {String.class, boolean.class}, times(1)),
         Arguments.of(
-            "recordException",
-            new Class<?>[] {Throwable.class, io.opentelemetry.api.common.Attributes.class},
-            times(1)),
+            "recordException", new Class<?>[] {Throwable.class, Attributes.class}, times(1)),
         Arguments.of("recordException", new Class<?>[] {Throwable.class}, times(1)),
-        Arguments.of(
-            "setAllAttributes",
-            new Class<?>[] {io.opentelemetry.api.common.Attributes.class},
-            times(1)),
+        Arguments.of("setAllAttributes", new Class<?>[] {Attributes.class}, times(1)),
         Arguments.of("updateName", new Class<?>[] {String.class}, times(1)),
+        Arguments.of("storeInContext", new Class<?>[] {Context.class}, times(1)),
+        Arguments.of("addEvent", new Class<?>[] {String.class, Instant.class}, times(1)),
         Arguments.of(
-            "storeInContext", new Class<?>[] {io.opentelemetry.context.Context.class}, times(1)),
-        Arguments.of("addEvent", new Class<?>[] {String.class, java.time.Instant.class}, times(1)),
+            "addEvent", new Class<?>[] {String.class, long.class, TimeUnit.class}, times(1)),
         Arguments.of(
-            "addEvent",
-            new Class<?>[] {String.class, long.class, java.util.concurrent.TimeUnit.class},
-            times(1)),
-        Arguments.of(
-            "addEvent",
-            new Class<?>[] {
-              String.class, io.opentelemetry.api.common.Attributes.class, java.time.Instant.class
-            },
-            times(1)),
+            "addEvent", new Class<?>[] {String.class, Attributes.class, Instant.class}, times(1)),
         Arguments.of("addEvent", new Class<?>[] {String.class}, times(1)),
         Arguments.of(
             "addEvent",
-            new Class<?>[] {
-              String.class,
-              io.opentelemetry.api.common.Attributes.class,
-              long.class,
-              java.util.concurrent.TimeUnit.class
-            },
+            new Class<?>[] {String.class, Attributes.class, long.class, TimeUnit.class},
             times(1)),
-        Arguments.of(
-            "addEvent",
-            new Class<?>[] {String.class, io.opentelemetry.api.common.Attributes.class},
-            times(1)),
-        Arguments.of(
-            "setStatus",
-            new Class<?>[] {io.opentelemetry.api.trace.StatusCode.class, String.class},
-            times(1)),
-        Arguments.of(
-            "setStatus", new Class<?>[] {io.opentelemetry.api.trace.StatusCode.class}, times(1)),
+        Arguments.of("addEvent", new Class<?>[] {String.class, Attributes.class}, times(1)),
+        Arguments.of("setStatus", new Class<?>[] {StatusCode.class, String.class}, times(1)),
+        Arguments.of("setStatus", new Class<?>[] {StatusCode.class}, times(1)),
         //
         // special cases
         //

sdk/trace/build.gradle.kts

@@ -22,6 +22,8 @@ dependencies {
   api(project(":api:all"))
   api(project(":sdk:common"))
 
+  implementation(project(":extensions:incubator"))
+
   compileOnly(project(":sdk:trace-shaded-deps"))
 
   annotationProcessor("com.google.auto.value:auto-value")

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

@@ -13,6 +13,7 @@
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.StatusCode;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.extension.incubator.trace.ExtendedSpan;
 import io.opentelemetry.sdk.common.Clock;
 import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
 import io.opentelemetry.sdk.internal.AttributeUtil;
@@ -35,7 +36,7 @@
 
 /** Implementation for the {@link Span} class that records trace events. */
 @ThreadSafe
-final class SdkSpan implements ReadWriteSpan {
+final class SdkSpan implements ReadWriteSpan, ExtendedSpan {
 
   private static final Logger logger = Logger.getLogger(SdkSpan.class.getName());
 
@@ -47,11 +48,6 @@ final class SdkSpan implements ReadWriteSpan {
   private final SpanContext parentSpanContext;
   // Handler called when the span starts and ends.
   private final SpanProcessor spanProcessor;
-  // The displayed name of the span.
-  // List of recorded links to parent and child spans.
-  private final List<LinkData> links;
-  // Number of links recorded.
-  private final int totalRecordedLinks;
   // The kind of the span.
   private final SpanKind kind;
   // The clock used to get the time.
@@ -81,6 +77,16 @@ final class SdkSpan implements ReadWriteSpan {
   @GuardedBy("lock")
   private int totalRecordedEvents = 0;
 
+  // The displayed name of the span.
+  // List of recorded links to parent and child spans.
+  @GuardedBy("lock")
+  @Nullable
+  List<LinkData> links;
+
+  // Number of links recorded.
+  @GuardedBy("lock")
+  private int totalRecordedLinks;
+
   // The status of the span.
   @GuardedBy("lock")
   private StatusData status = StatusData.unset();
@@ -104,7 +110,7 @@ private SdkSpan(
       AnchoredClock clock,
       Resource resource,
       @Nullable AttributesMap attributes,
-      List<LinkData> links,
+      @Nullable List<LinkData> links,
       int totalRecordedLinks,
       long startEpochNanos) {
     this.context = context;
@@ -151,7 +157,7 @@ static SdkSpan startSpan(
       Clock tracerClock,
       Resource resource,
       @Nullable AttributesMap attributes,
-      List<LinkData> links,
+      @Nullable List<LinkData> links,
       int totalRecordedLinks,
       long userStartEpochNanos) {
     boolean createdAnchoredClock;
@@ -206,11 +212,12 @@ public SpanData toSpanData() {
     synchronized (lock) {
       return SpanWrapper.create(
           this,
-          links,
+          getImmutableLinks(),
           getImmutableTimedEvents(),
           getImmutableAttributes(),
           (attributes == null) ? 0 : attributes.getTotalAddedValues(),
           totalRecordedEvents,
+          totalRecordedLinks,
           status,
           name,
           endEpochNanos,
@@ -428,6 +435,37 @@ public ReadWriteSpan updateName(String name) {
     return this;
   }
 
+  @Override
+  public Span addLink(SpanContext spanContext, Attributes attributes) {
+    if (spanContext == null || !spanContext.isValid()) {
+      return this;
+    }
+    if (attributes == null) {
+      attributes = Attributes.empty();
+    }
+    LinkData link =
+        LinkData.create(
+            spanContext,
+            AttributeUtil.applyAttributesLimit(
+                attributes,
+                spanLimits.getMaxNumberOfAttributesPerLink(),
+                spanLimits.getMaxAttributeValueLength()));
+    synchronized (lock) {
+      if (hasEnded) {
+        logger.log(Level.FINE, "Calling addLink() on an ended Span.");
+        return this;
+      }
+      if (links == null) {
+        links = new ArrayList<>(spanLimits.getMaxNumberOfLinks());
+      }
+      if (links.size() < spanLimits.getMaxNumberOfLinks()) {
+        links.add(link);
+      }
+      totalRecordedLinks++;
+    }
+    return this;
+  }
+
   @Override
   public void end() {
     endInternal(clock.now());
@@ -475,10 +513,6 @@ long getStartEpochNanos() {
     return startEpochNanos;
   }
 
-  int getTotalRecordedLinks() {
-    return totalRecordedLinks;
-  }
-
   @GuardedBy("lock")
   private List<EventData> getImmutableTimedEvents() {
     if (events.isEmpty()) {
@@ -508,19 +542,29 @@ private Attributes getImmutableAttributes() {
     return attributes.immutableCopy();
   }
 
+  @GuardedBy("lock")
+  private List<LinkData> getImmutableLinks() {
+    if (links == null || links.isEmpty()) {
+      return Collections.emptyList();
+    }
+    return Collections.unmodifiableList(links);
+  }
+
   @Override
   public String toString() {
     String name;
     String attributes;
     String status;
     long totalRecordedEvents;
     long endEpochNanos;
+    long totalRecordedLinks;
     synchronized (lock) {
       name = this.name;
       attributes = String.valueOf(this.attributes);
       status = String.valueOf(this.status);
       totalRecordedEvents = this.totalRecordedEvents;
       endEpochNanos = this.endEpochNanos;
+      totalRecordedLinks = this.totalRecordedLinks;
     }
     return "SdkSpan{traceId="
         + context.getTraceId()

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

@@ -179,8 +179,9 @@ public Span startSpan() {
       // New child span.
       traceId = parentSpanContext.getTraceId();
     }
+    List<LinkData> currentLinks = links;
     List<LinkData> immutableLinks =
-        links == null ? Collections.emptyList() : Collections.unmodifiableList(links);
+        currentLinks == null ? Collections.emptyList() : Collections.unmodifiableList(currentLinks);
     // Avoid any possibility to modify the links list by adding links to the Builder after the
     // startSpan is called. If that happens all the links will be added in a new list.
     links = null;
@@ -228,7 +229,7 @@ public Span startSpan() {
         tracerSharedState.getClock(),
         tracerSharedState.getResource(),
         recordedAttributes,
-        immutableLinks,
+        currentLinks,
         totalNumberOfLinksAdded,
         startEpochNanos);
   }

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

@@ -43,6 +43,8 @@ abstract class SpanWrapper implements SpanData {
 
   abstract int totalRecordedEvents();
 
+  abstract int totalRecordedLinks();
+
   abstract StatusData status();
 
   abstract String name();
@@ -62,6 +64,7 @@ static SpanWrapper create(
       Attributes attributes,
       int totalAttributeCount,
       int totalRecordedEvents,
+      int totalRecordedLinks,
       StatusData status,
       String name,
       long endEpochNanos,
@@ -73,6 +76,7 @@ static SpanWrapper create(
         attributes,
         totalAttributeCount,
         totalRecordedEvents,
+        totalRecordedLinks,
         status,
         name,
         endEpochNanos,
@@ -158,7 +162,7 @@ public int getTotalRecordedEvents() {
 
   @Override
   public int getTotalRecordedLinks() {
-    return delegate().getTotalRecordedLinks();
+    return totalRecordedLinks();
   }
 
   @Override