Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions api/all/src/main/java/io/opentelemetry/api/trace/Span.java
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,47 @@ default Span recordException(Throwable exception) {
*/
Span updateName(String name);

/**
* Adds a link to the newly created {@code Span}.
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
*
* <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 the newly created {@code Span}.
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
*
* <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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
}
}
5 changes: 4 additions & 1 deletion docs/apidiffs/current_vs_latest/opentelemetry-api.txt
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,19 @@
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.SpanContext;
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;
Expand Down Expand Up @@ -80,72 +87,44 @@ 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
//
// called never -- it's shared between OC and Otel Span types and is always true, so returns
// `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
Expand Down
66 changes: 54 additions & 12 deletions sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@
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.
Expand Down Expand Up @@ -81,6 +76,16 @@
@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();
Expand All @@ -104,7 +109,7 @@
AnchoredClock clock,
Resource resource,
@Nullable AttributesMap attributes,
List<LinkData> links,
@Nullable List<LinkData> links,
int totalRecordedLinks,
long startEpochNanos) {
this.context = context;
Expand Down Expand Up @@ -151,7 +156,7 @@
Clock tracerClock,
Resource resource,
@Nullable AttributesMap attributes,
List<LinkData> links,
@Nullable List<LinkData> links,
int totalRecordedLinks,
long userStartEpochNanos) {
boolean createdAnchoredClock;
Expand Down Expand Up @@ -204,11 +209,12 @@
synchronized (lock) {
return SpanWrapper.create(
this,
links,
getImmutableLinks(),
getImmutableTimedEvents(),
getImmutableAttributes(),
(attributes == null) ? 0 : attributes.getTotalAddedValues(),
totalRecordedEvents,
totalRecordedLinks,
status,
name,
endEpochNanos,
Expand Down Expand Up @@ -426,6 +432,36 @@
return this;
}

@Override
public Span addLink(SpanContext spanContext, Attributes attributes) {
if (spanContext == null || !spanContext.isValid()) {
return this;

Check warning on line 438 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L438 was not covered by tests
}
if (attributes == null) {
attributes = Attributes.empty();

Check warning on line 441 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L441 was not covered by tests
}
synchronized (lock) {
if (hasEnded) {
logger.log(Level.FINE, "Calling addLink() on an ended Span.");
return this;

Check warning on line 446 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java

View check run for this annotation

Codecov / codecov/patch

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java#L445-L446

Added lines #L445 - L446 were not covered by tests
}
if (links == null) {
links = new ArrayList<>(spanLimits.getMaxNumberOfLinks());

Check warning on line 449 in sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L449 was not covered by tests
}
if (links.size() < spanLimits.getMaxNumberOfLinks()) {
links.add(
LinkData.create(
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
spanContext,
AttributeUtil.applyAttributesLimit(
attributes,
spanLimits.getMaxNumberOfAttributesPerLink(),
spanLimits.getMaxAttributeValueLength())));
}
totalRecordedLinks++;
}
return this;
}

@Override
public void end() {
endInternal(clock.now());
Expand Down Expand Up @@ -471,10 +507,6 @@
return startEpochNanos;
}

int getTotalRecordedLinks() {
return totalRecordedLinks;
}

@GuardedBy("lock")
private List<EventData> getImmutableTimedEvents() {
if (events.isEmpty()) {
Expand Down Expand Up @@ -504,19 +536,29 @@
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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -228,7 +229,7 @@ public Span startSpan() {
tracerSharedState.getClock(),
tracerSharedState.getResource(),
recordedAttributes,
immutableLinks,
currentLinks,
totalNumberOfLinksAdded,
startEpochNanos);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ abstract class SpanWrapper implements SpanData {

abstract int totalRecordedEvents();

abstract int totalRecordedLinks();

abstract StatusData status();

abstract String name();
Expand All @@ -62,6 +64,7 @@ static SpanWrapper create(
Attributes attributes,
int totalAttributeCount,
int totalRecordedEvents,
int totalRecordedLinks,
StatusData status,
String name,
long endEpochNanos,
Expand All @@ -73,6 +76,7 @@ static SpanWrapper create(
attributes,
totalAttributeCount,
totalRecordedEvents,
totalRecordedLinks,
status,
name,
endEpochNanos,
Expand Down Expand Up @@ -158,7 +162,7 @@ public int getTotalRecordedEvents() {

@Override
public int getTotalRecordedLinks() {
return delegate().getTotalRecordedLinks();
return totalRecordedLinks();
}

@Override
Expand Down
Loading