From db35e582645c4e0950fe06ca2aca65ead370cb19 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 4 Oct 2022 23:09:47 +0200 Subject: [PATCH] Attaches error to Observation (#2743) without this change when there is no response and the observation closing web exception handler stops the observation we've forgotten to attach an error to it with this change we're attaching the error --- .../DefaultGatewayObservationConvention.java | 5 +++-- .../headers/observation/GatewayContext.java | 1 + ...ObservationClosingWebExceptionHandler.java | 1 + ...B3BraveObservedHttpHeadersFilterTests.java | 11 +++++----- ...vationClosingWebExceptionHandlerTests.java | 4 +++- .../ObservedHttpHeadersFilterTests.java | 21 +++++++++---------- 6 files changed, 23 insertions(+), 20 deletions(-) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java index cb44f48d..1bc4e15f 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java @@ -50,8 +50,9 @@ public class DefaultGatewayObservationConvention implements GatewayObservationCo return keyValues; } Route route = context.getServerWebExchange().getAttribute(ServerWebExchangeUtils.GATEWAY_ROUTE_ATTR); - keyValues = keyValues.and(ROUTE_URI.withValue(route.getUri().toString()), - METHOD.withValue(context.getRequest().getMethod().name())) + keyValues = keyValues + .and(ROUTE_URI.withValue(route.getUri().toString()), + METHOD.withValue(context.getRequest().getMethod().name())) .and(ROUTE_ID.withValue(route.getId())); ServerHttpResponse response = context.getResponse(); if (response != null && response.getStatusCode() != null) { diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/GatewayContext.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/GatewayContext.java index 75ea3b8b..dbe5b5b9 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/GatewayContext.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/GatewayContext.java @@ -53,4 +53,5 @@ public class GatewayContext extends RequestReplySenderContext "Observation was not previously stopped, will stop it."); + observation.error(ex); observation.stop(); } } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/B3BraveObservedHttpHeadersFilterTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/B3BraveObservedHttpHeadersFilterTests.java index a5e273e2..64de2e59 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/B3BraveObservedHttpHeadersFilterTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/B3BraveObservedHttpHeadersFilterTests.java @@ -104,12 +104,11 @@ class B3BraveObservedHttpHeadersFilterTests { assertThat(headers.get("b3").get(0)).matches("^" + context.traceId() + "-(.*)-1-" + context.spanId() + "$"); List finishedSpans = testSpanHandler.spans().stream().map(BraveFinishedSpan::new) .collect(Collectors.toList()); - SpansAssert.then(finishedSpans).hasASpanWithName("HTTP GET", spanAssert -> spanAssert - .hasTag("spring.cloud.gateway.route.id", "foo") - .hasTag("http.method", "GET") - .hasTag("http.status_code", "200") - .hasTag("spring.cloud.gateway.route.uri", "http://localhost:8080/") - .hasTag("http.uri", "http://localhost:8080/get")); + SpansAssert.then(finishedSpans).hasASpanWithName("HTTP GET", + spanAssert -> spanAssert.hasTag("spring.cloud.gateway.route.id", "foo").hasTag("http.method", "GET") + .hasTag("http.status_code", "200") + .hasTag("spring.cloud.gateway.route.uri", "http://localhost:8080/") + .hasTag("http.uri", "http://localhost:8080/get")); }); } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservationClosingWebExceptionHandlerTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservationClosingWebExceptionHandlerTests.java index fe4f7042..c7e6666a 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservationClosingWebExceptionHandlerTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservationClosingWebExceptionHandlerTests.java @@ -52,8 +52,10 @@ class ObservationClosingWebExceptionHandlerTests { void shouldStopTheObservationIfItWasNotStoppedPreviouslyAndThereWasAnError() { Observation observation = Mockito.mock(Observation.class); exchange.getAttributes().put(ObservedRequestHttpHeadersFilter.CHILD_OBSERVATION, observation); + RuntimeException runtimeException = new RuntimeException(); - assertThatNoException().isThrownBy(() -> handler.handle(exchange, new RuntimeException())); + assertThatNoException().isThrownBy(() -> handler.handle(exchange, runtimeException)); + Mockito.verify(observation).error(runtimeException); Mockito.verify(observation).stop(); } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservedHttpHeadersFilterTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservedHttpHeadersFilterTests.java index 5ff602fe..6c0953e3 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservedHttpHeadersFilterTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservedHttpHeadersFilterTests.java @@ -84,17 +84,16 @@ public class ObservedHttpHeadersFilterTests extends SampleTestRunner { .containsEntry("X-B3-TraceId", Collections.singletonList(context.traceId())) .doesNotContainEntry("X-B3-SpanId", Collections.singletonList(context.spanId())) .containsKey("X-B3-SpanId"); - SpansAssert.then(bb.getFinishedSpans()).hasASpanWithName("HTTP GET", spanAssert -> spanAssert - .hasTag("http.method", "GET").hasTag("http.status_code", "200") - .hasTag("http.uri", "http://localhost:8080/get") - .hasTag("spring.cloud.gateway.route.uri", "http://localhost:8080/") - .hasTag("spring.cloud.gateway.route.id", "foo")); - MeterRegistryAssert.then(meterRegistry) - .hasTimerWithNameAndTags("http.client.requests", - Tags.of("spring.cloud.gateway.route.id", "foo", "error", "none", "http.method", "GET", "http.status_code", "200", "spring.cloud.gateway.route.uri", - "http://localhost:8080/")) - .hasMeterWithNameAndTags("http.client.requests.active", - Tags.of("spring.cloud.gateway.route.id", "foo", "http.method", "GET", "spring.cloud.gateway.route.uri", "http://localhost:8080/")); + SpansAssert.then(bb.getFinishedSpans()).hasASpanWithName("HTTP GET", + spanAssert -> spanAssert.hasTag("http.method", "GET").hasTag("http.status_code", "200") + .hasTag("http.uri", "http://localhost:8080/get") + .hasTag("spring.cloud.gateway.route.uri", "http://localhost:8080/") + .hasTag("spring.cloud.gateway.route.id", "foo")); + MeterRegistryAssert.then(meterRegistry).hasTimerWithNameAndTags("http.client.requests", + Tags.of("spring.cloud.gateway.route.id", "foo", "error", "none", "http.method", "GET", + "http.status_code", "200", "spring.cloud.gateway.route.uri", "http://localhost:8080/")) + .hasMeterWithNameAndTags("http.client.requests.active", Tags.of("spring.cloud.gateway.route.id", + "foo", "http.method", "GET", "spring.cloud.gateway.route.uri", "http://localhost:8080/")); }; }