Tolerate failures when recording WebClient metrics

Fixes gh-30978

Author: scottfrederick

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,13 +40,16 @@
  *
  * @author Brian Clozel
  * @author Tadaya Tsuyukubo
+ * @author Scott Frederick
  * @since 2.1.0
  */
 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<ClientResponse> instrumentResponse(ClientRequest request, Mono<Clie
 		return Mono.deferContextual((ctx) -> responseMono.doOnEach((signal) -> {
 			if (signal.isOnNext() || signal.isOnError()) {
 				responseReceived.set(true);
-				Iterable<Tag> 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<Tag> tags = this.tagProvider.tags(request, null, null);
-				recordTimer(tags, getStartTime(ctx));
+				recordTimer(request, null, null, getStartTime(ctx));
 			}
 		}));
 	}
 
-	private void recordTimer(Iterable<Tag> 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<Tag> 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) {

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,15 +61,17 @@ class MetricsWebClientFilterFunctionTests {
 
 	private MetricsWebClientFilterFunction filterFunction;
 
+	private final FaultyTagsProvider tagsProvider = new FaultyTagsProvider();
+
 	private ClientResponse response;
 
 	private ExchangeFunction exchange;
 
 	@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<Tag> 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);
+		}
+
+	}
+
 }