Skip to content

Commit 9286aba

Browse files
opentelemetrybotlaurit
opentelemetrybot
and
laurit
authored
[release/v1.32.x] Set route only on the SERVER span (#10580)
Co-authored-by: Lauri Tulmin <[email protected]>
1 parent e4f3bc5 commit 9286aba

File tree

6 files changed

+130
-19
lines changed

6 files changed

+130
-19
lines changed

‎instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerRoute.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import io.opentelemetry.instrumentation.api.instrumenter.ContextCustomizer;
1111
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
1212
import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
13-
import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
1413
import io.opentelemetry.instrumentation.api.internal.HttpRouteState;
1514
import javax.annotation.Nullable;
1615

@@ -95,16 +94,17 @@ public static <T, U> void update(
9594
HttpServerRouteBiGetter<T, U> httpRouteGetter,
9695
T arg1,
9796
U arg2) {
98-
Span serverSpan = LocalRootSpan.fromContextOrNull(context);
97+
HttpRouteState httpRouteState = HttpRouteState.fromContextOrNull(context);
98+
if (httpRouteState == null) {
99+
return;
100+
}
101+
Span serverSpan = httpRouteState.getSpan();
99102
// even if the server span is not sampled, we have to continue - we need to compute the
100103
// http.route properly so that it can be captured by the server metrics
101104
if (serverSpan == null) {
102105
return;
103106
}
104-
HttpRouteState httpRouteState = HttpRouteState.fromContextOrNull(context);
105-
if (httpRouteState == null) {
106-
return;
107-
}
107+
108108
// special case for servlet filters, even when we have a route from previous filter see whether
109109
// the new route is better and if so use it instead
110110
boolean onlyIfBetterRoute =

‎instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerRouteTest.java

+23-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import static org.mockito.Mockito.when;
1313

1414
import io.opentelemetry.api.trace.Span;
15+
import io.opentelemetry.api.trace.SpanKind;
1516
import io.opentelemetry.context.Context;
1617
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
1718
import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension;
@@ -39,7 +40,7 @@ void setUp() {
3940
HttpServerRoute.builder(getter)
4041
.setKnownMethods(new HashSet<>(singletonList("GET")))
4142
.build())
42-
.buildInstrumenter();
43+
.buildInstrumenter(s -> SpanKind.SERVER);
4344
}
4445

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

65+
@Test
66+
void nonServerRootSpan() {
67+
Instrumenter<String, Void> testInstrumenter =
68+
Instrumenter.<String, Void>builder(testing.getOpenTelemetry(), "test", s -> s)
69+
.addContextCustomizer(
70+
HttpServerRoute.builder(getter)
71+
.setKnownMethods(new HashSet<>(singletonList("GET")))
72+
.build())
73+
.buildInstrumenter(s -> SpanKind.INTERNAL);
74+
75+
Context context = testInstrumenter.start(Context.root(), "test");
76+
assertNull(HttpServerRoute.get(context));
77+
78+
HttpServerRoute.update(context, HttpServerRouteSource.SERVER, "/get/:id");
79+
80+
testInstrumenter.end(context, "test", null, null);
81+
82+
assertNull(HttpServerRoute.get(context));
83+
assertThat(testing.getSpans()).satisfiesExactly(span -> assertThat(span).hasName("test"));
84+
}
85+
6486
@Test
6587
void shouldSetRoute() {
6688
when(getter.getHttpRequestMethod("test")).thenReturn("GET");

‎instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java

+4
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import io.opentelemetry.api.trace.SpanKind;
1212
import io.opentelemetry.api.trace.Tracer;
1313
import io.opentelemetry.context.Context;
14+
import io.opentelemetry.instrumentation.api.internal.HttpRouteState;
1415
import io.opentelemetry.instrumentation.api.internal.InstrumenterAccess;
1516
import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil;
1617
import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics;
@@ -203,6 +204,9 @@ private Context doStart(Context parentContext, REQUEST request, @Nullable Instan
203204

204205
if (localRoot) {
205206
context = LocalRootSpan.store(context, span);
207+
if (spanKind == SpanKind.SERVER) {
208+
HttpRouteState.updateSpan(context, span);
209+
}
206210
}
207211

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

‎instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/HttpRouteState.java

+24-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.instrumentation.api.internal;
77

8+
import io.opentelemetry.api.trace.Span;
89
import io.opentelemetry.context.Context;
910
import io.opentelemetry.context.ContextKey;
1011
import io.opentelemetry.context.ImplicitContextKeyed;
@@ -24,20 +25,36 @@ public static HttpRouteState fromContextOrNull(Context context) {
2425
return context.get(KEY);
2526
}
2627

28+
public static void updateSpan(Context context, Span span) {
29+
HttpRouteState state = fromContextOrNull(context);
30+
if (state != null) {
31+
state.span = span;
32+
}
33+
}
34+
35+
// this method is used reflectively from InstrumentationApiContextBridging
2736
public static HttpRouteState create(
2837
@Nullable String method, @Nullable String route, int updatedBySourceOrder) {
29-
return new HttpRouteState(method, route, updatedBySourceOrder);
38+
return create(method, route, updatedBySourceOrder, null);
39+
}
40+
41+
// this method is used reflectively from InstrumentationApiContextBridging
42+
public static HttpRouteState create(
43+
@Nullable String method, @Nullable String route, int updatedBySourceOrder, Span span) {
44+
return new HttpRouteState(method, route, updatedBySourceOrder, span);
3045
}
3146

3247
@Nullable private final String method;
3348
@Nullable private volatile String route;
3449
private volatile int updatedBySourceOrder;
50+
@Nullable private volatile Span span;
3551

3652
private HttpRouteState(
37-
@Nullable String method, @Nullable String route, int updatedBySourceOrder) {
53+
@Nullable String method, @Nullable String route, int updatedBySourceOrder, Span span) {
3854
this.method = method;
3955
this.updatedBySourceOrder = updatedBySourceOrder;
4056
this.route = route;
57+
this.span = span;
4158
}
4259

4360
@Override
@@ -59,6 +76,11 @@ public String getRoute() {
5976
return route;
6077
}
6178

79+
@Nullable
80+
public Span getSpan() {
81+
return span;
82+
}
83+
6284
public void update(
6385
@SuppressWarnings("unused")
6486
Context context, // context is used by the javaagent bridge instrumentation

‎instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java

+53-10
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,14 @@ final class InstrumentationApiContextBridging {
5959
private static final MethodHandle AGENT_GET_METHOD;
6060
private static final MethodHandle AGENT_GET_ROUTE;
6161
private static final MethodHandle AGENT_GET_UPDATED_BY_SOURCE_ORDER;
62+
private static final MethodHandle AGENT_GET_SPAN;
6263

6364
private static final Class<?> APPLICATION_HTTP_ROUTE_STATE;
6465
private static final MethodHandle APPLICATION_CREATE;
6566
private static final MethodHandle APPLICATION_GET_METHOD;
6667
private static final MethodHandle APPLICATION_GET_ROUTE;
6768
private static final MethodHandle APPLICATION_GET_UPDATED_BY_SOURCE_ORDER;
69+
private static final MethodHandle APPLICATION_GET_SPAN;
6870

6971
static {
7072
MethodHandles.Lookup lookup = MethodHandles.lookup();
@@ -74,11 +76,13 @@ final class InstrumentationApiContextBridging {
7476
MethodHandle agentGetMethod = null;
7577
MethodHandle agentGetRoute = null;
7678
MethodHandle agentGetUpdatedBySourceOrder = null;
79+
MethodHandle agentGetSpan = null;
7780
Class<?> applicationHttpRouteState = null;
7881
MethodHandle applicationCreate = null;
7982
MethodHandle applicationGetMethod = null;
8083
MethodHandle applicationGetRoute = null;
8184
MethodHandle applicationGetUpdatedBySourceOrder = null;
85+
MethodHandle applicationGetSpan = null;
8286

8387
try {
8488
agentHttpRouteState =
@@ -87,23 +91,43 @@ final class InstrumentationApiContextBridging {
8791
lookup.findStatic(
8892
agentHttpRouteState,
8993
"create",
90-
MethodType.methodType(agentHttpRouteState, String.class, String.class, int.class));
94+
MethodType.methodType(
95+
agentHttpRouteState,
96+
String.class,
97+
String.class,
98+
int.class,
99+
io.opentelemetry.api.trace.Span.class));
91100
agentGetMethod =
92101
lookup.findVirtual(agentHttpRouteState, "getMethod", MethodType.methodType(String.class));
93102
agentGetRoute =
94103
lookup.findVirtual(agentHttpRouteState, "getRoute", MethodType.methodType(String.class));
95104
agentGetUpdatedBySourceOrder =
96105
lookup.findVirtual(
97106
agentHttpRouteState, "getUpdatedBySourceOrder", MethodType.methodType(int.class));
107+
agentGetSpan =
108+
lookup.findVirtual(
109+
agentHttpRouteState,
110+
"getSpan",
111+
MethodType.methodType(io.opentelemetry.api.trace.Span.class));
98112

99113
applicationHttpRouteState =
100114
Class.forName("application.io.opentelemetry.instrumentation.api.internal.HttpRouteState");
101-
applicationCreate =
102-
lookup.findStatic(
103-
applicationHttpRouteState,
104-
"create",
105-
MethodType.methodType(
106-
applicationHttpRouteState, String.class, String.class, int.class));
115+
try {
116+
applicationCreate =
117+
lookup.findStatic(
118+
applicationHttpRouteState,
119+
"create",
120+
MethodType.methodType(
121+
applicationHttpRouteState, String.class, String.class, int.class, Span.class));
122+
} catch (NoSuchMethodException exception) {
123+
// older instrumentation-api has only the variant that does not take span
124+
applicationCreate =
125+
lookup.findStatic(
126+
applicationHttpRouteState,
127+
"create",
128+
MethodType.methodType(
129+
applicationHttpRouteState, String.class, String.class, int.class));
130+
}
107131
applicationGetMethod =
108132
lookup.findVirtual(
109133
applicationHttpRouteState, "getMethod", MethodType.methodType(String.class));
@@ -115,6 +139,13 @@ final class InstrumentationApiContextBridging {
115139
applicationHttpRouteState,
116140
"getUpdatedBySourceOrder",
117141
MethodType.methodType(int.class));
142+
try {
143+
applicationGetSpan =
144+
lookup.findVirtual(
145+
applicationHttpRouteState, "getSpan", MethodType.methodType(Span.class));
146+
} catch (NoSuchMethodException ignored) {
147+
// not present in older instrumentation-api
148+
}
118149
} catch (Throwable ignored) {
119150
// instrumentation-api may be absent on the classpath, or it might be an older version
120151
}
@@ -124,11 +155,13 @@ final class InstrumentationApiContextBridging {
124155
AGENT_GET_METHOD = agentGetMethod;
125156
AGENT_GET_ROUTE = agentGetRoute;
126157
AGENT_GET_UPDATED_BY_SOURCE_ORDER = agentGetUpdatedBySourceOrder;
158+
AGENT_GET_SPAN = agentGetSpan;
127159
APPLICATION_HTTP_ROUTE_STATE = applicationHttpRouteState;
128160
APPLICATION_CREATE = applicationCreate;
129161
APPLICATION_GET_METHOD = applicationGetMethod;
130162
APPLICATION_GET_ROUTE = applicationGetRoute;
131163
APPLICATION_GET_UPDATED_BY_SOURCE_ORDER = applicationGetUpdatedBySourceOrder;
164+
APPLICATION_GET_SPAN = applicationGetSpan;
132165
}
133166

134167
@Nullable
@@ -151,12 +184,16 @@ final class InstrumentationApiContextBridging {
151184
APPLICATION_CREATE,
152185
AGENT_GET_METHOD,
153186
AGENT_GET_ROUTE,
154-
AGENT_GET_UPDATED_BY_SOURCE_ORDER),
187+
AGENT_GET_UPDATED_BY_SOURCE_ORDER,
188+
AGENT_GET_SPAN,
189+
o -> o != null ? Bridging.toApplication((io.opentelemetry.api.trace.Span) o) : null),
155190
httpRouteStateConvert(
156191
AGENT_CREATE,
157192
APPLICATION_GET_METHOD,
158193
APPLICATION_GET_ROUTE,
159-
APPLICATION_GET_UPDATED_BY_SOURCE_ORDER));
194+
APPLICATION_GET_UPDATED_BY_SOURCE_ORDER,
195+
APPLICATION_GET_SPAN,
196+
o -> o != null ? Bridging.toAgentOrNull((Span) o) : null));
160197
} catch (Throwable ignored) {
161198
return null;
162199
}
@@ -166,12 +203,18 @@ private static Function<Object, Object> httpRouteStateConvert(
166203
MethodHandle create,
167204
MethodHandle getMethod,
168205
MethodHandle getRoute,
169-
MethodHandle getUpdatedBySourceOrder) {
206+
MethodHandle getUpdatedBySourceOrder,
207+
MethodHandle getSpan,
208+
Function<Object, Object> convertSpan) {
170209
return httpRouteHolder -> {
171210
try {
172211
String method = (String) getMethod.invoke(httpRouteHolder);
173212
String route = (String) getRoute.invoke(httpRouteHolder);
174213
int updatedBySourceOrder = (int) getUpdatedBySourceOrder.invoke(httpRouteHolder);
214+
if (create.type().parameterCount() == 4 && getSpan != null) {
215+
Object span = convertSpan.apply(getSpan.invoke(httpRouteHolder));
216+
return create.invoke(method, route, updatedBySourceOrder, span);
217+
}
175218
return create.invoke(method, route, updatedBySourceOrder);
176219
} catch (Throwable e) {
177220
return null;

‎instrumentation/opentelemetry-instrumentation-api/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/instrumentationapi/HttpRouteStateInstrumentation.java

+20
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
import static net.bytebuddy.matcher.ElementMatchers.named;
99
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
1010

11+
import application.io.opentelemetry.api.trace.Span;
1112
import application.io.opentelemetry.context.Context;
1213
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1314
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
1415
import io.opentelemetry.javaagent.instrumentation.opentelemetryapi.context.AgentContextStorage;
16+
import io.opentelemetry.javaagent.instrumentation.opentelemetryapi.trace.Bridging;
1517
import net.bytebuddy.asm.Advice;
1618
import net.bytebuddy.description.type.TypeDescription;
1719
import net.bytebuddy.matcher.ElementMatcher;
@@ -31,6 +33,11 @@ public void transform(TypeTransformer transformer) {
3133
.and(takesArgument(1, int.class))
3234
.and(takesArgument(2, String.class)),
3335
this.getClass().getName() + "$UpdateAdvice");
36+
transformer.applyAdviceToMethod(
37+
named("updateSpan")
38+
.and(takesArgument(0, named("application.io.opentelemetry.context.Context")))
39+
.and(takesArgument(1, named("application.io.opentelemetry.api.trace.Span"))),
40+
this.getClass().getName() + "$UpdateSpanAdvice");
3441
}
3542

3643
@SuppressWarnings("unused")
@@ -54,4 +61,17 @@ public static void onEnter(
5461
agentRouteState.update(agentContext, updatedBySourceOrder, route);
5562
}
5663
}
64+
65+
@SuppressWarnings("unused")
66+
public static class UpdateSpanAdvice {
67+
@Advice.OnMethodEnter(suppress = Throwable.class)
68+
public static void onEnter(
69+
@Advice.Argument(0) Context applicationContext, @Advice.Argument(1) Span applicationSpan) {
70+
71+
io.opentelemetry.context.Context agentContext =
72+
AgentContextStorage.getAgentContext(applicationContext);
73+
io.opentelemetry.instrumentation.api.internal.HttpRouteState.updateSpan(
74+
agentContext, Bridging.toAgentOrNull(applicationSpan));
75+
}
76+
}
5777
}

0 commit comments

Comments
 (0)