From 81ec5432ea08cf3cc7aed2d47c669fe37c840c46 Mon Sep 17 00:00:00 2001 From: JaroslawDembek Date: Wed, 31 Jan 2024 07:40:48 +0100 Subject: [PATCH] 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 8ed90b61..1e609284 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 @@ 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; @@ -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 ba1d873c..a45afc26 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 @@ final class 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 @@ final class LoadBalancerTags { RequestData requestData = responseData.getRequestData(); if (requestData != null) { tags = tags.and(valueOrUnknown("method", requestData.getHttpMethod()), - valueOrUnknown("uri", getPath(requestData, useUriTemplateAttribute))); + valueOrUnknown("uri", getPath(requestData))); } else { tags = tags.and(Tag.of("method", UNKNOWN), Tag.of("uri", UNKNOWN)); @@ -74,8 +73,8 @@ final class LoadBalancerTags { return responseData.getHttpStatus() != null ? responseData.getHttpStatus().value() : 200; } - private static String getPath(RequestData requestData, boolean useUriTemplateAttribute) { - if (useUriTemplateAttribute && requestData.getAttributes() != null) { + private static String getPath(RequestData requestData) { + if (requestData.getAttributes() != null) { var uriTemplate = (String) requestData.getAttributes().get(URI_TEMPLATE_ATTRIBUTE); if (uriTemplate != null) { return uriTemplate; @@ -84,15 +83,14 @@ final class LoadBalancerTags { return requestData.getUrl() != null ? requestData.getUrl().getPath() : UNKNOWN; } - static Iterable 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 @@ final class LoadBalancerTags { 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 @@ final class LoadBalancerTags { .getClientRequest(); if (requestData != null) { return tags.and(Tags.of(valueOrUnknown("method", requestData.getHttpMethod()), - valueOrUnknown("uri", getPath(requestData, useUriTemplateAttribute)))); + valueOrUnknown("uri", getPath(requestData)))); } } 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 ef426b3d..ebebc1e9 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 @@ -49,13 +49,10 @@ public class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecyc private final MeterRegistry meterRegistry; - private final boolean useUriTemplateAttribute; - private final ConcurrentHashMap 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 class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecyc 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 class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecyc 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 class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecyc 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 a5613db9..95613424 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 @@ class MicrometerStatsLoadBalancerLifecycleTests { 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);