From d6904a89be343cac965a70c7349ddf17d5b1b4bf Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Fri, 15 Nov 2024 17:36:59 +0100 Subject: [PATCH] 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 39a0b1f9..7da36f9c 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 Metrics metrics = new Metrics(); + 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 Metrics getMetrics() { + return metrics; + } + + public void setMetrics(Metrics metrics) { + this.metrics = metrics; + } + public static class StickySession { /** @@ -539,4 +553,23 @@ public class LoadBalancerProperties { } + 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 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 a45afc26..55d67412 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 @@ import org.springframework.web.reactive.function.client.WebClient; * @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; - private LoadBalancerTags() { - throw new UnsupportedOperationException("Cannot instantiate utility class"); + // 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(); @@ -73,18 +83,21 @@ final class LoadBalancerTags { 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 @@ 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 ebebc1e9..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,15 +27,16 @@ 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 @@ -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 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 +124,8 @@ 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)) + Timer.builder("loadbalancer.requests.failed") + .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 95613424..abfb38d9 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 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}. @@ -54,6 +53,8 @@ import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.URI_ */ 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 @@ class MicrometerStatsLoadBalancerLifecycleTests { 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);