From 50e27333d2446d0266bd3a50badff365995a09ab Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Tue, 17 May 2022 18:45:37 -0500 Subject: [PATCH] Tolerate failures when recording WebClient metrics Fixes gh-30978 --- .../MetricsWebClientFilterFunction.java | 26 +++++++++---- .../MetricsWebClientFilterFunctionTests.java | 38 +++++++++++++++++-- 2 files changed, 53 insertions(+), 11 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunction.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunction.java index 836dd6548473..17f258ffb56d 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunction.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunction.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2022 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. @@ -21,6 +21,8 @@ import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; import reactor.core.publisher.SignalType; import reactor.util.context.Context; @@ -38,6 +40,7 @@ * * @author Brian Clozel * @author Tadaya Tsuyukubo + * @author Scott Frederick * @since 2.1.0 */ public class MetricsWebClientFilterFunction implements ExchangeFilterFunction { @@ -45,6 +48,8 @@ public class MetricsWebClientFilterFunction implements ExchangeFilterFunction { private static final String METRICS_WEBCLIENT_START_TIME = MetricsWebClientFilterFunction.class.getName() + ".START_TIME"; + private static final Log logger = LogFactory.getLog(MetricsWebClientFilterFunction.class); + private final MeterRegistry meterRegistry; private final WebClientExchangeTagsProvider tagProvider; @@ -83,20 +88,25 @@ private Mono instrumentResponse(ClientRequest request, Mono responseMono.doOnEach((signal) -> { if (signal.isOnNext() || signal.isOnError()) { responseReceived.set(true); - Iterable tags = this.tagProvider.tags(request, signal.get(), signal.getThrowable()); - recordTimer(tags, getStartTime(ctx)); + recordTimer(request, signal.get(), signal.getThrowable(), getStartTime(ctx)); } }).doFinally((signalType) -> { if (!responseReceived.get() && SignalType.CANCEL.equals(signalType)) { - Iterable tags = this.tagProvider.tags(request, null, null); - recordTimer(tags, getStartTime(ctx)); + recordTimer(request, null, null, getStartTime(ctx)); } })); } - private void recordTimer(Iterable tags, Long startTime) { - this.autoTimer.builder(this.metricName).tags(tags).description("Timer of WebClient operation") - .register(this.meterRegistry).record(System.nanoTime() - startTime, TimeUnit.NANOSECONDS); + private void recordTimer(ClientRequest request, ClientResponse response, Throwable error, Long startTime) { + try { + Iterable tags = this.tagProvider.tags(request, response, error); + this.autoTimer.builder(this.metricName).tags(tags).description("Timer of WebClient operation") + .register(this.meterRegistry).record(System.nanoTime() - startTime, TimeUnit.NANOSECONDS); + } + catch (Exception ex) { + logger.warn("Failed to record timer metrics", ex); + // Allow request-response exchange to continue, unaffected by metrics problem + } } private Long getStartTime(ContextView context) { diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunctionTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunctionTests.java index b651560ec57c..ffc17f49fe6c 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunctionTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunctionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2022 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. @@ -20,9 +20,11 @@ import java.net.URI; import java.time.Duration; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.MockClock; +import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.search.MeterNotFoundException; import io.micrometer.core.instrument.simple.SimpleConfig; @@ -49,6 +51,7 @@ * Tests for {@link MetricsWebClientFilterFunction} * * @author Brian Clozel + * @author Scott Frederick */ class MetricsWebClientFilterFunctionTests { @@ -58,6 +61,8 @@ class MetricsWebClientFilterFunctionTests { private MetricsWebClientFilterFunction filterFunction; + private final FaultyTagsProvider tagsProvider = new FaultyTagsProvider(); + private ClientResponse response; private ExchangeFunction exchange; @@ -65,8 +70,8 @@ class MetricsWebClientFilterFunctionTests { @BeforeEach void setup() { this.registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock()); - this.filterFunction = new MetricsWebClientFilterFunction(this.registry, - new DefaultWebClientExchangeTagsProvider(), "http.client.requests", AutoTimer.ENABLED); + this.filterFunction = new MetricsWebClientFilterFunction(this.registry, this.tagsProvider, + "http.client.requests", AutoTimer.ENABLED); this.response = mock(ClientResponse.class); this.exchange = (r) -> Mono.just(this.response); } @@ -159,4 +164,31 @@ void filterWhenExceptionAndRetryShouldNotAccumulateRecordTime() { assertThat(timer.max(TimeUnit.MILLISECONDS)).isLessThan(2000); } + @Test + void whenMetricsRecordingFailsThenFilteringSucceeds() { + ClientRequest request = ClientRequest + .create(HttpMethod.GET, URI.create("https://example.com/projects/spring-boot")).build(); + given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value()); + this.tagsProvider.failOnce(); + this.filterFunction.filter(request, this.exchange).block(Duration.ofSeconds(5)); + } + + static class FaultyTagsProvider extends DefaultWebClientExchangeTagsProvider { + + private final AtomicBoolean fail = new AtomicBoolean(false); + + @Override + public Iterable tags(ClientRequest request, ClientResponse response, Throwable throwable) { + if (this.fail.compareAndSet(true, false)) { + throw new RuntimeException(); + } + return super.tags(request, response, throwable); + } + + void failOnce() { + this.fail.set(true); + } + + } + }