Skip to content

Commit a43c928

Browse files
committed
Fix NullPointerException in Tracing support
Prior to this commit, using the tracing support could result in an NullPointerException since the instrumentation expected the tracing propagation information to be always available from the request extensions. This commit improves the strategy for extracing the inbound propagation information. First, the instrumentation now looks up the propagation information in the GraphQL context first, then in the request extensions as a fallback. We ensure that no NPE can be raised if the information is missing. Some clients might change the propagation information in the request extensions, but in the HTTP world, most clients will send those as HTTP headers. We are adding a new `PropagationWebGraphQlInterceptor` that can be configured on the application to copy the relevant HTTP headers from the HTTP request to the GraphQL context. For other transports, we currently advise the request extensions - both WebSocket and RSocket multiplex over the same transport message, making this approach impossible. Fixes gh-547
1 parent 5b4eb8c commit a43c928

File tree

7 files changed

+336
-3
lines changed

7 files changed

+336
-3
lines changed

platform/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ dependencies {
1111
api(platform("com.fasterxml.jackson:jackson-bom:2.14.1"))
1212
api(platform("io.projectreactor:reactor-bom:2022.0.0"))
1313
api(platform("io.micrometer:micrometer-bom:1.10.2"))
14+
api(platform("io.micrometer:micrometer-tracing-bom:1.0.0"))
1415
api(platform("org.springframework.data:spring-data-bom:2022.0.0"))
1516
api(platform("org.springframework.security:spring-security-bom:6.0.0"))
1617
api(platform("com.querydsl:querydsl-bom:5.0.0"))

spring-graphql/build.gradle

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ dependencies {
99
implementation 'io.micrometer:context-propagation'
1010

1111
compileOnly 'io.micrometer:micrometer-observation'
12+
compileOnly 'io.micrometer:micrometer-tracing'
1213
compileOnly 'jakarta.annotation:jakarta.annotation-api'
1314
compileOnly 'org.springframework:spring-webflux'
1415
compileOnly 'org.springframework:spring-webmvc'
@@ -45,6 +46,7 @@ dependencies {
4546
testImplementation 'org.springframework.data:spring-data-keyvalue'
4647
testImplementation 'org.springframework.data:spring-data-jpa'
4748
testImplementation 'io.micrometer:micrometer-observation-test'
49+
testImplementation 'io.micrometer:micrometer-tracing-test'
4850
testImplementation 'com.h2database:h2'
4951
testImplementation 'org.hibernate:hibernate-core'
5052
testImplementation 'org.hibernate.validator:hibernate-validator'

spring-graphql/src/main/java/org/springframework/graphql/observation/ExecutionRequestObservationContext.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.graphql.observation;
1818

19+
import java.util.Map;
20+
1921
import graphql.ExecutionInput;
2022
import graphql.ExecutionResult;
2123
import io.micrometer.observation.transport.RequestReplyReceiverContext;
@@ -24,16 +26,32 @@
2426
* Context that holds information for metadata collection during observations
2527
* for {@link GraphQlObservationDocumentation#EXECUTION_REQUEST GraphQL requests}.
2628
* <p>This context also extends {@link RequestReplyReceiverContext} for propagating
27-
* tracing information with the HTTP server exchange.
29+
* tracing information from the {@link graphql.GraphQLContext}
30+
* or the {@link ExecutionInput#getExtensions() input extensions}.
2831
*
2932
* @author Brian Clozel
3033
* @since 1.1.0
3134
*/
3235
public class ExecutionRequestObservationContext extends RequestReplyReceiverContext<ExecutionInput, ExecutionResult> {
3336

3437
public ExecutionRequestObservationContext(ExecutionInput executionInput) {
35-
super((input, key) -> executionInput.getExtensions().get(key).toString());
38+
super(ExecutionRequestObservationContext::getContextValue);
3639
setCarrier(executionInput);
3740
}
3841

42+
/**
43+
* Read propagation field from the {@link graphql.GraphQLContext},
44+
* or the {@link ExecutionInput#getExtensions() input extensions} as a fallback.
45+
*/
46+
private static String getContextValue(ExecutionInput executionInput, String key) {
47+
String value = executionInput.getGraphQLContext().get(key);
48+
if (value == null) {
49+
Map<String, Object> extensions = executionInput.getExtensions();
50+
if (extensions != null) {
51+
value = (String) extensions.get(key);
52+
}
53+
}
54+
return value;
55+
}
56+
3957
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright 2020-2022 the original author or authors.
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+
* https://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+
17+
package org.springframework.graphql.observation;
18+
19+
import io.micrometer.tracing.propagation.Propagator;
20+
import reactor.core.publisher.Mono;
21+
22+
import org.springframework.graphql.server.WebGraphQlInterceptor;
23+
import org.springframework.graphql.server.WebGraphQlRequest;
24+
import org.springframework.graphql.server.WebGraphQlResponse;
25+
import org.springframework.http.HttpHeaders;
26+
import org.springframework.util.Assert;
27+
28+
/**
29+
* {@link WebGraphQlInterceptor} that copies {@link Propagator propagation} headers
30+
* from the HTTP request to the {@link graphql.GraphQLContext}.
31+
* This makes it possible to propagate tracing information sent by HTTP clients.
32+
*
33+
* @author Brian Clozel
34+
* @since 1.1.1
35+
*/
36+
public class PropagationWebGraphQlInterceptor implements WebGraphQlInterceptor {
37+
38+
private final Propagator propagator;
39+
40+
/**
41+
* Create an interceptor that leverages the field names used by the given
42+
* {@link Propagator} instance.
43+
*
44+
* @param propagator the propagator that will be used for tracing support
45+
*/
46+
public PropagationWebGraphQlInterceptor(Propagator propagator) {
47+
Assert.notNull(propagator, "propagator should not be null");
48+
this.propagator = propagator;
49+
}
50+
51+
@Override
52+
public Mono<WebGraphQlResponse> intercept(WebGraphQlRequest request, Chain chain) {
53+
request.configureExecutionInput((input, inputBuilder) -> {
54+
HttpHeaders headers = request.getHeaders();
55+
for (String field : this.propagator.fields()) {
56+
if (headers.containsKey(field)) {
57+
inputBuilder.graphQLContext(contextBuilder -> contextBuilder.of(field, headers.getFirst(field)));
58+
}
59+
}
60+
return inputBuilder.build();
61+
});
62+
return chain.next(request);
63+
}
64+
65+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Copyright 2020-2022 the original author or authors.
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+
* https://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+
17+
package org.springframework.graphql.observation;
18+
19+
20+
import java.util.Map;
21+
22+
import graphql.ExecutionInput;
23+
import org.junit.jupiter.api.Test;
24+
25+
import static org.assertj.core.api.Assertions.assertThat;
26+
27+
/**
28+
* Tests for {@link ExecutionRequestObservationContext}.
29+
*
30+
* @author Brian Clozel
31+
*/
32+
class ExecutionRequestObservationContextTests {
33+
34+
@Test
35+
void readPropagationFieldFromGraphQlContext() {
36+
ExecutionInput executionInput = ExecutionInput
37+
.newExecutionInput("{ notUsed }")
38+
.graphQLContext(builder -> builder.of("X-Tracing-Test", "traceId"))
39+
.build();
40+
ExecutionRequestObservationContext context = new ExecutionRequestObservationContext(executionInput);
41+
assertThat(context.getGetter().get(executionInput, "X-Tracing-Test")).isEqualTo("traceId");
42+
}
43+
44+
@Test
45+
void readPropagationFieldFromExtensions() {
46+
ExecutionInput executionInput = ExecutionInput
47+
.newExecutionInput("{ notUsed }")
48+
.extensions(Map.of("X-Tracing-Test", "traceId"))
49+
.build();
50+
ExecutionRequestObservationContext context = new ExecutionRequestObservationContext(executionInput);
51+
assertThat(context.getGetter().get(executionInput, "X-Tracing-Test")).isEqualTo("traceId");
52+
}
53+
54+
@Test
55+
void doesNotFailIsMissingPropagationField() {
56+
ExecutionInput executionInput = ExecutionInput
57+
.newExecutionInput("{ notUsed }")
58+
.build();
59+
ExecutionRequestObservationContext context = new ExecutionRequestObservationContext(executionInput);
60+
assertThat(context.getGetter().get(executionInput, "X-Tracing-Test")).isNull();
61+
}
62+
}

spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,35 @@
1616

1717
package org.springframework.graphql.observation;
1818

19+
import java.util.List;
1920
import java.util.concurrent.CompletableFuture;
2021

2122
import graphql.GraphqlErrorBuilder;
2223
import io.micrometer.observation.tck.TestObservationRegistry;
2324
import io.micrometer.observation.tck.TestObservationRegistryAssert;
25+
import io.micrometer.observation.transport.ReceiverContext;
26+
import io.micrometer.tracing.Span;
27+
import io.micrometer.tracing.TraceContext;
28+
import io.micrometer.tracing.handler.PropagatingReceiverTracingObservationHandler;
29+
import io.micrometer.tracing.handler.TracingObservationHandler;
30+
import io.micrometer.tracing.propagation.Propagator;
31+
import io.micrometer.tracing.test.simple.SimpleSpanBuilder;
32+
import io.micrometer.tracing.test.simple.SimpleTracer;
33+
import io.micrometer.tracing.test.simple.TracerAssert;
2434
import org.junit.jupiter.api.Test;
2535
import reactor.core.publisher.Mono;
2636

2737
import org.springframework.graphql.BookSource;
38+
import org.springframework.graphql.ExecutionGraphQlRequest;
2839
import org.springframework.graphql.ExecutionGraphQlResponse;
2940
import org.springframework.graphql.GraphQlSetup;
3041
import org.springframework.graphql.ResponseHelper;
3142
import org.springframework.graphql.TestExecutionRequest;
3243
import org.springframework.graphql.execution.DataFetcherExceptionResolver;
3344
import org.springframework.graphql.execution.ErrorType;
3445

46+
import static org.assertj.core.api.Assertions.assertThat;
47+
3548
/**
3649
* Tests for {@link GraphQlObservationInstrumentation}.
3750
*
@@ -186,6 +199,65 @@ void propagatesContextBetweenObservations() {
186199
.hasParentObservationContextMatching(context -> context instanceof ExecutionRequestObservationContext);
187200
}
188201

202+
@Test
203+
void inboundTracingInformationIsPropagated() {
204+
SimpleTracer simpleTracer = new SimpleTracer();
205+
String traceId = "traceId";
206+
TracingObservationHandler<ReceiverContext> tracingHandler = new PropagatingReceiverTracingObservationHandler<>(simpleTracer, new TestPropagator(simpleTracer, traceId));
207+
this.observationRegistry.observationConfig().observationHandler(tracingHandler);
208+
String document = """
209+
{
210+
bookById(id: 1) {
211+
name
212+
}
213+
}
214+
""";
215+
ExecutionGraphQlRequest executionRequest = TestExecutionRequest.forDocument(document);
216+
executionRequest.configureExecutionInput((input, builder) ->
217+
builder.graphQLContext(context -> context.of(TestPropagator.TRACING_HEADER_NAME, traceId)).build());
218+
Mono<ExecutionGraphQlResponse> responseMono = graphQlSetup
219+
.queryFetcher("bookById", env -> BookSource.getBookWithoutAuthor(1L))
220+
.toGraphQlService()
221+
.execute(executionRequest);
222+
ResponseHelper response = ResponseHelper.forResponse(responseMono);
223+
224+
TracerAssert.assertThat(simpleTracer)
225+
.onlySpan()
226+
.hasNameEqualTo("graphql query")
227+
.hasKindEqualTo(Span.Kind.SERVER)
228+
.hasTag("graphql.operation", "query")
229+
.hasTag("graphql.outcome", "SUCCESS")
230+
.hasTagWithKey("graphql.execution.id");
231+
}
232+
233+
static class TestPropagator implements Propagator {
234+
235+
public static String TRACING_HEADER_NAME = "X-Test-Tracing";
189236

237+
private final SimpleTracer tracer;
190238

191-
}
239+
private final String traceId;
240+
241+
TestPropagator(SimpleTracer tracer, String traceId) {
242+
this.tracer = tracer;
243+
this.traceId = traceId;
244+
}
245+
246+
@Override
247+
public List<String> fields() {
248+
return List.of(TRACING_HEADER_NAME);
249+
}
250+
251+
@Override
252+
public <C> void inject(TraceContext context, C carrier, Setter<C> setter) {
253+
setter.set(carrier, TRACING_HEADER_NAME, "traceId");
254+
}
255+
256+
@Override
257+
public <C> Span.Builder extract(C carrier, Getter<C> getter) {
258+
String foo = getter.get(carrier, TRACING_HEADER_NAME);
259+
assertThat(foo).isEqualTo(this.traceId);
260+
return new SimpleSpanBuilder(this.tracer);
261+
}
262+
}
263+
}

0 commit comments

Comments
 (0)