From c55de8d0913f335ef1f4e97dfa63ed32b48748ea Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Tue, 13 Feb 2024 23:20:11 +0100 Subject: [PATCH] Record response errors as events in Request Observations Prior to this commit, GraphQL Request Observations would not record errors as Observation errors, because with GraphQL errors can partially affect the response and there can be multiple. Instead, an invalid request (for example) would lead to a `"graphql.outcome", "REQUEST_ERROR"` low cardinality KeyValue. In this case, developers would not know what type of error occured nor if there were multiple. This commit records all errors listed in the GraphQL as Observation.Event on the request Observation. Such events are usually handled by the tracing handler and are recorded as span annotations for traces. Other `ObservationHandler` annotations can leverage events in a different fashion. Closes gh-859 --- .../modules/ROOT/pages/observability.adoc | 10 ++++ .../GraphQlObservationInstrumentation.java | 16 ++++--- ...raphQlObservationInstrumentationTests.java | 47 +++++++++++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/spring-graphql-docs/modules/ROOT/pages/observability.adoc b/spring-graphql-docs/modules/ROOT/pages/observability.adoc index b3582788..8bfac1f7 100644 --- a/spring-graphql-docs/modules/ROOT/pages/observability.adoc +++ b/spring-graphql-docs/modules/ROOT/pages/observability.adoc @@ -41,6 +41,16 @@ The `graphql.outcome` KeyValue will be `"SUCCESS"` if a valid GraphQL response h |`graphql.execution.id` _(required)_|`graphql.execution.ExecutionId` of the GraphQL request. |=== +Spring for GraphQL also contributes Events for Server Request Observations. +https://docs.micrometer.io/micrometer/reference/observation/components.html#micrometer-observation-events[Micrometer Observation Events] are usually handled as span annotations in traces. +This instrumentation records errors listed in the GraphQL response as events. + +.Observation Events +[cols="a,a"] +|=== +|Name | Contextual Name +|the GraphQL error type, e.g. `InvalidSyntax`|the full GraphQL error message, e.g. `"Invalid syntax with offending token 'invalid'..."` +|=== [[observability.server.datafetcher]] diff --git a/spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationInstrumentation.java b/spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationInstrumentation.java index b3b115a8..c976192e 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationInstrumentation.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationInstrumentation.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2023 the original author or authors. + * Copyright 2020-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -63,8 +63,10 @@ public class GraphQlObservationInstrumentation extends SimplePerformantInstrumen private final ObservationRegistry observationRegistry; + @Nullable private final ExecutionRequestObservationConvention requestObservationConvention; + @Nullable private final DataFetcherObservationConvention dataFetcherObservationConvention; /** @@ -74,9 +76,7 @@ public class GraphQlObservationInstrumentation extends SimplePerformantInstrumen * @param observationRegistry the registry to use for recording observations */ public GraphQlObservationInstrumentation(ObservationRegistry observationRegistry) { - this.observationRegistry = observationRegistry; - this.requestObservationConvention = new DefaultExecutionRequestObservationConvention(); - this.dataFetcherObservationConvention = new DefaultDataFetcherObservationConvention(); + this(observationRegistry, null, null); } /** @@ -87,8 +87,8 @@ public class GraphQlObservationInstrumentation extends SimplePerformantInstrumen * @param dateFetcherObservationConvention the convention to use for data fetcher observations */ public GraphQlObservationInstrumentation(ObservationRegistry observationRegistry, - ExecutionRequestObservationConvention requestObservationConvention, - DataFetcherObservationConvention dateFetcherObservationConvention) { + @Nullable ExecutionRequestObservationConvention requestObservationConvention, + @Nullable DataFetcherObservationConvention dateFetcherObservationConvention) { this.observationRegistry = observationRegistry; this.requestObservationConvention = requestObservationConvention; this.dataFetcherObservationConvention = dateFetcherObservationConvention; @@ -112,6 +112,10 @@ public class GraphQlObservationInstrumentation extends SimplePerformantInstrumen @Override public void onCompleted(ExecutionResult result, Throwable exc) { observationContext.setExecutionResult(result); + result.getErrors().forEach(graphQLError -> { + Observation.Event event = Observation.Event.of(graphQLError.getErrorType().toString(), graphQLError.getMessage()); + requestObservation.event(event); + }); if (exc != null) { requestObservation.error(exc); } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java b/spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java index a2dd367a..1fb63c73 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java @@ -23,6 +23,7 @@ import graphql.schema.AsyncDataFetcher; import graphql.schema.DataFetcher; import io.micrometer.common.KeyValue; import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationHandler; import io.micrometer.observation.ObservationRegistry; import io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor; import io.micrometer.observation.tck.TestObservationRegistry; @@ -36,6 +37,8 @@ import org.springframework.graphql.execution.DataFetcherExceptionResolver; import org.springframework.graphql.execution.ErrorType; import reactor.core.publisher.Mono; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.stream.Stream; @@ -319,4 +322,48 @@ class GraphQlObservationInstrumentationTests { ResponseHelper.forResponse(responseMono); } + @Test + void shouldRecordGraphQlErrorsAsTraceEvents() { + EventListeningObservationHandler observationHandler = new EventListeningObservationHandler(); + this.observationRegistry.observationConfig().observationHandler(observationHandler); + + String document = "invalid{}"; + Mono responseMono = graphQlSetup + .toGraphQlService() + .execute(document); + ResponseHelper response = ResponseHelper.forResponse(responseMono); + + assertThat(response.errorCount()).isEqualTo(1); + assertThat(response.error(0).errorType()).isEqualTo("InvalidSyntax"); + assertThat(response.error(0).message()).startsWith("Invalid syntax with offending token 'invalid'"); + + assertThat(observationHandler.getEvents()).hasSize(1); + Observation.Event errorEvent = observationHandler.getEvents().get(0); + assertThat(errorEvent.getName()).isEqualTo("InvalidSyntax"); + assertThat(errorEvent.getContextualName()).startsWith("Invalid syntax with offending token 'invalid'"); + + TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request") + .that().hasLowCardinalityKeyValue("graphql.outcome", "REQUEST_ERROR") + .hasHighCardinalityKeyValueWithKey("graphql.execution.id"); + } + + static class EventListeningObservationHandler implements ObservationHandler { + + private final List events = new ArrayList<>(); + + @Override + public boolean supportsContext(Observation.Context context) { + return context instanceof ExecutionRequestObservationContext; + } + + @Override + public void onEvent(Observation.Event event, ExecutionRequestObservationContext context) { + this.events.add(event); + } + + public List getEvents() { + return this.events; + } + } + }