Skip to content

Commit

Permalink
Introduce LocalRootSpan (replacing ServerSpan) (#5896)
Browse files Browse the repository at this point in the history
* Introduce LocalRootSpan (replacing ServerSpan)

* fix tests

* bridge old ServerSpan class
  • Loading branch information
Mateusz Rzeszutek authored Apr 21, 2022
1 parent 0c1afee commit 2c4acd7
Show file tree
Hide file tree
Showing 24 changed files with 299 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import io.opentelemetry.context.ContextKey;
import io.opentelemetry.instrumentation.api.instrumenter.ContextCustomizer;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.server.ServerSpan;
import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -101,7 +101,7 @@ public static <T, U> void updateHttpRoute(
HttpRouteBiGetter<T, U> httpRouteGetter,
T arg1,
U arg2) {
Span serverSpan = ServerSpan.fromContextOrNull(context);
Span serverSpan = LocalRootSpan.fromContextOrNull(context);
// even if the server span is not sampled, we have to continue - we need to compute the
// http.route properly so that it can be captured by the server metrics
if (serverSpan == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.internal.SpanKey;
import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
import javax.annotation.Nullable;

/**
* This class encapsulates the context key for storing the current {@link SpanKind#SERVER} span in
* the {@link Context}.
*
* @deprecated Use {@link LocalRootSpan} instead.
*/
@Deprecated
public final class ServerSpan {

/**
Expand All @@ -23,13 +26,7 @@ public final class ServerSpan {
*/
@Nullable
public static Span fromContextOrNull(Context context) {
// try the HTTP semconv span first
Span span = SpanKey.HTTP_SERVER.fromContextOrNull(context);
// if it's absent then try the SERVER one - perhaps suppression by span kind is enabled
if (span == null) {
span = SpanKey.KIND_SERVER.fromContextOrNull(context);
}
return span;
return LocalRootSpan.fromContextOrNull(context);
}

private ServerSpan() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,31 +8,24 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanContext;
import io.opentelemetry.api.trace.TraceFlags;
import io.opentelemetry.api.trace.TraceState;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.internal.SpanKey;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

class HttpRouteHolderTest {

@RegisterExtension final OpenTelemetryExtension testing = OpenTelemetryExtension.create();

@Test
void shouldSetRouteEvenIfSpanIsNotSampled() {
Span span =
Span.wrap(
SpanContext.create(
"00000000000000000000000000000042",
"0000000000000012",
TraceFlags.getDefault(),
TraceState.getDefault()));

Context context = Context.root();
context = context.with(span);
context = SpanKey.HTTP_SERVER.storeInContext(context, span);
context = HttpRouteHolder.get().start(context, null, Attributes.empty());
Instrumenter<String, Void> instrumenter =
Instrumenter.<String, Void>builder(testing.getOpenTelemetry(), "test", s -> s)
.addContextCustomizer(HttpRouteHolder.get())
.newInstrumenter();

Context context = instrumenter.start(Context.root(), "test");

assertNull(HttpRouteHolder.getRoute(context));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ public Context start(Context parentContext, REQUEST request) {
}
}

if (LocalRootSpan.isLocalRoot(parentContext)) {
context = LocalRootSpan.store(context, span);
}

return spanSuppressor.storeInContext(context, spanKind, span);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.instrumenter;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanContext;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.ContextKey;
import javax.annotation.Nullable;

/**
* A local root span is a span that either does not have a parent span (it is the root span of a
* trace), or its parent span is a remote span (context was propagated from another application).
*/
public final class LocalRootSpan {

private static final ContextKey<Span> KEY =
ContextKey.named("opentelemetry-traces-local-root-span");

/**
* Returns the local root span from the current context or {@linkplain Span#getInvalid() invalid
* span} if there is no local root span.
*/
public static Span current() {
return fromContext(Context.current());
}

/**
* Returns the local root span from the given context or {@linkplain Span#getInvalid() invalid
* span} if there is no local root span in the context.
*/
public static Span fromContext(Context context) {
Span span = fromContextOrNull(context);
return span == null ? Span.getInvalid() : span;
}

/**
* Returns the local root span from the given context or {@code null} if there is no local root
* span in the context.
*/
@Nullable
public static Span fromContextOrNull(Context context) {
return context.get(KEY);
}

static boolean isLocalRoot(Context parentContext) {
SpanContext spanContext = Span.fromContext(parentContext).getSpanContext();
return !spanContext.isValid() || spanContext.isRemote();
}

static Context store(Context context, Span span) {
return context.with(KEY, span);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import io.opentelemetry.instrumentation.api.config.Config;
import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.BySpanKey;
import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.DelegateBySpanKind;
import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.JustStoreServer;
import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.Noop;
import io.opentelemetry.instrumentation.api.internal.SpanKey;
import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
Expand All @@ -21,16 +20,11 @@
import java.util.Set;

enum SpanSuppressionStrategy {
/**
* Do not suppress spans at all.
*
* <p>This strategy will mark spans of {@link SpanKind#SERVER SERVER} kind in the context, so that
* they can be accessed using {@code ServerSpan}, but will not suppress server spans.
*/
/** Do not suppress spans at all. */
NONE {
@Override
SpanSuppressor create(Set<SpanKey> spanKeys) {
return JustStoreServer.INSTANCE;
return Noop.INSTANCE;
}
},
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ void server_parent() {

Context context = instrumenter.start(Context.root(), request);
assertThat(Span.fromContext(context).getSpanContext().isValid()).isTrue();
assertThat(LocalRootSpan.fromContext(context)).isSameAs(Span.fromContext(context));

instrumenter.end(context, request, RESPONSE, null);

Expand Down Expand Up @@ -358,6 +359,7 @@ void client_parent() {

assertThat(spanContext.isValid()).isTrue();
assertThat(request).containsKey("traceparent");
assertThat(LocalRootSpan.fromContextOrNull(context)).isNull();

instrumenter.end(context, request, RESPONSE, new IllegalStateException("test"));

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.instrumenter;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanContext;
import io.opentelemetry.api.trace.TraceFlags;
import io.opentelemetry.api.trace.TraceState;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import org.junit.jupiter.api.Test;

class LocalRootSpanTest {

@Test
void spanWithoutParentIsLocalRoot() {
Context parentContext = Context.root();
assertTrue(LocalRootSpan.isLocalRoot(parentContext));
}

@Test
void spanWithInvalidParentIsLocalRoot() {
Context parentContext = Context.root().with(Span.getInvalid());
assertTrue(LocalRootSpan.isLocalRoot(parentContext));
}

@Test
void spanWithRemoteParentIsLocalRoot() {
Context parentContext =
Context.root()
.with(
Span.wrap(
SpanContext.createFromRemoteParent(
"00000000000000000000000000000001",
"0000000000000002",
TraceFlags.getSampled(),
TraceState.getDefault())));
assertTrue(LocalRootSpan.isLocalRoot(parentContext));
}

@Test
void spanWithValidLocalParentIsNotLocalRoot() {
Context parentContext =
Context.root()
.with(
Span.wrap(
SpanContext.create(
"00000000000000000000000000000001",
"0000000000000002",
TraceFlags.getSampled(),
TraceState.getDefault())));
assertFalse(LocalRootSpan.isLocalRoot(parentContext));
}

@Test
void shouldGetLocalRootSpan() {
Span span = Span.getInvalid();
Context context = LocalRootSpan.store(Context.root(), span);

try (Scope ignored = context.makeCurrent()) {
assertSame(span, LocalRootSpan.current());
}
assertSame(span, LocalRootSpan.fromContext(context));
assertSame(span, LocalRootSpan.fromContextOrNull(context));
}

@Test
void shouldNotGetLocalRootSpanIfThereIsNone() {
Context context = Context.root();

try (Scope ignored = context.makeCurrent()) {
assertFalse(LocalRootSpan.current().getSpanContext().isValid());
}
assertFalse(LocalRootSpan.fromContext(context).getSpanContext().isValid());
assertNull(LocalRootSpan.fromContextOrNull(context));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,9 @@ void none_shouldNotSuppressAnything(SpanKind spanKind, SpanKey spanKey) {
assertFalse(suppressor.shouldSuppress(context, spanKind));
}

@Test
void none_shouldStoreServerSpanInContext() {
SpanSuppressor suppressor = SpanSuppressionStrategy.NONE.create(emptySet());
Context context = Context.root();

Context newContext = suppressor.storeInContext(context, SpanKind.SERVER, span);

assertNotSame(newContext, context);
assertSame(span, SpanKey.KIND_SERVER.fromContextOrNull(newContext));
}

@ParameterizedTest
@EnumSource(value = SpanKind.class, mode = EnumSource.Mode.EXCLUDE, names = "SERVER")
void none_shouldNotStoreNonServerSpansInContext(SpanKind spanKind) {
@EnumSource(value = SpanKind.class)
void none_shouldNotStoreSpansInContext(SpanKind spanKind) {
SpanSuppressor suppressor = SpanSuppressionStrategy.NONE.create(emptySet());
Context context = Context.root();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource;
import io.opentelemetry.instrumentation.api.server.ServerSpan;
import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
import javax.ws.rs.container.ContainerRequestContext;

Expand All @@ -25,7 +25,7 @@ public static Context createOrUpdateAbortSpan(

requestContext.setProperty(JaxrsSingletons.ABORT_HANDLED, true);
Context parentContext = Java8BytecodeBridge.currentContext();
Span serverSpan = ServerSpan.fromContextOrNull(parentContext);
Span serverSpan = LocalRootSpan.fromContextOrNull(parentContext);
Span currentSpan = Java8BytecodeBridge.spanFromContext(parentContext);

HttpRouteHolder.updateHttpRoute(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.server.ServerSpan;
import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath;
import javax.servlet.http.HttpServletRequest;
import org.apache.axis2.jaxws.core.MessageContext;

public final class Axis2ServerSpanNaming {

public static void updateServerSpan(Context context, Axis2Request axis2Request) {
Span serverSpan = ServerSpan.fromContextOrNull(context);
Span serverSpan = LocalRootSpan.fromContextOrNull(context);
if (serverSpan == null) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.server.ServerSpan;
import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath;
import javax.servlet.http.HttpServletRequest;

Expand All @@ -19,7 +19,7 @@ public static void updateServerSpanName(Context context, CxfRequest cxfRequest)
return;
}

Span serverSpan = ServerSpan.fromContextOrNull(context);
Span serverSpan = LocalRootSpan.fromContextOrNull(context);
if (serverSpan == null) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import com.sun.xml.ws.api.message.Packet;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.server.ServerSpan;
import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath;
import javax.servlet.http.HttpServletRequest;
import javax.xml.ws.handler.MessageContext;
Expand All @@ -21,7 +21,7 @@ public static void updateServerSpanName(Context context, MetroRequest metroReque
return;
}

Span serverSpan = ServerSpan.fromContextOrNull(context);
Span serverSpan = LocalRootSpan.fromContextOrNull(context);
if (serverSpan == null) {
return;
}
Expand Down
Loading

0 comments on commit 2c4acd7

Please sign in to comment.