Skip to content

Commit

Permalink
OtelSpanBuilder now uses SpanBuilder properly
Browse files Browse the repository at this point in the history
without this change we would first create a SpanBuilder with an empty name and then update the name on the span. There are cases that depending on the name of the span OTel can already interact with the SpanBuilder and alter its behaviour.

with this change SpanBuilder is created on the start() method where it should already contain all the necessary information including the proper name. We set the name on the builder instead of the span.

fixes gh-259
  • Loading branch information
marcingrzejszczak committed Aug 18, 2023
1 parent 3c5845c commit b5204aa
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public String get(C carrier, String key) {
});
io.opentelemetry.api.trace.Span span = io.opentelemetry.api.trace.Span.fromContextOrNull(extracted);
OtelTraceContext otelTraceContext = getOtelTraceContext(extracted, span);
return OtelSpanBuilder.fromOtel(this.tracer.spanBuilder("")).setParent(otelTraceContext);
return OtelSpanBuilder.fromOtel(this.tracer).setParent(otelTraceContext);
}

private static OtelTraceContext getOtelTraceContext(Context extracted, io.opentelemetry.api.trace.Span span) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
import io.micrometer.common.util.StringUtils;
import io.micrometer.tracing.Span;
import io.micrometer.tracing.TraceContext;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.api.trace.SpanBuilder;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;

import java.util.LinkedList;
Expand All @@ -35,34 +39,43 @@ class OtelSpanBuilder implements Span.Builder {

static final String REMOTE_SERVICE_NAME_KEY = "peer.service";

private final io.opentelemetry.api.trace.SpanBuilder delegate;
private final Tracer tracer;

private final List<String> annotations = new LinkedList<>();

private final AttributesBuilder attributes = Attributes.builder();

private String name;

private Throwable error;

private TraceContext parentTraceContext;

OtelSpanBuilder(io.opentelemetry.api.trace.SpanBuilder delegate) {
this.delegate = delegate;
private boolean noParent;

private SpanKind spanKind = SpanKind.INTERNAL;

private long startTimestamp;

private TimeUnit startTimestampUnit;

OtelSpanBuilder(Tracer tracer) {
this.tracer = tracer;
}

static Span.Builder fromOtel(io.opentelemetry.api.trace.SpanBuilder builder) {
return new OtelSpanBuilder(builder);
static Span.Builder fromOtel(Tracer tracer) {
return new OtelSpanBuilder(tracer);
}

@Override
public Span.Builder setParent(TraceContext context) {
this.delegate.setParent(OtelTraceContext.toOtelContext(context));
this.parentTraceContext = context;
return this;
}

@Override
public Span.Builder setNoParent() {
this.delegate.setNoParent();
this.noParent = true;
return this;
}

Expand All @@ -80,7 +93,7 @@ public Span.Builder event(String value) {

@Override
public Span.Builder tag(String key, String value) {
this.delegate.setAttribute(key, value);
this.attributes.put(key, value);
return this;
}

Expand All @@ -93,7 +106,7 @@ public Span.Builder error(Throwable throwable) {
@Override
public Span.Builder kind(Span.Kind spanKind) {
if (spanKind == null) {
this.delegate.setSpanKind(SpanKind.INTERNAL);
this.spanKind = SpanKind.INTERNAL;
return this;
}
SpanKind kind = SpanKind.INTERNAL;
Expand All @@ -111,37 +124,47 @@ public Span.Builder kind(Span.Kind spanKind) {
kind = SpanKind.CONSUMER;
break;
}
this.delegate.setSpanKind(kind);
this.spanKind = kind;
return this;
}

@Override
public Span.Builder remoteServiceName(String remoteServiceName) {
this.delegate.setAttribute(REMOTE_SERVICE_NAME_KEY, remoteServiceName);
this.attributes.put(REMOTE_SERVICE_NAME_KEY, remoteServiceName);
return this;
}

@Override
public Span.Builder remoteIpAndPort(String ip, int port) {
this.delegate.setAttribute(SemanticAttributes.NET_SOCK_PEER_ADDR, ip);
this.delegate.setAttribute(SemanticAttributes.NET_PEER_PORT, (long) port);
this.attributes.put(SemanticAttributes.NET_SOCK_PEER_ADDR.getKey(), ip);
this.attributes.put(SemanticAttributes.NET_PEER_PORT.getKey(), port);
return this;
}

@Override
public Span.Builder startTimestamp(long startTimestamp, TimeUnit unit) {
this.delegate.setStartTimestamp(startTimestamp, unit);
this.startTimestamp = startTimestamp;
this.startTimestampUnit = unit;
return this;
}

@Override
public Span start() {
io.opentelemetry.api.trace.Span span = this.delegate.startSpan();
if (StringUtils.isNotEmpty(this.name)) {
span.updateName(this.name);
SpanBuilder spanBuilder = this.tracer.spanBuilder(StringUtils.isNotEmpty(this.name) ? this.name : "");
if (this.parentTraceContext != null) {
spanBuilder.setParent(OtelTraceContext.toOtelContext(this.parentTraceContext));
}
if (this.noParent) {
spanBuilder.setNoParent();
}
spanBuilder.setAllAttributes(this.attributes.build());
spanBuilder.setSpanKind(this.spanKind);
if (this.startTimestampUnit != null) {
spanBuilder.setStartTimestamp(this.startTimestamp, this.startTimestampUnit);
}
io.opentelemetry.api.trace.Span span = spanBuilder.startSpan();
if (this.error != null) {
span.recordException(error);
span.recordException(this.error);
}
this.annotations.forEach(span::addEvent);
Span otelSpan = OtelSpan.fromOtel(span);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public ScopedSpan startScopedSpan(String name) {

@Override
public Span.Builder spanBuilder() {
return new OtelSpanBuilder(this.tracer.spanBuilder(""));
return new OtelSpanBuilder(this.tracer);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ void should_set_child_span_when_using_builders() {
.build();
io.opentelemetry.api.trace.Tracer otelTracer = openTelemetrySdk.getTracer("io.micrometer.micrometer-tracing");

Span.Builder builder = new OtelSpanBuilder(otelTracer.spanBuilder("foo"));
Span.Builder builder = new OtelSpanBuilder(otelTracer).name("foo");
Span parentSpan = OtelSpan.fromOtel(otelTracer.spanBuilder("bar").startSpan());

Span child = builder.setParent(parentSpan.context()).start();
Expand Down

0 comments on commit b5204aa

Please sign in to comment.