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

Set route only on the SERVER span #10290

Merged
merged 5 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.internal.InstrumenterAccess;
import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil;
import io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan;
import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics;
import java.time.Instant;
import java.util.ArrayList;
Expand Down Expand Up @@ -203,6 +204,9 @@ private Context doStart(Context parentContext, REQUEST request, @Nullable Instan

if (localRoot) {
context = LocalRootSpan.store(context, span);
if (spanKind == SpanKind.SERVER) {
context = LocalRootServerSpan.store(context, span);
}
}

return spanSuppressor.storeInContext(context, spanKind, span);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.internal;

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

/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time.
*/
public final class LocalRootServerSpan {

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

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

public static Context store(Context context, Span span) {
return context.with(KEY, span);
}

private LocalRootServerSpan() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
import io.opentelemetry.instrumentation.api.instrumenter.ContextCustomizer;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
import io.opentelemetry.instrumentation.api.internal.HttpRouteState;
import io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -97,7 +97,7 @@ public static <T, U> void update(
HttpServerRouteBiGetter<T, U> httpRouteGetter,
T arg1,
U arg2) {
Span serverSpan = LocalRootSpan.fromContextOrNull(context);
Span serverSpan = LocalRootServerSpan.fromContextOrNull(context);
trask marked this conversation as resolved.
Show resolved Hide resolved
// 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 @@ -12,6 +12,7 @@
import static org.mockito.Mockito.when;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension;
Expand Down Expand Up @@ -39,7 +40,7 @@ void setUp() {
HttpServerRoute.builder(getter)
.setKnownMethods(new HashSet<>(singletonList("GET")))
.build())
.buildInstrumenter();
.buildInstrumenter(s -> SpanKind.SERVER);
}

@Test
Expand All @@ -61,6 +62,27 @@ void noLocalRootSpan() {
span -> assertThat(span).hasName("parent"), span -> assertThat(span).hasName("test"));
}

@Test
void nonServerRootSpan() {
Instrumenter<String, Void> testInstrumenter =
Instrumenter.<String, Void>builder(testing.getOpenTelemetry(), "test", s -> s)
.addContextCustomizer(
HttpServerRoute.builder(getter)
.setKnownMethods(new HashSet<>(singletonList("GET")))
.build())
.buildInstrumenter(s -> SpanKind.INTERNAL);

Context context = testInstrumenter.start(Context.root(), "test");
assertNull(HttpServerRoute.get(context));

HttpServerRoute.update(context, HttpServerRouteSource.SERVER, "/get/:id");

testInstrumenter.end(context, "test", null, null);

assertNull(HttpServerRoute.get(context));
assertThat(testing.getSpans()).satisfiesExactly(span -> assertThat(span).hasName("test"));
}

@Test
void shouldSetRoute() {
when(getter.getHttpRequestMethod("test")).thenReturn("GET");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ final class InstrumentationApiContextBridging {
// no instrumentation-api on classpath
}

try {
bridges.add(
new ContextKeyBridge<Span, io.opentelemetry.api.trace.Span>(
"application.io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan",
"io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan",
Bridging::toApplication,
Bridging::toAgentOrNull));
} catch (Throwable e) {
// no instrumentation-api on classpath
}

try {
// old SERVER_KEY bridge - needed to make legacy ServerSpan work, for users who're using old
// instrumentation-api version with the newest agent version
Expand Down
Loading