From a14d6449e8eafa3da13e8683cdba1836e7c4502f Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Wed, 31 Jul 2024 17:04:56 +0200 Subject: [PATCH 1/8] Reapply "Resolve bean post processor warnings (#1361)" This reverts commit 246bc321c748c76b8f222f7201cc5e5dae487237. --- .../loadbalancer/LoadBalancerAutoConfiguration.java | 8 +++++--- ...oadBalancerBeanPostProcessorAutoConfiguration.java | 11 +++++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java index a2cb39cec..2a1101a6f 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java @@ -37,6 +37,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Lazy; import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.retry.support.RetryTemplate; import org.springframework.web.client.RestTemplate; @@ -49,6 +50,7 @@ * @author Will Tran * @author Gang Li * @author Olga Maciaszek-Sharma + * @author Henning Pöttker */ @AutoConfiguration @Conditional(BlockingRestClassesPresentCondition.class) @@ -86,7 +88,7 @@ static class DeferringLoadBalancerInterceptorConfig { @Bean @ConditionalOnMissingBean - public DeferringLoadBalancerInterceptor deferringLoadBalancerInterceptor( + public static DeferringLoadBalancerInterceptor deferringLoadBalancerInterceptor( ObjectProvider loadBalancerInterceptorObjectProvider) { return new DeferringLoadBalancerInterceptor(loadBalancerInterceptorObjectProvider); } @@ -94,8 +96,8 @@ public DeferringLoadBalancerInterceptor deferringLoadBalancerInterceptor( @Bean @ConditionalOnBean(DeferringLoadBalancerInterceptor.class) @ConditionalOnMissingBean - LoadBalancerRestClientBuilderBeanPostProcessor lbRestClientPostProcessor( - DeferringLoadBalancerInterceptor loadBalancerInterceptor, ApplicationContext context) { + static LoadBalancerRestClientBuilderBeanPostProcessor lbRestClientPostProcessor( + @Lazy DeferringLoadBalancerInterceptor loadBalancerInterceptor, ApplicationContext context) { return new LoadBalancerRestClientBuilderBeanPostProcessor(loadBalancerInterceptor, context); } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerBeanPostProcessorAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerBeanPostProcessorAutoConfiguration.java index 8e61f33c0..3f61c3731 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerBeanPostProcessorAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerBeanPostProcessorAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-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. @@ -27,6 +27,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Lazy; import org.springframework.context.annotation.Primary; import org.springframework.web.reactive.function.client.WebClient; @@ -39,6 +40,7 @@ * beans. * * @author Olga Maciaszek-Sharma + * @author Henning Pöttker * @since 2.2.0 */ @Configuration(proxyBeanMethods = false) @@ -47,8 +49,9 @@ public class LoadBalancerBeanPostProcessorAutoConfiguration { @Bean - public LoadBalancerWebClientBuilderBeanPostProcessor loadBalancerWebClientBuilderBeanPostProcessor( - DeferringLoadBalancerExchangeFilterFunction deferringExchangeFilterFunction, ApplicationContext context) { + public static LoadBalancerWebClientBuilderBeanPostProcessor loadBalancerWebClientBuilderBeanPostProcessor( + @Lazy DeferringLoadBalancerExchangeFilterFunction deferringExchangeFilterFunction, + ApplicationContext context) { return new LoadBalancerWebClientBuilderBeanPostProcessor(deferringExchangeFilterFunction, context); } @@ -58,7 +61,7 @@ protected static class ReactorDeferringLoadBalancerFilterConfig { @Bean @Primary - DeferringLoadBalancerExchangeFilterFunction reactorDeferringLoadBalancerExchangeFilterFunction( + static DeferringLoadBalancerExchangeFilterFunction reactorDeferringLoadBalancerExchangeFilterFunction( ObjectProvider exchangeFilterFunctionProvider) { return new DeferringLoadBalancerExchangeFilterFunction<>(exchangeFilterFunctionProvider); } From f92bdf7df5d8523e71039519ae4bc00f617ffdd0 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Wed, 31 Jul 2024 17:05:27 +0200 Subject: [PATCH 2/8] Reapply "Use ObjectProvider for DeferringLoadBalancerExchangeFilterFunction." This reverts commit c1b08f8b9bcb9a762218b78b9416f6b7ab3fe31a. --- .../loadbalancer/SimpleObjectProvider.java | 60 +++++++++++++++++++ ...ingLoadBalancerExchangeFilterFunction.java | 4 +- ...cerBeanPostProcessorAutoConfiguration.java | 4 +- ...ncerWebClientBuilderBeanPostProcessor.java | 28 +++++++-- 4 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/SimpleObjectProvider.java diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/SimpleObjectProvider.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/SimpleObjectProvider.java new file mode 100644 index 000000000..b12ce9771 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/SimpleObjectProvider.java @@ -0,0 +1,60 @@ +/* + * Copyright 2012-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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.client.loadbalancer; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerWebClientBuilderBeanPostProcessor; + +/** + * Wrapper for {@link ObjectProvider}. Added to use for a workaround in + * {@link LoadBalancerWebClientBuilderBeanPostProcessor}. + * + * @param type of the object to fetch + * @author Spencer Gibb + * @deprecated for removal in 4.0 + */ +@Deprecated(forRemoval = true) +public class SimpleObjectProvider implements ObjectProvider { + + private final T object; + + public SimpleObjectProvider(T object) { + this.object = object; + } + + @Override + public T getObject(Object... args) throws BeansException { + return this.object; + } + + @Override + public T getIfAvailable() throws BeansException { + return this.object; + } + + @Override + public T getIfUnique() throws BeansException { + return this.object; + } + + @Override + public T getObject() throws BeansException { + return this.object; + } + +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/DeferringLoadBalancerExchangeFilterFunction.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/DeferringLoadBalancerExchangeFilterFunction.java index 4e419770f..d370b981d 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/DeferringLoadBalancerExchangeFilterFunction.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/DeferringLoadBalancerExchangeFilterFunction.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-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. @@ -55,7 +55,7 @@ void tryResolveDelegate() { if (delegate == null) { delegate = exchangeFilterFunctionProvider.getIfAvailable(); if (delegate == null) { - throw new IllegalStateException("ReactorLoadBalancerExchangeFilterFunction not available."); + throw new IllegalStateException("LoadBalancerExchangeFilterFunction not available."); } } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerBeanPostProcessorAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerBeanPostProcessorAutoConfiguration.java index 3f61c3731..f755f95d3 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerBeanPostProcessorAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerBeanPostProcessorAutoConfiguration.java @@ -27,7 +27,6 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Lazy; import org.springframework.context.annotation.Primary; import org.springframework.web.reactive.function.client.WebClient; @@ -48,9 +47,10 @@ @Conditional(LoadBalancerBeanPostProcessorAutoConfiguration.OnAnyLoadBalancerImplementationPresentCondition.class) public class LoadBalancerBeanPostProcessorAutoConfiguration { + @SuppressWarnings("rawtypes") @Bean public static LoadBalancerWebClientBuilderBeanPostProcessor loadBalancerWebClientBuilderBeanPostProcessor( - @Lazy DeferringLoadBalancerExchangeFilterFunction deferringExchangeFilterFunction, + ObjectProvider deferringExchangeFilterFunction, ApplicationContext context) { return new LoadBalancerWebClientBuilderBeanPostProcessor(deferringExchangeFilterFunction, context); } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerWebClientBuilderBeanPostProcessor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerWebClientBuilderBeanPostProcessor.java index bb8db3ff5..e07f9590b 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerWebClientBuilderBeanPostProcessor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerWebClientBuilderBeanPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-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. @@ -17,8 +17,10 @@ package org.springframework.cloud.client.loadbalancer.reactive; import org.springframework.beans.BeansException; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.cloud.client.loadbalancer.LoadBalanced; +import org.springframework.cloud.client.loadbalancer.SimpleObjectProvider; import org.springframework.context.ApplicationContext; import org.springframework.web.reactive.function.client.WebClient; @@ -30,15 +32,28 @@ * @author Olga Maciaszek-Sharma * @since 2.2.0 */ +@SuppressWarnings({ "removal", "rawtypes" }) public class LoadBalancerWebClientBuilderBeanPostProcessor implements BeanPostProcessor { - private final DeferringLoadBalancerExchangeFilterFunction exchangeFilterFunction; + private final ObjectProvider exchangeFilterFunctionObjectProvider; private final ApplicationContext context; + /** + * @deprecated in favour of + * {@link LoadBalancerWebClientBuilderBeanPostProcessor#LoadBalancerWebClientBuilderBeanPostProcessor(ObjectProvider, ApplicationContext)} + */ + @Deprecated(forRemoval = true) public LoadBalancerWebClientBuilderBeanPostProcessor( DeferringLoadBalancerExchangeFilterFunction exchangeFilterFunction, ApplicationContext context) { - this.exchangeFilterFunction = exchangeFilterFunction; + this.exchangeFilterFunctionObjectProvider = new SimpleObjectProvider<>(exchangeFilterFunction); + this.context = context; + } + + public LoadBalancerWebClientBuilderBeanPostProcessor( + ObjectProvider exchangeFilterFunction, + ApplicationContext context) { + this.exchangeFilterFunctionObjectProvider = exchangeFilterFunction; this.context = context; } @@ -48,7 +63,12 @@ public Object postProcessBeforeInitialization(Object bean, String beanName) thro if (context.findAnnotationOnBean(beanName, LoadBalanced.class) == null) { return bean; } - ((WebClient.Builder) bean).filter(exchangeFilterFunction); + DeferringLoadBalancerExchangeFilterFunction exchangeFilterFunction = exchangeFilterFunctionObjectProvider + .getIfAvailable(); + if (exchangeFilterFunction == null) { + throw new IllegalStateException("LoadBalancerExchangeFilterFunction not found"); + } + ((WebClient.Builder) bean).filter(exchangeFilterFunctionObjectProvider.getIfAvailable()); } return bean; } From 057398859a5a1508b2f2afa95cbbbe097cdbaccf Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Tue, 15 Oct 2024 14:07:05 +0200 Subject: [PATCH 3/8] Use URI_TEMPLATE_ATTRIBUTE when available with feature flag. --- .../LoadBalancerStatsAutoConfiguration.java | 6 ++- .../loadbalancer/stats/LoadBalancerTags.java | 29 +++++++++++---- .../MicrometerStatsLoadBalancerLifecycle.java | 12 ++++-- ...ometerStatsLoadBalancerLifecycleTests.java | 37 ++++++++++++++++++- 4 files changed, 69 insertions(+), 15 deletions(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java index 1e6092847..8ed90b612 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java @@ -18,6 +18,7 @@ import io.micrometer.core.instrument.MeterRegistry; +import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -38,8 +39,9 @@ public class LoadBalancerStatsAutoConfiguration { @Bean @ConditionalOnBean(MeterRegistry.class) - public MicrometerStatsLoadBalancerLifecycle micrometerStatsLifecycle(MeterRegistry meterRegistry) { - return new MicrometerStatsLoadBalancerLifecycle(meterRegistry); + public MicrometerStatsLoadBalancerLifecycle micrometerStatsLifecycle(MeterRegistry meterRegistry, + @Value("${spring.cloud.loadbalancer.stats.micrometer.use-uri-template-attribute: false}") boolean useUriTemplateAttribute) { + return new MicrometerStatsLoadBalancerLifecycle(meterRegistry, useUriTemplateAttribute); } } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java index 81c6d7690..ba1d873c3 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java @@ -25,22 +25,27 @@ import org.springframework.cloud.client.loadbalancer.RequestDataContext; import org.springframework.cloud.client.loadbalancer.ResponseData; import org.springframework.util.StringUtils; +import org.springframework.web.reactive.function.client.WebClient; /** * Utility class for building metrics tags for load-balanced calls. * * @author Olga Maciaszek-Sharma + * @author Jaroslaw Dembek * @since 3.0.0 */ final class LoadBalancerTags { static final String UNKNOWN = "UNKNOWN"; + static final String URI_TEMPLATE_ATTRIBUTE = WebClient.class.getName() + ".uriTemplate"; + private LoadBalancerTags() { throw new UnsupportedOperationException("Cannot instantiate utility class"); } - static Iterable buildSuccessRequestTags(CompletionContext completionContext) { + static Iterable buildSuccessRequestTags(CompletionContext completionContext, + boolean useUriTemplateAttribute) { ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer(); Tags tags = Tags.of(buildServiceInstanceTags(serviceInstance)); Object clientResponse = completionContext.getClientResponse(); @@ -48,7 +53,7 @@ static Iterable buildSuccessRequestTags(CompletionContext buildDiscardedRequestTags( - CompletionContext completionContext) { + static Iterable buildDiscardedRequestTags(CompletionContext completionContext, + boolean useUriTemplateAttribute) { if (completionContext.getLoadBalancerRequest().getContext() instanceof RequestDataContext) { RequestData requestData = ((RequestDataContext) completionContext.getLoadBalancerRequest().getContext()) .getClientRequest(); if (requestData != null) { return Tags.of(valueOrUnknown("method", requestData.getHttpMethod()), - valueOrUnknown("uri", getPath(requestData)), valueOrUnknown("serviceId", getHost(requestData))); + valueOrUnknown("uri", getPath(requestData, useUriTemplateAttribute)), + valueOrUnknown("serviceId", getHost(requestData))); } } return Tags.of(valueOrUnknown("method", UNKNOWN), valueOrUnknown("uri", UNKNOWN), @@ -92,7 +104,8 @@ private static String getHost(RequestData requestData) { return requestData.getUrl() != null ? requestData.getUrl().getHost() : UNKNOWN; } - static Iterable buildFailedRequestTags(CompletionContext completionContext) { + static Iterable buildFailedRequestTags(CompletionContext completionContext, + boolean useUriTemplateAttribute) { ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer(); Tags tags = Tags.of(buildServiceInstanceTags(serviceInstance)).and(exception(completionContext.getThrowable())); if (completionContext.getLoadBalancerRequest().getContext() instanceof RequestDataContext) { @@ -100,7 +113,7 @@ static Iterable buildFailedRequestTags(CompletionContext { private final MeterRegistry meterRegistry; + private final boolean useUriTemplateAttribute; + private final ConcurrentHashMap activeRequestsPerInstance = new ConcurrentHashMap<>(); - public MicrometerStatsLoadBalancerLifecycle(MeterRegistry meterRegistry) { + public MicrometerStatsLoadBalancerLifecycle(MeterRegistry meterRegistry, boolean useUriTemplateAttribute) { this.meterRegistry = meterRegistry; + this.useUriTemplateAttribute = useUriTemplateAttribute; } @Override @@ -88,7 +92,7 @@ public void onComplete(CompletionContext comple long requestFinishedTimestamp = System.nanoTime(); if (CompletionContext.Status.DISCARD.equals(completionContext.status())) { Counter.builder("loadbalancer.requests.discard") - .tags(buildDiscardedRequestTags(completionContext)) + .tags(buildDiscardedRequestTags(completionContext, useUriTemplateAttribute)) .register(meterRegistry) .increment(); return; @@ -102,7 +106,7 @@ public void onComplete(CompletionContext comple if (requestHasBeenTimed(loadBalancerRequestContext)) { if (CompletionContext.Status.FAILED.equals(completionContext.status())) { Timer.builder("loadbalancer.requests.failed") - .tags(buildFailedRequestTags(completionContext)) + .tags(buildFailedRequestTags(completionContext, useUriTemplateAttribute)) .register(meterRegistry) .record(requestFinishedTimestamp - ((TimedRequestContext) loadBalancerRequestContext).getRequestStartTime(), @@ -110,7 +114,7 @@ public void onComplete(CompletionContext comple return; } Timer.builder("loadbalancer.requests.success") - .tags(buildSuccessRequestTags(completionContext)) + .tags(buildSuccessRequestTags(completionContext, useUriTemplateAttribute)) .register(meterRegistry) .record(requestFinishedTimestamp - ((TimedRequestContext) loadBalancerRequestContext).getRequestStartTime(), diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java index 28104c47c..a5613db93 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java @@ -18,6 +18,7 @@ import java.net.URI; import java.util.HashMap; +import java.util.Map; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; @@ -43,17 +44,23 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.UNKNOWN; +import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.URI_TEMPLATE_ATTRIBUTE; /** * Tests for {@link MicrometerStatsLoadBalancerLifecycle}. * * @author Olga Maciaszek-Sharma + * @author Jaroslaw Dembek */ class MicrometerStatsLoadBalancerLifecycleTests { MeterRegistry meterRegistry = new SimpleMeterRegistry(); - MicrometerStatsLoadBalancerLifecycle statsLifecycle = new MicrometerStatsLoadBalancerLifecycle(meterRegistry); + MicrometerStatsLoadBalancerLifecycle statsLifecycle = new MicrometerStatsLoadBalancerLifecycle(meterRegistry, + false); + + MicrometerStatsLoadBalancerLifecycle statsLifecycleWithUriTemplateAttributeUse = new MicrometerStatsLoadBalancerLifecycle( + meterRegistry, true); @Test void shouldRecordSuccessfulTimedRequest() { @@ -80,6 +87,34 @@ void shouldRecordSuccessfulTimedRequest() { Tag.of("serviceInstance.port", "8080"), Tag.of("status", "200"), Tag.of("uri", "/test")); } + @Test + void shouldRecordSuccessfulTimedRequestWithUriTemplate() { + Map attributes = new HashMap<>(); + String uriTemplate = "/test/{pathParam}/test"; + attributes.put(URI_TEMPLATE_ATTRIBUTE, uriTemplate); + RequestData requestData = new RequestData(HttpMethod.GET, URI.create("http://test.org/test/123/test"), + new HttpHeaders(), new HttpHeaders(), attributes); + Request lbRequest = new DefaultRequest<>(new RequestDataContext(requestData)); + Response lbResponse = new DefaultResponse( + new DefaultServiceInstance("test-1", "test", "test.org", 8080, false, new HashMap<>())); + ResponseData responseData = new ResponseData(HttpStatus.OK, new HttpHeaders(), + new MultiValueMapAdapter<>(new HashMap<>()), requestData); + statsLifecycleWithUriTemplateAttributeUse.onStartRequest(lbRequest, lbResponse); + assertThat(meterRegistry.get("loadbalancer.requests.active").gauge().value()).isEqualTo(1); + + statsLifecycleWithUriTemplateAttributeUse.onComplete( + new CompletionContext<>(CompletionContext.Status.SUCCESS, lbRequest, lbResponse, responseData)); + + assertThat(meterRegistry.getMeters()).hasSize(2); + assertThat(meterRegistry.get("loadbalancer.requests.active").gauge().value()).isEqualTo(0); + assertThat(meterRegistry.get("loadbalancer.requests.success").timers()).hasSize(1); + assertThat(meterRegistry.get("loadbalancer.requests.success").timer().count()).isEqualTo(1); + assertThat(meterRegistry.get("loadbalancer.requests.success").timer().getId().getTags()).contains( + Tag.of("method", "GET"), Tag.of("outcome", "SUCCESS"), Tag.of("serviceId", "test"), + Tag.of("serviceInstance.host", "test.org"), Tag.of("serviceInstance.instanceId", "test-1"), + Tag.of("serviceInstance.port", "8080"), Tag.of("status", "200"), Tag.of("uri", uriTemplate)); + } + @Test void shouldRecordFailedTimedRequest() { RequestData requestData = new RequestData(HttpMethod.GET, URI.create("http://test.org/test"), new HttpHeaders(), From 81ec5432ea08cf3cc7aed2d47c669fe37c840c46 Mon Sep 17 00:00:00 2001 From: JaroslawDembek Date: Wed, 31 Jan 2024 07:40:48 +0100 Subject: [PATCH 4/8] Revert usage of feature flag. --- .../LoadBalancerStatsAutoConfiguration.java | 6 ++---- .../loadbalancer/stats/LoadBalancerTags.java | 21 ++++++++----------- .../MicrometerStatsLoadBalancerLifecycle.java | 12 ++++------- ...ometerStatsLoadBalancerLifecycleTests.java | 10 +++------ 4 files changed, 18 insertions(+), 31 deletions(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java index 8ed90b612..1e6092847 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java @@ -18,7 +18,6 @@ import io.micrometer.core.instrument.MeterRegistry; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -39,9 +38,8 @@ public class LoadBalancerStatsAutoConfiguration { @Bean @ConditionalOnBean(MeterRegistry.class) - public MicrometerStatsLoadBalancerLifecycle micrometerStatsLifecycle(MeterRegistry meterRegistry, - @Value("${spring.cloud.loadbalancer.stats.micrometer.use-uri-template-attribute: false}") boolean useUriTemplateAttribute) { - return new MicrometerStatsLoadBalancerLifecycle(meterRegistry, useUriTemplateAttribute); + public MicrometerStatsLoadBalancerLifecycle micrometerStatsLifecycle(MeterRegistry meterRegistry) { + return new MicrometerStatsLoadBalancerLifecycle(meterRegistry); } } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java index ba1d873c3..a45afc269 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java @@ -44,8 +44,7 @@ private LoadBalancerTags() { throw new UnsupportedOperationException("Cannot instantiate utility class"); } - static Iterable buildSuccessRequestTags(CompletionContext completionContext, - boolean useUriTemplateAttribute) { + static Iterable buildSuccessRequestTags(CompletionContext completionContext) { ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer(); Tags tags = Tags.of(buildServiceInstanceTags(serviceInstance)); Object clientResponse = completionContext.getClientResponse(); @@ -53,7 +52,7 @@ static Iterable buildSuccessRequestTags(CompletionContext buildDiscardedRequestTags(CompletionContext completionContext, - boolean useUriTemplateAttribute) { + static Iterable buildDiscardedRequestTags( + CompletionContext completionContext) { if (completionContext.getLoadBalancerRequest().getContext() instanceof RequestDataContext) { RequestData requestData = ((RequestDataContext) completionContext.getLoadBalancerRequest().getContext()) .getClientRequest(); if (requestData != null) { return Tags.of(valueOrUnknown("method", requestData.getHttpMethod()), - valueOrUnknown("uri", getPath(requestData, useUriTemplateAttribute)), - valueOrUnknown("serviceId", getHost(requestData))); + valueOrUnknown("uri", getPath(requestData)), valueOrUnknown("serviceId", getHost(requestData))); } } return Tags.of(valueOrUnknown("method", UNKNOWN), valueOrUnknown("uri", UNKNOWN), @@ -104,8 +102,7 @@ private static String getHost(RequestData requestData) { return requestData.getUrl() != null ? requestData.getUrl().getHost() : UNKNOWN; } - static Iterable buildFailedRequestTags(CompletionContext completionContext, - boolean useUriTemplateAttribute) { + static Iterable buildFailedRequestTags(CompletionContext completionContext) { ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer(); Tags tags = Tags.of(buildServiceInstanceTags(serviceInstance)).and(exception(completionContext.getThrowable())); if (completionContext.getLoadBalancerRequest().getContext() instanceof RequestDataContext) { @@ -113,7 +110,7 @@ static Iterable buildFailedRequestTags(CompletionContext activeRequestsPerInstance = new ConcurrentHashMap<>(); - public MicrometerStatsLoadBalancerLifecycle(MeterRegistry meterRegistry, boolean useUriTemplateAttribute) { + public MicrometerStatsLoadBalancerLifecycle(MeterRegistry meterRegistry) { this.meterRegistry = meterRegistry; - this.useUriTemplateAttribute = useUriTemplateAttribute; } @Override @@ -92,7 +89,7 @@ public void onComplete(CompletionContext comple long requestFinishedTimestamp = System.nanoTime(); if (CompletionContext.Status.DISCARD.equals(completionContext.status())) { Counter.builder("loadbalancer.requests.discard") - .tags(buildDiscardedRequestTags(completionContext, useUriTemplateAttribute)) + .tags(buildDiscardedRequestTags(completionContext)) .register(meterRegistry) .increment(); return; @@ -105,8 +102,7 @@ public void onComplete(CompletionContext comple Object loadBalancerRequestContext = completionContext.getLoadBalancerRequest().getContext(); if (requestHasBeenTimed(loadBalancerRequestContext)) { if (CompletionContext.Status.FAILED.equals(completionContext.status())) { - Timer.builder("loadbalancer.requests.failed") - .tags(buildFailedRequestTags(completionContext, useUriTemplateAttribute)) + Timer.builder("loadbalancer.requests.failed").tags(buildFailedRequestTags(completionContext)) .register(meterRegistry) .record(requestFinishedTimestamp - ((TimedRequestContext) loadBalancerRequestContext).getRequestStartTime(), @@ -114,7 +110,7 @@ public void onComplete(CompletionContext comple return; } Timer.builder("loadbalancer.requests.success") - .tags(buildSuccessRequestTags(completionContext, useUriTemplateAttribute)) + .tags(buildSuccessRequestTags(completionContext)) .register(meterRegistry) .record(requestFinishedTimestamp - ((TimedRequestContext) loadBalancerRequestContext).getRequestStartTime(), diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java index a5613db93..956134246 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java @@ -56,11 +56,7 @@ class MicrometerStatsLoadBalancerLifecycleTests { MeterRegistry meterRegistry = new SimpleMeterRegistry(); - MicrometerStatsLoadBalancerLifecycle statsLifecycle = new MicrometerStatsLoadBalancerLifecycle(meterRegistry, - false); - - MicrometerStatsLoadBalancerLifecycle statsLifecycleWithUriTemplateAttributeUse = new MicrometerStatsLoadBalancerLifecycle( - meterRegistry, true); + MicrometerStatsLoadBalancerLifecycle statsLifecycle = new MicrometerStatsLoadBalancerLifecycle(meterRegistry); @Test void shouldRecordSuccessfulTimedRequest() { @@ -99,10 +95,10 @@ void shouldRecordSuccessfulTimedRequestWithUriTemplate() { new DefaultServiceInstance("test-1", "test", "test.org", 8080, false, new HashMap<>())); ResponseData responseData = new ResponseData(HttpStatus.OK, new HttpHeaders(), new MultiValueMapAdapter<>(new HashMap<>()), requestData); - statsLifecycleWithUriTemplateAttributeUse.onStartRequest(lbRequest, lbResponse); + statsLifecycle.onStartRequest(lbRequest, lbResponse); assertThat(meterRegistry.get("loadbalancer.requests.active").gauge().value()).isEqualTo(1); - statsLifecycleWithUriTemplateAttributeUse.onComplete( + statsLifecycle.onComplete( new CompletionContext<>(CompletionContext.Status.SUCCESS, lbRequest, lbResponse, responseData)); assertThat(meterRegistry.getMeters()).hasSize(2); From d6904a89be343cac965a70c7349ddf17d5b1b4bf Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Fri, 15 Nov 2024 17:36:59 +0100 Subject: [PATCH 5/8] Handle non-reactive HTTP clients. --- .../loadbalancer/LoadBalancerProperties.java | 47 +++++++++++++++--- .../LoadBalancerStatsAutoConfiguration.java | 9 ++-- .../loadbalancer/stats/LoadBalancerTags.java | 49 ++++++++++++------- .../MicrometerStatsLoadBalancerLifecycle.java | 41 ++++++++++++---- ...ometerStatsLoadBalancerLifecycleTests.java | 9 ++-- 5 files changed, 114 insertions(+), 41 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java index 39a0b1f9c..7da36f9c1 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-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. @@ -33,6 +33,7 @@ import org.springframework.core.env.PropertyResolver; import org.springframework.http.HttpMethod; import org.springframework.util.LinkedCaseInsensitiveMap; +import org.springframework.web.client.RestTemplate; /** * The base configuration bean for Spring Cloud LoadBalancer. @@ -90,10 +91,20 @@ public class LoadBalancerProperties { /** * Properties for - * {@link org.springframework.cloud.loadbalancer.core.SubsetServiceInstanceListSupplier}. + * {@code org.springframework.cloud.loadbalancer.core.SubsetServiceInstanceListSupplier}. */ private Subset subset = new Subset(); + /** + * Enabling X-Forwarded Host and Proto Headers. + */ + private XForwarded xForwarded = new XForwarded(); + + /** + * Properties for LoadBalancer metrics. + */ + private Metrics metrics = new Metrics(); + public HealthCheck getHealthCheck() { return healthCheck; } @@ -134,11 +145,6 @@ public void setHintHeaderName(String hintHeaderName) { this.hintHeaderName = hintHeaderName; } - /** - * Enabling X-Forwarded Host and Proto Headers. - */ - private XForwarded xForwarded = new XForwarded(); - // TODO: fix spelling in a major release public void setxForwarded(XForwarded xForwarded) { this.xForwarded = xForwarded; @@ -164,6 +170,14 @@ public void setCallGetWithRequestOnDelegates(boolean callGetWithRequestOnDelegat this.callGetWithRequestOnDelegates = callGetWithRequestOnDelegates; } + public Metrics getMetrics() { + return metrics; + } + + public void setMetrics(Metrics metrics) { + this.metrics = metrics; + } + public static class StickySession { /** @@ -539,4 +553,23 @@ public void setSize(int size) { } + public static class Metrics { + + /** + * Indicates whether path values should be included in metric tags. When + * {@link RestTemplate} is used to execute load-balanced requests with high + * cardinality paths, setting it to {@code false} is recommended. + */ + private boolean includePath = true; + + public boolean isIncludePath() { + return includePath; + } + + public void setIncludePath(boolean includePath) { + this.includePath = includePath; + } + + } + } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java index 1e6092847..8e7de30de 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerStatsAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-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. @@ -21,6 +21,8 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; import org.springframework.cloud.loadbalancer.stats.MicrometerStatsLoadBalancerLifecycle; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -38,8 +40,9 @@ public class LoadBalancerStatsAutoConfiguration { @Bean @ConditionalOnBean(MeterRegistry.class) - public MicrometerStatsLoadBalancerLifecycle micrometerStatsLifecycle(MeterRegistry meterRegistry) { - return new MicrometerStatsLoadBalancerLifecycle(meterRegistry); + public MicrometerStatsLoadBalancerLifecycle micrometerStatsLifecycle(MeterRegistry meterRegistry, + ReactiveLoadBalancer.Factory loadBalancerFactory) { + return new MicrometerStatsLoadBalancerLifecycle(meterRegistry, loadBalancerFactory); } } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java index a45afc269..55d67412d 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-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. @@ -16,16 +16,21 @@ package org.springframework.cloud.loadbalancer.stats; +import java.util.Collections; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; + import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tags; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.CompletionContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; import org.springframework.cloud.client.loadbalancer.RequestData; import org.springframework.cloud.client.loadbalancer.RequestDataContext; import org.springframework.cloud.client.loadbalancer.ResponseData; import org.springframework.util.StringUtils; -import org.springframework.web.reactive.function.client.WebClient; /** * Utility class for building metrics tags for load-balanced calls. @@ -34,17 +39,22 @@ * @author Jaroslaw Dembek * @since 3.0.0 */ -final class LoadBalancerTags { +class LoadBalancerTags { static final String UNKNOWN = "UNKNOWN"; - static final String URI_TEMPLATE_ATTRIBUTE = WebClient.class.getName() + ".uriTemplate"; + private final LoadBalancerProperties properties; + + // Not using class references in case not in classpath + private static final Set URI_TEMPLATE_ATTRIBUTES = Set.of( + "org.springframework.web.reactive.function.client.WebClient.uriTemplate", + "org.springframework.web.client.RestClient.uriTemplate"); - private LoadBalancerTags() { - throw new UnsupportedOperationException("Cannot instantiate utility class"); + LoadBalancerTags(LoadBalancerProperties properties) { + this.properties = properties; } - static Iterable buildSuccessRequestTags(CompletionContext completionContext) { + Iterable buildSuccessRequestTags(CompletionContext completionContext) { ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer(); Tags tags = Tags.of(buildServiceInstanceTags(serviceInstance)); Object clientResponse = completionContext.getClientResponse(); @@ -73,18 +83,21 @@ private static int statusValue(ResponseData responseData) { return responseData.getHttpStatus() != null ? responseData.getHttpStatus().value() : 200; } - private static String getPath(RequestData requestData) { - if (requestData.getAttributes() != null) { - var uriTemplate = (String) requestData.getAttributes().get(URI_TEMPLATE_ATTRIBUTE); - if (uriTemplate != null) { - return uriTemplate; - } - } - return requestData.getUrl() != null ? requestData.getUrl().getPath() : UNKNOWN; + private String getPath(RequestData requestData) { + Optional uriTemplateValue = Optional.ofNullable(requestData.getAttributes()) + .orElse(Collections.emptyMap()) + .keySet() + .stream() + .filter(URI_TEMPLATE_ATTRIBUTES::contains) + .map(key -> requestData.getAttributes().get(key)) + .filter(Objects::nonNull) + .findAny(); + return uriTemplateValue.map(uriTemplate -> (String) uriTemplate) + .orElseGet(() -> (properties.getMetrics().isIncludePath() && requestData.getUrl() != null) + ? requestData.getUrl().getPath() : UNKNOWN); } - static Iterable buildDiscardedRequestTags( - CompletionContext completionContext) { + Iterable buildDiscardedRequestTags(CompletionContext completionContext) { if (completionContext.getLoadBalancerRequest().getContext() instanceof RequestDataContext) { RequestData requestData = ((RequestDataContext) completionContext.getLoadBalancerRequest().getContext()) .getClientRequest(); @@ -102,7 +115,7 @@ private static String getHost(RequestData requestData) { return requestData.getUrl() != null ? requestData.getUrl().getHost() : UNKNOWN; } - static Iterable buildFailedRequestTags(CompletionContext completionContext) { + Iterable buildFailedRequestTags(CompletionContext completionContext) { ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer(); Tags tags = Tags.of(buildServiceInstanceTags(serviceInstance)).and(exception(completionContext.getThrowable())); if (completionContext.getLoadBalancerRequest().getContext() instanceof RequestDataContext) { diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycle.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycle.java index ebebc1e9f..1d77caead 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycle.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycle.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-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. @@ -27,15 +27,16 @@ import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.CompletionContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancerClientsProperties; import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; +import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; import org.springframework.cloud.client.loadbalancer.Request; import org.springframework.cloud.client.loadbalancer.Response; import org.springframework.cloud.client.loadbalancer.TimedRequestContext; +import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; +import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; -import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.buildDiscardedRequestTags; -import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.buildFailedRequestTags; import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.buildServiceInstanceTags; -import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.buildSuccessRequestTags; /** * An implementation of {@link LoadBalancerLifecycle} that records metrics for @@ -49,10 +50,27 @@ public class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecyc private final MeterRegistry meterRegistry; + private final ReactiveLoadBalancer.Factory loadBalancerFactory; + private final ConcurrentHashMap activeRequestsPerInstance = new ConcurrentHashMap<>(); - public MicrometerStatsLoadBalancerLifecycle(MeterRegistry meterRegistry) { + public MicrometerStatsLoadBalancerLifecycle(MeterRegistry meterRegistry, + ReactiveLoadBalancer.Factory loadBalancerFactory) { this.meterRegistry = meterRegistry; + this.loadBalancerFactory = loadBalancerFactory; + } + + /** + * Creates a MicrometerStatsLoadBalancerLifecycle instance based on the provided + * {@link MeterRegistry}. + * @param meterRegistry {@link MeterRegistry} to use for Micrometer metrics. + * @deprecated in favour of + * {@link MicrometerStatsLoadBalancerLifecycle#MicrometerStatsLoadBalancerLifecycle(MeterRegistry, ReactiveLoadBalancer.Factory)} + */ + @Deprecated(forRemoval = true) + public MicrometerStatsLoadBalancerLifecycle(MeterRegistry meterRegistry) { + // use default properties when calling deprecated constructor + this(meterRegistry, new LoadBalancerClientFactory(new LoadBalancerClientsProperties())); } @Override @@ -86,15 +104,19 @@ public void onStartRequest(Request request, Response lb @Override public void onComplete(CompletionContext completionContext) { + ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer(); + LoadBalancerProperties properties = serviceInstance != null + ? loadBalancerFactory.getProperties(serviceInstance.getServiceId()) + : loadBalancerFactory.getProperties(null); + LoadBalancerTags loadBalancerTags = new LoadBalancerTags(properties); long requestFinishedTimestamp = System.nanoTime(); if (CompletionContext.Status.DISCARD.equals(completionContext.status())) { Counter.builder("loadbalancer.requests.discard") - .tags(buildDiscardedRequestTags(completionContext)) + .tags(loadBalancerTags.buildDiscardedRequestTags(completionContext)) .register(meterRegistry) .increment(); return; } - ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer(); AtomicLong activeRequestsCounter = activeRequestsPerInstance.get(serviceInstance); if (activeRequestsCounter != null) { activeRequestsCounter.decrementAndGet(); @@ -102,7 +124,8 @@ public void onComplete(CompletionContext comple Object loadBalancerRequestContext = completionContext.getLoadBalancerRequest().getContext(); if (requestHasBeenTimed(loadBalancerRequestContext)) { if (CompletionContext.Status.FAILED.equals(completionContext.status())) { - Timer.builder("loadbalancer.requests.failed").tags(buildFailedRequestTags(completionContext)) + Timer.builder("loadbalancer.requests.failed") + .tags(loadBalancerTags.buildFailedRequestTags(completionContext)) .register(meterRegistry) .record(requestFinishedTimestamp - ((TimedRequestContext) loadBalancerRequestContext).getRequestStartTime(), @@ -110,7 +133,7 @@ public void onComplete(CompletionContext comple return; } Timer.builder("loadbalancer.requests.success") - .tags(buildSuccessRequestTags(completionContext)) + .tags(loadBalancerTags.buildSuccessRequestTags(completionContext)) .register(meterRegistry) .record(requestFinishedTimestamp - ((TimedRequestContext) loadBalancerRequestContext).getRequestStartTime(), diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java index 956134246..abfb38d9a 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-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. @@ -44,7 +44,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.UNKNOWN; -import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.URI_TEMPLATE_ATTRIBUTE; /** * Tests for {@link MicrometerStatsLoadBalancerLifecycle}. @@ -54,6 +53,8 @@ */ class MicrometerStatsLoadBalancerLifecycleTests { + private static final String URI_TEMPLATE_ATTRIBUTE = "org.springframework.web.reactive.function.client.WebClient.uriTemplate"; + MeterRegistry meterRegistry = new SimpleMeterRegistry(); MicrometerStatsLoadBalancerLifecycle statsLifecycle = new MicrometerStatsLoadBalancerLifecycle(meterRegistry); @@ -98,8 +99,8 @@ void shouldRecordSuccessfulTimedRequestWithUriTemplate() { statsLifecycle.onStartRequest(lbRequest, lbResponse); assertThat(meterRegistry.get("loadbalancer.requests.active").gauge().value()).isEqualTo(1); - statsLifecycle.onComplete( - new CompletionContext<>(CompletionContext.Status.SUCCESS, lbRequest, lbResponse, responseData)); + statsLifecycle + .onComplete(new CompletionContext<>(CompletionContext.Status.SUCCESS, lbRequest, lbResponse, responseData)); assertThat(meterRegistry.getMeters()).hasSize(2); assertThat(meterRegistry.get("loadbalancer.requests.active").gauge().value()).isEqualTo(0); From 8ddfc232c3b7a88d68d3a11892f41b8d13f8772a Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Mon, 18 Nov 2024 16:52:56 +0100 Subject: [PATCH 6/8] Add tests. --- .../loadbalancer/stats/LoadBalancerTags.java | 5 ++- ...ometerStatsLoadBalancerLifecycleTests.java | 43 +++++++++++++++++-- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java index 55d67412d..e2db81a1f 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java @@ -84,6 +84,9 @@ private static int statusValue(ResponseData responseData) { } private String getPath(RequestData requestData) { + if (!properties.getMetrics().isIncludePath()) { + return UNKNOWN; + } Optional uriTemplateValue = Optional.ofNullable(requestData.getAttributes()) .orElse(Collections.emptyMap()) .keySet() @@ -93,7 +96,7 @@ private String getPath(RequestData requestData) { .filter(Objects::nonNull) .findAny(); return uriTemplateValue.map(uriTemplate -> (String) uriTemplate) - .orElseGet(() -> (properties.getMetrics().isIncludePath() && requestData.getUrl() != null) + .orElseGet(() -> (requestData.getUrl() != null) ? requestData.getUrl().getPath() : UNKNOWN); } diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java index abfb38d9a..62f0c9543 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java @@ -24,6 +24,8 @@ import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; @@ -32,17 +34,21 @@ import org.springframework.cloud.client.loadbalancer.DefaultRequestContext; import org.springframework.cloud.client.loadbalancer.DefaultResponse; import org.springframework.cloud.client.loadbalancer.EmptyResponse; +import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; import org.springframework.cloud.client.loadbalancer.Request; import org.springframework.cloud.client.loadbalancer.RequestData; import org.springframework.cloud.client.loadbalancer.RequestDataContext; import org.springframework.cloud.client.loadbalancer.Response; import org.springframework.cloud.client.loadbalancer.ResponseData; +import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.util.MultiValueMapAdapter; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.UNKNOWN; /** @@ -53,7 +59,8 @@ */ class MicrometerStatsLoadBalancerLifecycleTests { - private static final String URI_TEMPLATE_ATTRIBUTE = "org.springframework.web.reactive.function.client.WebClient.uriTemplate"; + private static final String WEB_CLIENT_URI_TEMPLATE_ATTRIBUTE = "org.springframework.web.reactive.function.client.WebClient.uriTemplate"; + private static final String REST_CLIENT_URI_TEMPLATE_ATTRIBUTE = "org.springframework.web.reactive.function.client.WebClient.uriTemplate"; MeterRegistry meterRegistry = new SimpleMeterRegistry(); @@ -85,10 +92,37 @@ void shouldRecordSuccessfulTimedRequest() { } @Test - void shouldRecordSuccessfulTimedRequestWithUriTemplate() { + void shouldNotAddPathValueWhenDisabled() { + ReactiveLoadBalancer.Factory factory = mock(ReactiveLoadBalancer.Factory.class); + LoadBalancerProperties properties = new LoadBalancerProperties(); + properties.getMetrics().setIncludePath(false); + when(factory.getProperties("test")).thenReturn(properties); + MicrometerStatsLoadBalancerLifecycle statsLifecycle = new MicrometerStatsLoadBalancerLifecycle(meterRegistry, factory); + RequestData requestData = new RequestData(HttpMethod.GET, URI.create("http://test.org/test"), new HttpHeaders(), + new HttpHeaders(), new HashMap<>()); + Request lbRequest = new DefaultRequest<>(new RequestDataContext(requestData)); + Response lbResponse = new DefaultResponse( + new DefaultServiceInstance("test-1", "test", "test.org", 8080, false, new HashMap<>())); + ResponseData responseData = new ResponseData(HttpStatus.OK, new HttpHeaders(), + new MultiValueMapAdapter<>(new HashMap<>()), requestData); + statsLifecycle.onStartRequest(lbRequest, lbResponse); + assertThat(meterRegistry.get("loadbalancer.requests.active").gauge() + .value()).isEqualTo(1); + + statsLifecycle + .onComplete(new CompletionContext<>(CompletionContext.Status.SUCCESS, lbRequest, lbResponse, responseData)); + + assertThat(meterRegistry.getMeters()).hasSize(2); + assertThat(meterRegistry.get("loadbalancer.requests.success").timer().getId() + .getTags()).doesNotContain(Tag.of("uri", "/test")); + } + + @ParameterizedTest + @ValueSource(strings = {WEB_CLIENT_URI_TEMPLATE_ATTRIBUTE, REST_CLIENT_URI_TEMPLATE_ATTRIBUTE}) + void shouldRecordSuccessfulTimedRequestWithUriTemplate(String attributeName) { Map attributes = new HashMap<>(); String uriTemplate = "/test/{pathParam}/test"; - attributes.put(URI_TEMPLATE_ATTRIBUTE, uriTemplate); + attributes.put(attributeName, uriTemplate); RequestData requestData = new RequestData(HttpMethod.GET, URI.create("http://test.org/test/123/test"), new HttpHeaders(), new HttpHeaders(), attributes); Request lbRequest = new DefaultRequest<>(new RequestDataContext(requestData)); @@ -106,7 +140,8 @@ void shouldRecordSuccessfulTimedRequestWithUriTemplate() { assertThat(meterRegistry.get("loadbalancer.requests.active").gauge().value()).isEqualTo(0); assertThat(meterRegistry.get("loadbalancer.requests.success").timers()).hasSize(1); assertThat(meterRegistry.get("loadbalancer.requests.success").timer().count()).isEqualTo(1); - assertThat(meterRegistry.get("loadbalancer.requests.success").timer().getId().getTags()).contains( + assertThat(meterRegistry.get("loadbalancer.requests.success").timer().getId() + .getTags()).containsExactlyInAnyOrder( Tag.of("method", "GET"), Tag.of("outcome", "SUCCESS"), Tag.of("serviceId", "test"), Tag.of("serviceInstance.host", "test.org"), Tag.of("serviceInstance.instanceId", "test-1"), Tag.of("serviceInstance.port", "8080"), Tag.of("status", "200"), Tag.of("uri", uriTemplate)); From 3a259ec8c8933516e30074ac0d24ea48a6a69572 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Mon, 18 Nov 2024 17:15:47 +0100 Subject: [PATCH 7/8] Rename property. Add docs. --- .../spring-cloud-commons/loadbalancer.adoc | 6 +++++ .../loadbalancer/LoadBalancerProperties.java | 12 ++++----- .../loadbalancer/stats/LoadBalancerTags.java | 5 ++-- ...ometerStatsLoadBalancerLifecycleTests.java | 27 ++++++++++--------- 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/docs/modules/ROOT/pages/spring-cloud-commons/loadbalancer.adoc b/docs/modules/ROOT/pages/spring-cloud-commons/loadbalancer.adoc index 2bac92d36..4dbfc4d0f 100644 --- a/docs/modules/ROOT/pages/spring-cloud-commons/loadbalancer.adoc +++ b/docs/modules/ROOT/pages/spring-cloud-commons/loadbalancer.adoc @@ -506,6 +506,12 @@ set the value of the `spring.cloud.loadbalancer.stats.micrometer.enabled` to `tr Additional information regarding the service instances, request data, and response data is added to metrics via tags whenever available. +NOTE: For `WebClient` and `RestClient`-backed load-balancing, we use `uriTemplate` for the `uri` tag whenever available. + +TIP: It is possible to disable adding `uri` tag by setting `spring.cloud.loadbalancer.metrics.include-uri-tag` to `false`. + +WARNING: As with `RestTemplate`-backed load-balancing, we don't have access to `uriTemplate`, full path is always used in the `uri` tag. In order to avoid high cardinality issues, if path is a high cardinality value (for example, `/orders/\{id\}`, where `id` takes a big number of values), it is strongly recommended to disable adding `uri` tag by setting `spring.cloud.loadbalancer.metrics.include-uri-tag` to `false`. + NOTE: For some implementations, such as `BlockingLoadBalancerClient`, request and response data might not be available, as we establish generic types from arguments and might not be able to determine the types and read the data. NOTE: The meters are registered in the registry when at least one record is added for a given meter. diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java index 7da36f9c1..f8769ffe7 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java @@ -556,18 +556,18 @@ public void setSize(int size) { public static class Metrics { /** - * Indicates whether path values should be included in metric tags. When + * Indicates whether the `uri` tag should be added to metrics. When * {@link RestTemplate} is used to execute load-balanced requests with high * cardinality paths, setting it to {@code false} is recommended. */ - private boolean includePath = true; + private boolean includeUriTag = true; - public boolean isIncludePath() { - return includePath; + public boolean isIncludeUriTag() { + return includeUriTag; } - public void setIncludePath(boolean includePath) { - this.includePath = includePath; + public void setIncludeUriTag(boolean includeUriTag) { + this.includeUriTag = includeUriTag; } } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java index e2db81a1f..8f5be8929 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java @@ -84,7 +84,7 @@ private static int statusValue(ResponseData responseData) { } private String getPath(RequestData requestData) { - if (!properties.getMetrics().isIncludePath()) { + if (!properties.getMetrics().isIncludeUriTag()) { return UNKNOWN; } Optional uriTemplateValue = Optional.ofNullable(requestData.getAttributes()) @@ -96,8 +96,7 @@ private String getPath(RequestData requestData) { .filter(Objects::nonNull) .findAny(); return uriTemplateValue.map(uriTemplate -> (String) uriTemplate) - .orElseGet(() -> (requestData.getUrl() != null) - ? requestData.getUrl().getPath() : UNKNOWN); + .orElseGet(() -> (requestData.getUrl() != null) ? requestData.getUrl().getPath() : UNKNOWN); } Iterable buildDiscardedRequestTags(CompletionContext completionContext) { diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java index 62f0c9543..a9bcbb62e 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java @@ -60,6 +60,7 @@ class MicrometerStatsLoadBalancerLifecycleTests { private static final String WEB_CLIENT_URI_TEMPLATE_ATTRIBUTE = "org.springframework.web.reactive.function.client.WebClient.uriTemplate"; + private static final String REST_CLIENT_URI_TEMPLATE_ATTRIBUTE = "org.springframework.web.reactive.function.client.WebClient.uriTemplate"; MeterRegistry meterRegistry = new SimpleMeterRegistry(); @@ -95,9 +96,10 @@ void shouldRecordSuccessfulTimedRequest() { void shouldNotAddPathValueWhenDisabled() { ReactiveLoadBalancer.Factory factory = mock(ReactiveLoadBalancer.Factory.class); LoadBalancerProperties properties = new LoadBalancerProperties(); - properties.getMetrics().setIncludePath(false); + properties.getMetrics().setIncludeUriTag(false); when(factory.getProperties("test")).thenReturn(properties); - MicrometerStatsLoadBalancerLifecycle statsLifecycle = new MicrometerStatsLoadBalancerLifecycle(meterRegistry, factory); + MicrometerStatsLoadBalancerLifecycle statsLifecycle = new MicrometerStatsLoadBalancerLifecycle(meterRegistry, + factory); RequestData requestData = new RequestData(HttpMethod.GET, URI.create("http://test.org/test"), new HttpHeaders(), new HttpHeaders(), new HashMap<>()); Request lbRequest = new DefaultRequest<>(new RequestDataContext(requestData)); @@ -106,19 +108,18 @@ void shouldNotAddPathValueWhenDisabled() { ResponseData responseData = new ResponseData(HttpStatus.OK, new HttpHeaders(), new MultiValueMapAdapter<>(new HashMap<>()), requestData); statsLifecycle.onStartRequest(lbRequest, lbResponse); - assertThat(meterRegistry.get("loadbalancer.requests.active").gauge() - .value()).isEqualTo(1); + assertThat(meterRegistry.get("loadbalancer.requests.active").gauge().value()).isEqualTo(1); statsLifecycle - .onComplete(new CompletionContext<>(CompletionContext.Status.SUCCESS, lbRequest, lbResponse, responseData)); + .onComplete(new CompletionContext<>(CompletionContext.Status.SUCCESS, lbRequest, lbResponse, responseData)); assertThat(meterRegistry.getMeters()).hasSize(2); - assertThat(meterRegistry.get("loadbalancer.requests.success").timer().getId() - .getTags()).doesNotContain(Tag.of("uri", "/test")); + assertThat(meterRegistry.get("loadbalancer.requests.success").timer().getId().getTags()) + .doesNotContain(Tag.of("uri", "/test")); } @ParameterizedTest - @ValueSource(strings = {WEB_CLIENT_URI_TEMPLATE_ATTRIBUTE, REST_CLIENT_URI_TEMPLATE_ATTRIBUTE}) + @ValueSource(strings = { WEB_CLIENT_URI_TEMPLATE_ATTRIBUTE, REST_CLIENT_URI_TEMPLATE_ATTRIBUTE }) void shouldRecordSuccessfulTimedRequestWithUriTemplate(String attributeName) { Map attributes = new HashMap<>(); String uriTemplate = "/test/{pathParam}/test"; @@ -140,11 +141,11 @@ void shouldRecordSuccessfulTimedRequestWithUriTemplate(String attributeName) { assertThat(meterRegistry.get("loadbalancer.requests.active").gauge().value()).isEqualTo(0); assertThat(meterRegistry.get("loadbalancer.requests.success").timers()).hasSize(1); assertThat(meterRegistry.get("loadbalancer.requests.success").timer().count()).isEqualTo(1); - assertThat(meterRegistry.get("loadbalancer.requests.success").timer().getId() - .getTags()).containsExactlyInAnyOrder( - Tag.of("method", "GET"), Tag.of("outcome", "SUCCESS"), Tag.of("serviceId", "test"), - Tag.of("serviceInstance.host", "test.org"), Tag.of("serviceInstance.instanceId", "test-1"), - Tag.of("serviceInstance.port", "8080"), Tag.of("status", "200"), Tag.of("uri", uriTemplate)); + assertThat(meterRegistry.get("loadbalancer.requests.success").timer().getId().getTags()) + .containsExactlyInAnyOrder(Tag.of("method", "GET"), Tag.of("outcome", "SUCCESS"), + Tag.of("serviceId", "test"), Tag.of("serviceInstance.host", "test.org"), + Tag.of("serviceInstance.instanceId", "test-1"), Tag.of("serviceInstance.port", "8080"), + Tag.of("status", "200"), Tag.of("uri", uriTemplate)); } @Test From d5d2b78be1cca82afae2b2d34c44fe028b73a0ce Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Mon, 18 Nov 2024 17:51:26 +0100 Subject: [PATCH 8/8] Rename property. --- .../spring-cloud-commons/loadbalancer.adoc | 4 +-- docs/modules/ROOT/partials/_configprops.adoc | 1 + .../loadbalancer/LoadBalancerProperties.java | 28 +++++++++---------- .../loadbalancer/stats/LoadBalancerTags.java | 2 +- ...ometerStatsLoadBalancerLifecycleTests.java | 2 +- 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/docs/modules/ROOT/pages/spring-cloud-commons/loadbalancer.adoc b/docs/modules/ROOT/pages/spring-cloud-commons/loadbalancer.adoc index 4dbfc4d0f..6468a80f4 100644 --- a/docs/modules/ROOT/pages/spring-cloud-commons/loadbalancer.adoc +++ b/docs/modules/ROOT/pages/spring-cloud-commons/loadbalancer.adoc @@ -508,9 +508,9 @@ Additional information regarding the service instances, request data, and respon NOTE: For `WebClient` and `RestClient`-backed load-balancing, we use `uriTemplate` for the `uri` tag whenever available. -TIP: It is possible to disable adding `uri` tag by setting `spring.cloud.loadbalancer.metrics.include-uri-tag` to `false`. +TIP: It is possible to disable adding `path` to `uri` tag by setting `spring.cloud.loadbalancer.stats.include-path` to `false`. -WARNING: As with `RestTemplate`-backed load-balancing, we don't have access to `uriTemplate`, full path is always used in the `uri` tag. In order to avoid high cardinality issues, if path is a high cardinality value (for example, `/orders/\{id\}`, where `id` takes a big number of values), it is strongly recommended to disable adding `uri` tag by setting `spring.cloud.loadbalancer.metrics.include-uri-tag` to `false`. +WARNING: As with `RestTemplate`-backed load-balancing, we don't have access to `uriTemplate`, full path is always used in the `uri` tag. In order to avoid high cardinality issues, if path is a high cardinality value (for example, `/orders/\{id\}`, where `id` takes a big number of values), it is strongly recommended to disable adding path to `uri` tag by setting `spring.cloud.loadbalancer.stats.include-path` to `false`. NOTE: For some implementations, such as `BlockingLoadBalancerClient`, request and response data might not be available, as we establish generic types from arguments and might not be able to determine the types and read the data. diff --git a/docs/modules/ROOT/partials/_configprops.adoc b/docs/modules/ROOT/partials/_configprops.adoc index 0f7da897f..7d261e798 100644 --- a/docs/modules/ROOT/partials/_configprops.adoc +++ b/docs/modules/ROOT/partials/_configprops.adoc @@ -66,6 +66,7 @@ |spring.cloud.loadbalancer.retry.retryable-exceptions | `+++{}+++` | A `Set` of `Throwable` classes that should trigger a retry. |spring.cloud.loadbalancer.retry.retryable-status-codes | `+++{}+++` | A `Set` of status codes that should trigger a retry. |spring.cloud.loadbalancer.service-discovery.timeout | | String representation of Duration of the timeout for calls to service discovery. +|spring.cloud.loadbalancer.stats.include-path | `+++true+++` | Indicates whether the {@code path} should be added to {@code uri} tag in metrics. When {@link RestTemplate} is used to execute load-balanced requests with high cardinality paths, setting it to {@code false} is recommended. |spring.cloud.loadbalancer.stats.micrometer.enabled | `+++false+++` | Enables Spring Cloud LoadBalancer Micrometer stats. |spring.cloud.loadbalancer.sticky-session.add-service-instance-cookie | `+++false+++` | Indicates whether a cookie with the newly selected instance should be added by LoadBalancer. |spring.cloud.loadbalancer.sticky-session.instance-id-cookie-name | `+++sc-lb-instance-id+++` | The name of the cookie holding the preferred instance id. diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java index f8769ffe7..49a0d80a5 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java @@ -103,7 +103,7 @@ public class LoadBalancerProperties { /** * Properties for LoadBalancer metrics. */ - private Metrics metrics = new Metrics(); + private Stats stats = new Stats(); public HealthCheck getHealthCheck() { return healthCheck; @@ -170,12 +170,12 @@ public void setCallGetWithRequestOnDelegates(boolean callGetWithRequestOnDelegat this.callGetWithRequestOnDelegates = callGetWithRequestOnDelegates; } - public Metrics getMetrics() { - return metrics; + public Stats getStats() { + return stats; } - public void setMetrics(Metrics metrics) { - this.metrics = metrics; + public void setStats(Stats stats) { + this.stats = stats; } public static class StickySession { @@ -553,21 +553,21 @@ public void setSize(int size) { } - public static class Metrics { + public static class Stats { /** - * Indicates whether the `uri` tag should be added to metrics. When - * {@link RestTemplate} is used to execute load-balanced requests with high - * cardinality paths, setting it to {@code false} is recommended. + * Indicates whether the {@code path} should be added to {@code uri} tag in + * metrics. When {@link RestTemplate} is used to execute load-balanced requests + * with high cardinality paths, setting it to {@code false} is recommended. */ - private boolean includeUriTag = true; + private boolean includePath = true; - public boolean isIncludeUriTag() { - return includeUriTag; + public boolean isIncludePath() { + return includePath; } - public void setIncludeUriTag(boolean includeUriTag) { - this.includeUriTag = includeUriTag; + public void setIncludePath(boolean includePath) { + this.includePath = includePath; } } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java index 8f5be8929..42afc64f7 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java @@ -84,7 +84,7 @@ private static int statusValue(ResponseData responseData) { } private String getPath(RequestData requestData) { - if (!properties.getMetrics().isIncludeUriTag()) { + if (!properties.getStats().isIncludePath()) { return UNKNOWN; } Optional uriTemplateValue = Optional.ofNullable(requestData.getAttributes()) diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java index a9bcbb62e..27d360dd7 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java @@ -96,7 +96,7 @@ void shouldRecordSuccessfulTimedRequest() { void shouldNotAddPathValueWhenDisabled() { ReactiveLoadBalancer.Factory factory = mock(ReactiveLoadBalancer.Factory.class); LoadBalancerProperties properties = new LoadBalancerProperties(); - properties.getMetrics().setIncludeUriTag(false); + properties.getStats().setIncludePath(false); when(factory.getProperties("test")).thenReturn(properties); MicrometerStatsLoadBalancerLifecycle statsLifecycle = new MicrometerStatsLoadBalancerLifecycle(meterRegistry, factory);