Skip to content

Commit f55b804

Browse files
authored
4.x fix span nesting in filter (#9788)
* Refine logic to select parent span in MP Telemetry filter Signed-off-by: Tim Quinn <[email protected]>
1 parent 0c5bc19 commit f55b804

File tree

3 files changed

+166
-12
lines changed

3 files changed

+166
-12
lines changed

microprofile/telemetry/pom.xml

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@
157157
<excludes>
158158
<exclude>**/AgentDetectorTest.java</exclude>
159159
<exclude>**/TestSpanListenersWithInjection.java</exclude>
160-
<exclude>**/TestGlobalTracerAssignment.java</exclude>
160+
<exclude>**/TestFilterSpanNesting.java</exclude>
161161
</excludes>
162162
</configuration>
163163
</execution>
@@ -188,16 +188,13 @@
188188
</configuration>
189189
</execution>
190190
<execution>
191-
<!--
192-
This test sets the global tracer to a no-op tracer which would interfere with other tests.
193-
-->
194-
<id>test-global-tracer-assignment</id>
191+
<id>filter-span-nesting-test</id>
195192
<goals>
196193
<goal>test</goal>
197194
</goals>
198195
<configuration>
199196
<includes>
200-
<include>**/TestGlobalTracerAssignment.java</include>
197+
<include>**/TestFilterSpanNesting.java</include>
201198
</includes>
202199
</configuration>
203200
</execution>

microprofile/telemetry/src/main/java/io/helidon/microprofile/telemetry/HelidonTelemetryContainerFilter.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,7 @@ public void filter(ContainerRequestContext requestContext) {
140140

141141
//Start new span for container request.
142142
String route = route(requestContext);
143-
Optional<SpanContext> extractedSpanContext =
144-
helidonTracer.extract(new RequestContextHeaderProvider(requestContext.getHeaders()));
143+
Optional<SpanContext> parentSpanContext = parentSpanContext(requestContext);
145144
Span helidonSpan = helidonTracer.spanBuilder(spanName(requestContext, route))
146145
.kind(Span.Kind.SERVER)
147146
.tag(HTTP_METHOD, requestContext.getMethod())
@@ -150,7 +149,7 @@ public void filter(ContainerRequestContext requestContext) {
150149
.tag(HTTP_ROUTE, route)
151150
.tag(SemanticAttributes.NET_HOST_NAME.getKey(), requestContext.getUriInfo().getBaseUri().getHost())
152151
.tag(SemanticAttributes.NET_HOST_PORT.getKey(), requestContext.getUriInfo().getBaseUri().getPort())
153-
.update(builder -> extractedSpanContext.ifPresent(builder::parent))
152+
.update(builder -> parentSpanContext.ifPresent(builder::parent))
154153
.start();
155154

156155
Scope helidonScope = helidonSpan.activate();
@@ -203,9 +202,11 @@ public void filter(final ContainerRequestContext request, final ContainerRespons
203202
}
204203
}
205204

206-
// private static List<HelidonTelemetryContainerFilterHelper> helpers() {
207-
// return HelidonServiceLoader.create(ServiceLoader.load(HelidonTelemetryContainerFilterHelper.class)).asList();
208-
// }
205+
private Optional<SpanContext> parentSpanContext(ContainerRequestContext requestContext) {
206+
// Prefer the current span if there is one.
207+
return Span.current().map(Span::context)
208+
.or(() -> helidonTracer.extract(new RequestContextHeaderProvider(requestContext.getHeaders())));
209+
}
209210

210211
private String spanName(ContainerRequestContext requestContext, String route) {
211212
// @Deprecated(forRemoval = true) In 5.x remove the option of excluding the HTTP method from the REST span name.
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
/*
2+
* Copyright (c) 2025 Oracle and/or its affiliates.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.helidon.microprofile.telemetry;
17+
18+
import java.io.IOException;
19+
import java.util.List;
20+
import java.util.Optional;
21+
22+
import io.helidon.common.testing.junit5.OptionalMatcher;
23+
import io.helidon.microprofile.testing.junit5.AddBean;
24+
import io.helidon.microprofile.testing.junit5.AddConfig;
25+
import io.helidon.microprofile.testing.junit5.HelidonTest;
26+
import io.helidon.tracing.Scope;
27+
import io.helidon.tracing.Span;
28+
import io.helidon.tracing.SpanContext;
29+
import io.helidon.tracing.Tracer;
30+
31+
import io.opentelemetry.sdk.trace.data.SpanData;
32+
import jakarta.annotation.Priority;
33+
import jakarta.enterprise.context.ApplicationScoped;
34+
import jakarta.inject.Inject;
35+
import jakarta.ws.rs.GET;
36+
import jakarta.ws.rs.Path;
37+
import jakarta.ws.rs.Priorities;
38+
import jakarta.ws.rs.Produces;
39+
import jakarta.ws.rs.client.WebTarget;
40+
import jakarta.ws.rs.container.ContainerRequestContext;
41+
import jakarta.ws.rs.container.ContainerRequestFilter;
42+
import jakarta.ws.rs.container.ContainerResponseContext;
43+
import jakarta.ws.rs.container.ContainerResponseFilter;
44+
import jakarta.ws.rs.core.MediaType;
45+
import jakarta.ws.rs.core.Request;
46+
import jakarta.ws.rs.core.Response;
47+
import jakarta.ws.rs.ext.Provider;
48+
import org.junit.jupiter.api.BeforeEach;
49+
import org.junit.jupiter.api.Test;
50+
51+
import static org.hamcrest.MatcherAssert.assertThat;
52+
import static org.hamcrest.Matchers.equalTo;
53+
import static org.hamcrest.Matchers.is;
54+
55+
@HelidonTest
56+
@AddBean(TestSpanExporter.class)
57+
@AddBean(TestFilterSpanNesting.TestBean.class)
58+
@AddBean(TestFilterSpanNesting.IngressSpanSetter.class)
59+
@AddConfig(key = "otel.sdk.disabled", value = "false")
60+
@AddConfig(key = "otel.traces.exporter", value = "in-memory")
61+
class TestFilterSpanNesting {
62+
63+
private static Tracer staticTracer;
64+
65+
@Inject
66+
private WebTarget webTarget;
67+
68+
@Inject
69+
private TestSpanExporter testSpanExporter;
70+
71+
@Inject
72+
public TestFilterSpanNesting(Tracer tracer) {
73+
staticTracer = tracer;
74+
}
75+
76+
@BeforeEach
77+
void setUp() {
78+
testSpanExporter.clear();
79+
}
80+
81+
@Test
82+
void testExternalParentSpan() {
83+
84+
var requestBuilder = webTarget.path("/parentSpanCheck")
85+
.request(MediaType.TEXT_PLAIN);
86+
87+
// Our client filter will automatically establish a span for the outgoing Jakarta REST client request.
88+
Response response = requestBuilder.get();
89+
90+
assertThat("Response status", response.getStatus(), is(200));
91+
92+
// Check structure of nested spans.
93+
List<SpanData> spanData = testSpanExporter.spanData(3);
94+
Optional<SpanData> ingressSpanData = spanData.stream()
95+
.filter(sd -> sd.getName().equals("ingressSpan"))
96+
.findFirst();
97+
assertThat("ingress span data", ingressSpanData, OptionalMatcher.optionalPresent());
98+
99+
Optional<SpanData> spanFromJakartaFilter = spanData.stream()
100+
.filter(sd -> sd.getName().equals("/parentSpanCheck"))
101+
.findFirst();
102+
assertThat("/parentSpanCheck span data", spanFromJakartaFilter, OptionalMatcher.optionalPresent());
103+
104+
// Make sure the parent for the span created by the container filter is the current span we set in our test filter,
105+
// not the span inspired by the incoming headers.
106+
assertThat("/parentSpanCheck parent span ID",
107+
spanFromJakartaFilter.get().getParentSpanContext().getSpanId(),
108+
equalTo(ingressSpanData.get().getSpanContext().getSpanId()));
109+
110+
}
111+
112+
@ApplicationScoped
113+
@Path("/parentSpanCheck")
114+
public static class TestBean {
115+
116+
@GET
117+
@Produces(MediaType.TEXT_PLAIN)
118+
public String parentSpanCheck(Request request) {
119+
// The HelidonTelemetryContainerFilter should have been run to establish a new current span. Create a new child.
120+
return "Hello World!";
121+
}
122+
}
123+
124+
/**
125+
* Filter to kind-of play the role of upstream ingress code which sets a current span before our normal filter
126+
* HelidonTelemetryContainerFilter runs.
127+
*/
128+
@Provider
129+
@Priority(Priorities.HEADER_DECORATOR)
130+
static class IngressSpanSetter implements ContainerRequestFilter, ContainerResponseFilter {
131+
132+
private Span pseudoIngressSpan;
133+
private Scope pseudoIngressScope;
134+
135+
@Override
136+
public void filter(ContainerRequestContext requestContext) throws IOException {
137+
// Create a span that's a child of the span represented in the headers and make it current.
138+
// Then the HelidonTelemetryContainerFilter will find this one as current and the span *it* adds should be a child
139+
// of this new pseudo-ingress span which we'll check in the test code.
140+
141+
Optional<SpanContext> helidonSpanContext =
142+
staticTracer.extract(new RequestContextHeaderProvider(requestContext.getHeaders()));
143+
144+
pseudoIngressSpan = staticTracer.spanBuilder("ingressSpan")
145+
.update(spanBuilder -> helidonSpanContext.ifPresent(spanBuilder::parent))
146+
.build();
147+
pseudoIngressScope = pseudoIngressSpan.activate();
148+
}
149+
150+
@Override
151+
public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException {
152+
pseudoIngressScope.close();
153+
pseudoIngressSpan.end();
154+
}
155+
}
156+
}

0 commit comments

Comments
 (0)