From d7ed85045bf97eec51ed31ce5d7dfe6f06e5e7c8 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 9 Oct 2024 12:14:18 +0200 Subject: [PATCH] Produce INTERNAL_ERROR observation outcome Prior to this commit, the `DefaultExecutionRequestObservationConvention` would only produce "INTERNAL_ERROR" outcomes if the response is null or if an unresolved exception remains. The `ExceptionResolversExceptionHandler` will catch all unresolved exceptions and add them to the errors map with the `ErorType.INTERNAL_ERROR` error type. This means that the "INTERNAL_ERROR" outcome is never used. This commit ensures that this outcome is also used if at least one resolved error is of `ErorType.INTERNAL_ERROR`. Fixes gh-1058 --- .../modules/ROOT/pages/observability.adoc | 6 +++++- .../DefaultExecutionRequestObservationConvention.java | 6 +++++- ...efaultExecutionRequestObservationConventionTests.java | 9 +++++++-- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/spring-graphql-docs/modules/ROOT/pages/observability.adoc b/spring-graphql-docs/modules/ROOT/pages/observability.adoc index 793977ac..688dc011 100644 --- a/spring-graphql-docs/modules/ROOT/pages/observability.adoc +++ b/spring-graphql-docs/modules/ROOT/pages/observability.adoc @@ -32,7 +32,11 @@ By default, the following KeyValues are created: |=== The `graphql.operation` KeyValue will use the custom name of the provided query, or http://spec.graphql.org/draft/#sec-Language.Operations[the standard name for the operation] if none (`"query"`, `"mutation"` or `"subscription"`). -The `graphql.outcome` KeyValue will be `"SUCCESS"` if a valid GraphQL response has been sent, `"REQUEST_ERROR"` if the request could not be parsed, or `"INTERNAL_ERROR"` if no valid GraphQL response could be produced. +The `graphql.outcome` KeyValue will be: + +* `"SUCCESS"` if a valid GraphQL response has been sent and it contains no errors +* `"REQUEST_ERROR"` if the request could not be parsed, or if the response contains errors (none of them being of type `org.springframework.graphql.execution.ErrorType.INTERNAL_ERROR`) +* `"INTERNAL_ERROR"` if no valid GraphQL response could be produced, or if the response contains at least one error of type `org.springframework.graphql.execution.ErrorType.INTERNAL_ERROR` .High cardinality Keys [cols="a,a"] diff --git a/spring-graphql/src/main/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConvention.java b/spring-graphql/src/main/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConvention.java index 9cd4e62d..4f94eb22 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConvention.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConvention.java @@ -19,6 +19,7 @@ package org.springframework.graphql.observation; import io.micrometer.common.KeyValue; import io.micrometer.common.KeyValues; +import org.springframework.graphql.execution.ErrorType; import org.springframework.graphql.observation.GraphQlObservationDocumentation.ExecutionRequestHighCardinalityKeyNames; import org.springframework.graphql.observation.GraphQlObservationDocumentation.ExecutionRequestLowCardinalityKeyNames; @@ -73,7 +74,10 @@ public class DefaultExecutionRequestObservationConvention implements ExecutionRe if (context.getError() != null || context.getExecutionResult() == null) { return OUTCOME_INTERNAL_ERROR; } - else if (context.getExecutionResult().getErrors().size() > 0) { + else if (!context.getExecutionResult().getErrors().isEmpty()) { + if (context.getExecutionResult().getErrors().stream().anyMatch((error) -> ErrorType.INTERNAL_ERROR.equals(error.getErrorType()))) { + return OUTCOME_INTERNAL_ERROR; + } return OUTCOME_REQUEST_ERROR; } return OUTCOME_SUCCESS; diff --git a/spring-graphql/src/test/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConventionTests.java b/spring-graphql/src/test/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConventionTests.java index e31d8e99..deee966b 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConventionTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConventionTests.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. @@ -19,12 +19,16 @@ package org.springframework.graphql.observation; import java.util.function.Consumer; import graphql.ExecutionInput; +import graphql.ExecutionResult; import graphql.ExecutionResultImpl; +import graphql.GraphQLError; import graphql.execution.ExecutionId; import graphql.schema.idl.errors.QueryOperationMissingError; import io.micrometer.common.KeyValue; import org.junit.jupiter.api.Test; +import org.springframework.graphql.execution.ErrorType; + import static org.assertj.core.api.Assertions.assertThat; /** @@ -86,7 +90,8 @@ class DefaultExecutionRequestObservationConventionTests { void hasOutcomeKeyValueWhenInternalError() { ExecutionRequestObservationContext context = createObservationContext(this.input, builder -> { }); - context.setError(new IllegalStateException("custom internal error")); + GraphQLError graphQLError = GraphQLError.newError().errorType(ErrorType.INTERNAL_ERROR).message(ErrorType.INTERNAL_ERROR + " for [executionId]").build(); + context.setExecutionResult(ExecutionResult.newExecutionResult().addError(graphQLError).build()); assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.outcome", "INTERNAL_ERROR")); }