From faea9d1c8bcbf3b8cf67a58d26dfe50e43d90515 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 11 May 2023 14:41:48 +0200 Subject: [PATCH] Fix observability instrumentation for CompletionStage This commit fixes the observability instrumentation for data fetchers when the return type is of `CompletionStage`. Prior to this commit, the instrumentation of `CompletionStage` return values as `DataFetcherResult` would wrap the asynchronous value twice. Closes gh-676 --- .../observation/GraphQlObservationInstrumentation.java | 7 +++---- .../GraphQlObservationInstrumentationTests.java | 8 +++++++- 2 files changed, 10 insertions(+), 5 deletions(-) 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 9b232308..74e9af62 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 @@ -35,7 +35,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.lang.Nullable; -import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.concurrent.CompletionStage; /** @@ -152,11 +152,10 @@ public class GraphQlObservationInstrumentation extends SimpleInstrumentation { if (error != null) { dataFetcherObservation.error(error); dataFetcherObservation.stop(); - return CompletableFuture.failedStage(error); + throw new CompletionException(error); } dataFetcherObservation.stop(); - return CompletableFuture.completedStage(wrapAsDataFetcherResult(result, dataFetcherObservation)); - + return wrapAsDataFetcherResult(result, dataFetcherObservation); }); } else { 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 3bef74d2..c02fbe76 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 @@ -68,6 +68,10 @@ class GraphQlObservationInstrumentationTests { .toGraphQlService() .execute(TestExecutionRequest.forDocument(document)); ResponseHelper response = ResponseHelper.forResponse(responseMono); + + String name = response.rawValue("bookById.name"); + assertThat(name).isEqualTo("Nineteen Eighty-Four"); + TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request") .that().hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS") .hasHighCardinalityKeyValueWithKey("graphql.execution.id"); @@ -167,8 +171,10 @@ class GraphQlObservationInstrumentationTests { .toGraphQlService() .execute(TestExecutionRequest.forDocument(document)); ResponseHelper response = ResponseHelper.forResponse(responseMono); + assertThat(response.error(0).message()).isEqualTo("Resolved error: book fetching failure"); + TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request") - .that().hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS") + .that().hasLowCardinalityKeyValue("graphql.outcome", "REQUEST_ERROR") .hasHighCardinalityKeyValueWithKey("graphql.execution.id"); TestObservationRegistryAssert.assertThat(this.observationRegistry)