From 057398859a5a1508b2f2afa95cbbbe097cdbaccf Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Tue, 15 Oct 2024 14:07:05 +0200 Subject: [PATCH] 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 1e609284..8ed90b61 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 @@ package org.springframework.cloud.loadbalancer.config; 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 81c6d769..ba1d873c 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.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. * * @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 @@ final class LoadBalancerTags { RequestData requestData = responseData.getRequestData(); if (requestData != null) { tags = tags.and(valueOrUnknown("method", requestData.getHttpMethod()), - valueOrUnknown("uri", getPath(requestData))); + valueOrUnknown("uri", getPath(requestData, useUriTemplateAttribute))); } else { tags = tags.and(Tag.of("method", UNKNOWN), Tag.of("uri", UNKNOWN)); @@ -69,18 +74,25 @@ final class LoadBalancerTags { return responseData.getHttpStatus() != null ? responseData.getHttpStatus().value() : 200; } - private static String getPath(RequestData requestData) { + private static String getPath(RequestData requestData, boolean useUriTemplateAttribute) { + if (useUriTemplateAttribute && 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; } - static Iterable 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 @@ final class LoadBalancerTags { 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 @@ final class LoadBalancerTags { .getClientRequest(); if (requestData != null) { return tags.and(Tags.of(valueOrUnknown("method", requestData.getHttpMethod()), - valueOrUnknown("uri", getPath(requestData)))); + valueOrUnknown("uri", getPath(requestData, useUriTemplateAttribute)))); } } return tags.and(Tags.of(valueOrUnknown("method", UNKNOWN), valueOrUnknown("uri", UNKNOWN))); 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 ca67721b..ef426b3d 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 @@ -42,16 +42,20 @@ import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.buil * load-balanced calls. * * @author Olga Maciaszek-Sharma + * @author Jaroslaw Dembek * @since 3.0.0 */ public class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecycle { 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 class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecyc 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 class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecyc 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 class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecyc 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 28104c47..a5613db9 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 @@ package org.springframework.cloud.loadbalancer.stats; 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 org.springframework.util.MultiValueMapAdapter; 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 @@ class MicrometerStatsLoadBalancerLifecycleTests { 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(),