open-telemetry
/
opentelemetry-java
mirror of https://github.com/open-telemetry/opentelemetry-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove addLink that accepts Link interface (#1732) 

* Remove addLink that accepts Link interface

Currently the Link interface cannot easily be removed, used in the Sampler interface,
will do a separate PR for that.

Based on the specs we don't have to offer a "lazy" formatted Link API (was removed some time ago).

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

* Copy javadoc instead of reference

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

Co-authored-by: Anuraag Agrawal <aanuraag@amazon.co.jp>
This commit is contained in:
Bogdan Drutu 2020-10-01 18:24:02 -07:00 committed by GitHub
parent b78fbb31fa
commit 790018f004
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 26 additions and 131 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
api/src/main/java/io/opentelemetry/trace/DefaultTracer.java
View File

@ -102,11 +102,6 @@ public final class DefaultTracer implements Tracer {
return this;
}
@Override
public NoopSpanBuilder addLink(Link link) {
return this;
}
@Override
public NoopSpanBuilder setAttribute(String key, String value) {
Utils.checkNotNull(key, "key");

26
api/src/main/java/io/opentelemetry/trace/Span.java
View File

@ -421,27 +421,17 @@ public interface Span {
/**
* Adds a {@link Link} to the newly created {@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.
*
* @param spanContext the context of the linked {@code Span}.
* @return this.
* @throws NullPointerException if {@code spanContext} is {@code null}.
* @see #addLink(Link)
* @since 0.1.0
*/
Builder addLink(SpanContext spanContext);
/**
* Adds a {@link Link} to the newly created {@code Span}.
*
* @param spanContext the context of the linked {@code Span}.
* @param attributes the attributes of the {@code Link}.
* @return this.
* @throws NullPointerException if {@code spanContext} is {@code null}.
* @throws NullPointerException if {@code attributes} is {@code null}.
* @see #addLink(Link)
* @since 0.1.0
*/
Builder addLink(SpanContext spanContext, Attributes attributes);
/**
* Adds a {@link Link} to the newly created {@code Span}.
*
@ -449,12 +439,14 @@ public interface Span {
* operations, where a single batch handler processes multiple requests from different traces or
* the same trace.
*
* @param link the {@link Link} to be added.
* @param spanContext the context of the linked {@code Span}.
* @param attributes the attributes of the {@code Link}.
* @return this.
* @throws NullPointerException if {@code link} is {@code null}.
* @throws NullPointerException if {@code spanContext} is {@code null}.
* @throws NullPointerException if {@code attributes} is {@code null}.
* @since 0.1.0
*/
Builder addLink(Link link);
Builder addLink(SpanContext spanContext, Attributes attributes);
/**
* Sets an attribute to the newly created {@code Span}. If {@code Span.Builder} previously

14
api/src/test/java/io/opentelemetry/trace/SpanBuilderTest.java
View File

@ -38,20 +38,6 @@ class SpanBuilderTest {
spanBuilder.setNoParent();
spanBuilder.addLink(DefaultSpan.getInvalid().getContext());
spanBuilder.addLink(DefaultSpan.getInvalid().getContext(), Attributes.empty());
spanBuilder.addLink(
new Link() {
private final SpanContext spanContext = DefaultSpan.getInvalid().getContext();
@Override
public SpanContext getContext() {
return spanContext;
}
@Override
public Attributes getAttributes() {
return Attributes.empty();
}
});
spanBuilder.setAttribute("key", "value");
spanBuilder.setAttribute("key", 12345L);
spanBuilder.setAttribute("key", .12345);

16
sdk/all/src/jmh/java/io/opentelemetry/sdk/trace/SpanPipelineBenchmark.java
View File

@ -28,10 +28,8 @@ import io.opentelemetry.sdk.common.CompletableResultCode;
import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import io.opentelemetry.trace.Link;
import io.opentelemetry.trace.Span;
import io.opentelemetry.trace.Span.Kind;
import io.opentelemetry.trace.SpanContext;
import io.opentelemetry.trace.Status;
import java.util.Collection;
import java.util.concurrent.TimeUnit;
@ -49,7 +47,6 @@ import org.openjdk.jmh.annotations.Warmup;
@State(Scope.Benchmark)
public class SpanPipelineBenchmark {
private static final AttributeKey<String> LINK_ATTRIBUTE_KEY = stringKey("linkAttr");
private static final AttributeKey<String> OPERATION_KEY = stringKey("operation");
private static final AttributeKey<Long> LONG_ATTRIBUTE_KEY = longKey("longAttribute");
private static final AttributeKey<String> STRING_ATTRIBUTE_KEY = stringKey("stringAttribute");
@ -80,7 +77,6 @@ public class SpanPipelineBenchmark {
.spanBuilder("benchmarkSpan")
.setSpanKind(Kind.CLIENT)
.setAttribute("key", "value")
.addLink(new TestLink())
.startSpan();
span.addEvent("started", Attributes.of(OPERATION_KEY, "some_work"));
span.setAttribute(LONG_ATTRIBUTE_KEY, 33L);
@ -110,16 +106,4 @@ public class SpanPipelineBenchmark {
return CompletableResultCode.ofSuccess();
}
}
private static class TestLink implements Link {
@Override
public SpanContext getContext() {
return SpanContext.getInvalid();
}
@Override
public Attributes getAttributes() {
return Attributes.of(LINK_ATTRIBUTE_KEY, "linkValue");
}
}
}

30
sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java
View File

@ -70,7 +70,7 @@ final class RecordEventsReadableSpan implements ReadWriteSpan {
private final SpanProcessor spanProcessor;
// The displayed name of the span.
// List of recorded links to parent and child spans.
private final List<io.opentelemetry.trace.Link> links;
private final List<Link> links;
// Number of links recorded.
private final int totalRecordedLinks;
@ -122,7 +122,7 @@ final class RecordEventsReadableSpan implements ReadWriteSpan {
Clock clock,
Resource resource,
@Nullable AttributesMap attributes,
List<io.opentelemetry.trace.Link> links,
List<Link> links,
int totalRecordedLinks,
long startEpochNanos) {
this.context = context;
@ -158,7 +158,7 @@ final class RecordEventsReadableSpan implements ReadWriteSpan {
* @param clock the clock used to get the time.
* @param resource the resource associated with this span.
* @param attributes the attributes set during span creation.
* @param links the links set during span creation, may be truncated.
* @param links the links set during span creation, may be truncated. The list MUST be immutable.
* @return a new and started span.
*/
static RecordEventsReadableSpan startSpan(
@ -173,7 +173,7 @@ final class RecordEventsReadableSpan implements ReadWriteSpan {
Clock clock,
Resource resource,
AttributesMap attributes,
List<io.opentelemetry.trace.Link> links,
List<Link> links,
int totalRecordedLinks,
long startEpochNanos) {
RecordEventsReadableSpan span =
@ -204,7 +204,7 @@ final class RecordEventsReadableSpan implements ReadWriteSpan {
synchronized (lock) {
return SpanWrapper.create(
this,
getImmutableLinks(),
links,
getImmutableTimedEvents(),
getImmutableAttributes(),
(attributes == null) ? 0 : attributes.getTotalAddedValues(),
@ -517,26 +517,6 @@ final class RecordEventsReadableSpan implements ReadWriteSpan {
return totalRecordedLinks;
}
@GuardedBy("lock")
private List<Link> getImmutableLinks() {
if (links.isEmpty()) {
return Collections.emptyList();
}
List<Link> result = new ArrayList<>(links.size());
for (io.opentelemetry.trace.Link link : links) {
Link newLink;
if (!(link instanceof Link)) {
// Make a copy because the given Link may not be immutable and we may reference a lot of
// memory.
newLink = Link.create(link.getContext(), link.getAttributes());
} else {
newLink = (Link) link;
}
result.add(newLink);
}
return Collections.unmodifiableList(result);
}
@GuardedBy("lock")
private List<Event> getImmutableTimedEvents() {
if (events.isEmpty()) {

19
sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java
View File

@ -61,7 +61,7 @@ final class SpanBuilderSdk implements Span.Builder {
@Nullable private Context parent;
private Kind spanKind = Kind.INTERNAL;
@Nullable private AttributesMap attributes;
@Nullable private List<io.opentelemetry.trace.Link> links;
@Nullable private List<Link> links;
private int totalNumberOfLinksAdded = 0;
private long startEpochNanos = 0;
private boolean isRootSpan;
@ -122,8 +122,7 @@ final class SpanBuilderSdk implements Span.Builder {
return this;
}
@Override
public Span.Builder addLink(io.opentelemetry.trace.Link link) {
private void addLink(Link link) {
Objects.requireNonNull(link, "link");
totalNumberOfLinksAdded++;
if (links == null) {
@ -132,11 +131,10 @@ final class SpanBuilderSdk implements Span.Builder {
// don't bother doing anything with any links beyond the max.
if (links.size() == traceConfig.getMaxNumberOfLinks()) {
return this;
return;
}
links.add(link);
return this;
}
@Override
@ -201,10 +199,7 @@ final class SpanBuilderSdk implements Span.Builder {
traceId = parentSpanContext.getTraceIdAsHexString();
traceState = parentSpanContext.getTraceState();
}
List<io.opentelemetry.trace.Link> immutableLinks =
links == null
? Collections.<io.opentelemetry.trace.Link>emptyList()
: Collections.unmodifiableList(links);
List<Link> oldLinks = links;
// 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;
@ -218,7 +213,9 @@ final class SpanBuilderSdk implements Span.Builder {
spanName,
spanKind,
immutableAttributes,
immutableLinks);
oldLinks == null
? Collections.emptyList()
: Collections.unmodifiableList(oldLinks));
Sampler.Decision samplingDecision = samplingResult.getDecision();
SpanContext spanContext =
@ -258,7 +255,7 @@ final class SpanBuilderSdk implements Span.Builder {
getClock(parentSpan, clock),
resource,
recordedAttributes,
immutableLinks,
oldLinks == null ? Collections.emptyList() : Collections.unmodifiableList(oldLinks),
totalNumberOfLinksAdded,
startEpochNanos);
}

33
sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java
View File

@ -85,7 +85,7 @@ class RecordEventsReadableSpanTest {
InstrumentationLibraryInfo.create("theName", null);
private final Map<AttributeKey, Object> attributes = new HashMap<>();
private Attributes expectedAttributes;
private final io.opentelemetry.trace.Link link = Link.create(spanContext);
private final Link link = Link.create(spanContext);
@Mock private SpanProcessor spanProcessor;
private TestClock testClock;
@ -126,35 +126,6 @@ class RecordEventsReadableSpanTest {
/*hasEnded=*/ true);
}
@Test
void lazyLinksAreResolved() {
final Attributes attributes = Attributes.of(stringKey("attr"), "val");
io.opentelemetry.trace.Link link =
new io.opentelemetry.trace.Link() {
@Override
public SpanContext getContext() {
return spanContext;
}
@Override
public Attributes getAttributes() {
return attributes;
}
};
RecordEventsReadableSpan span =
createTestSpan(
Kind.CLIENT,
TraceConfig.getDefault(),
parentSpanId,
null,
Collections.singletonList(link));
Link resultingLink = span.toSpanData().getLinks().get(0);
assertThat(resultingLink.getTotalAttributeCount()).isEqualTo(1);
assertThat(resultingLink.getContext()).isSameAs(spanContext);
assertThat(resultingLink.getAttributes()).isEqualTo(attributes);
}
@Test
void endSpanTwice_DoNotCrash() {
RecordEventsReadableSpan span = createTestSpan(Kind.INTERNAL);
@ -778,7 +749,7 @@ class RecordEventsReadableSpanTest {
TraceConfig config,
@Nullable String parentSpanId,
@Nullable AttributesMap attributes,
List<io.opentelemetry.trace.Link> links) {
List<Link> links) {
RecordEventsReadableSpan span =
RecordEventsReadableSpan.startSpan(

14
sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java
View File

@ -81,13 +81,12 @@ class SpanBuilderSdkTest {
void addLink() {
// Verify methods do not crash.
Span.Builder spanBuilder = tracerSdk.spanBuilder(SPAN_NAME);
spanBuilder.addLink(Link.create(DefaultSpan.getInvalid().getContext()));
spanBuilder.addLink(DefaultSpan.getInvalid().getContext());
spanBuilder.addLink(DefaultSpan.getInvalid().getContext(), Attributes.empty());
RecordEventsReadableSpan span = (RecordEventsReadableSpan) spanBuilder.startSpan();
try {
assertThat(span.toSpanData().getLinks()).hasSize(3);
assertThat(span.toSpanData().getLinks()).hasSize(2);
} finally {
span.end();
}
@ -173,18 +172,9 @@ class SpanBuilderSdkTest {
}
}
@Test
void addLink_null() {
assertThrows(
NullPointerException.class,
() -> tracerSdk.spanBuilder(SPAN_NAME).addLink((io.opentelemetry.trace.Link) null));
}
@Test
void addLinkSpanContext_null() {
assertThrows(
NullPointerException.class,
() -> tracerSdk.spanBuilder(SPAN_NAME).addLink((SpanContext) null));
assertThrows(NullPointerException.class, () -> tracerSdk.spanBuilder(SPAN_NAME).addLink(null));
}
@Test