diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index b44059e7..8de18274 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -17,7 +17,7 @@ given the ability to merge pull requests. ## Code of Conduct This project adheres to the Contributor Covenant [code of -conduct](https://github.com/spring-cloud/spring-cloud-build/blob/main/docs/src/main/asciidoc/code-of-conduct.adoc). By participating, you are expected to uphold this code. Please report +conduct](https://github.com/spring-cloud/spring-cloud-build/blob/main/docs/modules/ROOT/partials/code-of-conduct.adoc). By participating, you are expected to uphold this code. Please report unacceptable behavior to spring-code-of-conduct@pivotal.io. ## Code Conventions and Housekeeping diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 7979a5c5..9d5357d8 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -31,7 +31,7 @@ jobs: - name: Build with Maven run: ./mvnw -s .settings.xml clean org.jacoco:jacoco-maven-plugin:prepare-agent install -U -P sonar -nsu --batch-mode -Dmaven.test.redirectTestOutputToFile=true -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn - name: Publish Test Report - uses: mikepenz/action-junit-report@v4 + uses: mikepenz/action-junit-report@v5 if: always() # always run even if the previous step fails with: report_paths: '**/surefire-reports/TEST-*.xml' diff --git a/README.adoc b/README.adoc index 97139044..03410549 100644 --- a/README.adoc +++ b/README.adoc @@ -121,9 +121,9 @@ given the ability to merge pull requests. [[code-of-conduct]] == Code of Conduct -This project adheres to the Contributor Covenant https://github.com/spring-cloud/spring-cloud-build/blob/main/docs/src/main/asciidoc/code-of-conduct.adoc[code of +This project adheres to the Contributor Covenant https://github.com/spring-cloud/spring-cloud-build/blob/main/docs/modules/ROOT/partials/code-of-conduct.adoc[code of conduct]. By participating, you are expected to uphold this code. Please report -unacceptable behavior to spring-code-of-conduct@pivotal.io. +unacceptable behavior to code-of-conduct@spring.io. [[code-conventions-and-housekeeping]] == Code Conventions and Housekeeping diff --git a/docs/antora.yml b/docs/antora.yml index ab1a8894..59ebef94 100644 --- a/docs/antora.yml +++ b/docs/antora.yml @@ -6,7 +6,7 @@ nav: ext: collector: run: - command: ./mvnw --no-transfer-progress -B process-resources -Pdocs -pl docs -Dantora-maven-plugin.phase=none -Dgenerate-docs.phase=none -Dgenerate-readme.phase=none -Dgenerate-cloud-resources.phase=none -Dmaven-dependency-plugin-for-docs.phase=none -Dmaven-dependency-plugin-for-docs-classes.phase=none -DskipTests + command: ./mvnw --no-transfer-progress -B process-resources -Pdocs -pl docs -Dantora-maven-plugin.phase=none -Dgenerate-docs.phase=none -Dgenerate-readme.phase=none -Dgenerate-cloud-resources.phase=none -Dmaven-dependency-plugin-for-docs.phase=none -Dmaven-dependency-plugin-for-docs-classes.phase=none -DskipTests -DdisableConfigurationProperties local: true scan: dir: ./target/classes/antora-resources/ diff --git a/docs/modules/ROOT/pages/spring-cloud-commons/loadbalancer.adoc b/docs/modules/ROOT/pages/spring-cloud-commons/loadbalancer.adoc index 2bac92d3..6468a80f 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 `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 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. NOTE: The meters are registered in the registry when at least one record is added for a given meter. diff --git a/docs/modules/ROOT/partials/_configprops.adoc b/docs/modules/ROOT/partials/_configprops.adoc index 0f7da897..7d261e79 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/docs/package.json b/docs/package.json index 1c8155de..567c1f3a 100644 --- a/docs/package.json +++ b/docs/package.json @@ -1,8 +1,8 @@ { "dependencies": { - "antora": "3.2.0-alpha.6", + "antora": "3.2.0-alpha.8", "@antora/atlas-extension": "1.0.0-alpha.2", - "@antora/collector-extension": "1.0.0-beta.3", + "@antora/collector-extension": "1.0.1", "@asciidoctor/tabs": "1.0.0-beta.6", "@springio/antora-extensions": "1.14.2", "@springio/asciidoctor-extensions": "1.0.0-alpha.14" diff --git a/docs/pom.xml b/docs/pom.xml index 655955a7..e9f98a28 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -8,7 +8,7 @@ org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT jar Spring Cloud Commons Docs @@ -26,20 +26,28 @@ .* ${maven.multiModuleProjectDirectory}/docs/modules/ROOT/partials/ - - - ${project.groupId} - spring-cloud-starter - - - ${project.groupId} - spring-cloud-starter-loadbalancer - - src/main/asciidoc + + enable-configuration-properties + + + !disableConfigurationProperties + + + + + ${project.groupId} + spring-cloud-starter + + + ${project.groupId} + spring-cloud-starter-loadbalancer + + + docs diff --git a/pom.xml b/pom.xml index b8a53a12..8b6f2781 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ 4.0.0 org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT pom Spring Cloud Commons Parent Spring Cloud Commons Parent @@ -13,7 +13,7 @@ org.springframework.cloud spring-cloud-build - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT diff --git a/spring-cloud-commons-dependencies/pom.xml b/spring-cloud-commons-dependencies/pom.xml index c6399daa..af72a3c7 100644 --- a/spring-cloud-commons-dependencies/pom.xml +++ b/spring-cloud-commons-dependencies/pom.xml @@ -6,11 +6,11 @@ spring-cloud-dependencies-parent org.springframework.cloud - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT spring-cloud-commons-dependencies - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT pom spring-cloud-commons-dependencies Spring Cloud Commons Dependencies diff --git a/spring-cloud-commons/pom.xml b/spring-cloud-commons/pom.xml index 64d61129..a0c55a7c 100644 --- a/spring-cloud-commons/pom.xml +++ b/spring-cloud-commons/pom.xml @@ -7,7 +7,7 @@ org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT .. spring-cloud-commons 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 39a0b1f9..49a0d80a 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.cloud.commons.util.IdUtils; 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 Stats stats = new Stats(); + public HealthCheck getHealthCheck() { return healthCheck; } @@ -134,11 +145,6 @@ public class LoadBalancerProperties { 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 class LoadBalancerProperties { this.callGetWithRequestOnDelegates = callGetWithRequestOnDelegates; } + public Stats getStats() { + return stats; + } + + public void setStats(Stats stats) { + this.stats = stats; + } + public static class StickySession { /** @@ -539,4 +553,23 @@ public class LoadBalancerProperties { } + public static class Stats { + + /** + * 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 includePath = true; + + public boolean isIncludePath() { + return includePath; + } + + public void setIncludePath(boolean includePath) { + this.includePath = includePath; + } + + } + } diff --git a/spring-cloud-context-integration-tests/pom.xml b/spring-cloud-context-integration-tests/pom.xml index 00477adf..4d29611b 100644 --- a/spring-cloud-context-integration-tests/pom.xml +++ b/spring-cloud-context-integration-tests/pom.xml @@ -7,7 +7,7 @@ org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT .. spring-cloud-context-integration-tests diff --git a/spring-cloud-context-webflux-integration-tests/pom.xml b/spring-cloud-context-webflux-integration-tests/pom.xml index 68986af6..84e83679 100644 --- a/spring-cloud-context-webflux-integration-tests/pom.xml +++ b/spring-cloud-context-webflux-integration-tests/pom.xml @@ -7,7 +7,7 @@ org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT .. spring-cloud-context-webflux-integration-tests diff --git a/spring-cloud-context/pom.xml b/spring-cloud-context/pom.xml index 06db6b04..8eeb6c06 100644 --- a/spring-cloud-context/pom.xml +++ b/spring-cloud-context/pom.xml @@ -7,7 +7,7 @@ org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT .. spring-cloud-context diff --git a/spring-cloud-loadbalancer/pom.xml b/spring-cloud-loadbalancer/pom.xml index 17051040..f8a0d717 100644 --- a/spring-cloud-loadbalancer/pom.xml +++ b/spring-cloud-loadbalancer/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT .. spring-cloud-loadbalancer 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..8e7de30d 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 io.micrometer.core.instrument.MeterRegistry; 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 81c6d769..42afc64f 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,11 +16,17 @@ 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; @@ -30,17 +36,25 @@ import org.springframework.util.StringUtils; * Utility class for building metrics tags for load-balanced calls. * * @author Olga Maciaszek-Sharma + * @author Jaroslaw Dembek * @since 3.0.0 */ -final class LoadBalancerTags { +class LoadBalancerTags { static final String UNKNOWN = "UNKNOWN"; - private LoadBalancerTags() { - throw new UnsupportedOperationException("Cannot instantiate utility class"); + 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"); + + 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(); @@ -69,12 +83,23 @@ final class LoadBalancerTags { return responseData.getHttpStatus() != null ? responseData.getHttpStatus().value() : 200; } - private static String getPath(RequestData requestData) { - return requestData.getUrl() != null ? requestData.getUrl().getPath() : UNKNOWN; + private String getPath(RequestData requestData) { + if (!properties.getStats().isIncludePath()) { + return UNKNOWN; + } + 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(() -> (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(); @@ -92,7 +117,7 @@ final class LoadBalancerTags { 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 ca67721b..1d77caea 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,31 +27,50 @@ import io.micrometer.core.instrument.Timer; 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 * 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 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 @@ -85,15 +104,19 @@ public class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecyc @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 +125,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(loadBalancerTags.buildFailedRequestTags(completionContext)) .register(meterRegistry) .record(requestFinishedTimestamp - ((TimedRequestContext) loadBalancerRequestContext).getRequestStartTime(), @@ -110,7 +133,7 @@ public class MicrometerStatsLoadBalancerLifecycle implements LoadBalancerLifecyc 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 28104c47..27d360dd 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. @@ -18,11 +18,14 @@ 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; 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; @@ -31,26 +34,35 @@ import org.springframework.cloud.client.loadbalancer.DefaultRequest; 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; /** * Tests for {@link MicrometerStatsLoadBalancerLifecycle}. * * @author Olga Maciaszek-Sharma + * @author Jaroslaw Dembek */ 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(); MicrometerStatsLoadBalancerLifecycle statsLifecycle = new MicrometerStatsLoadBalancerLifecycle(meterRegistry); @@ -80,6 +92,62 @@ class MicrometerStatsLoadBalancerLifecycleTests { Tag.of("serviceInstance.port", "8080"), Tag.of("status", "200"), Tag.of("uri", "/test")); } + @Test + void shouldNotAddPathValueWhenDisabled() { + ReactiveLoadBalancer.Factory factory = mock(ReactiveLoadBalancer.Factory.class); + LoadBalancerProperties properties = new LoadBalancerProperties(); + properties.getStats().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(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)); + 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.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)); + } + @Test void shouldRecordFailedTimedRequest() { RequestData requestData = new RequestData(HttpMethod.GET, URI.create("http://test.org/test"), new HttpHeaders(), diff --git a/spring-cloud-starter-bootstrap/pom.xml b/spring-cloud-starter-bootstrap/pom.xml index 0d641e77..9394e2c9 100644 --- a/spring-cloud-starter-bootstrap/pom.xml +++ b/spring-cloud-starter-bootstrap/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT .. jar @@ -38,4 +38,4 @@ test - \ No newline at end of file + diff --git a/spring-cloud-starter-loadbalancer/pom.xml b/spring-cloud-starter-loadbalancer/pom.xml index f030aede..01f9b82e 100644 --- a/spring-cloud-starter-loadbalancer/pom.xml +++ b/spring-cloud-starter-loadbalancer/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT .. 4.0.0 @@ -40,4 +40,4 @@ ${evictor.version} - \ No newline at end of file + diff --git a/spring-cloud-starter/pom.xml b/spring-cloud-starter/pom.xml index cf3a89c4..3eebcede 100644 --- a/spring-cloud-starter/pom.xml +++ b/spring-cloud-starter/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT spring-cloud-starter spring-cloud-starter diff --git a/spring-cloud-test-support/pom.xml b/spring-cloud-test-support/pom.xml index c4a6f3b2..8f78edeb 100644 --- a/spring-cloud-test-support/pom.xml +++ b/spring-cloud-test-support/pom.xml @@ -7,7 +7,7 @@ org.springframework.cloud spring-cloud-commons-parent - 4.2.0-SNAPSHOT + 4.2.1-SNAPSHOT .. spring-cloud-test-support