spring-cloud-commons icon indicating copy to clipboard operation
spring-cloud-commons copied to clipboard

WebClient's URI_TEMPLATE_ATTRIBUTE ignored by MicrometerStatsLoadBalancerLifecycle

Open JaroslawDembek opened this issue 2 years ago • 3 comments

Version: 4.0.4

Problem

When spring.cloud.loadbalancer.stats.micrometer.enabled: true using WebClient with loadbalancing can lead to generation of huge number of metrics when path contains params, e.g. /test/{somePerCallId}/... what eventually with high workload can cause even OOM.

Common approach

Spring metrics collectors for WebClient uses URI_TEMPLATE_ATTRIBUTE (which contains placeholder instead of actual param value) to overcome problem mention above.

Solution:

Make MicrometerStatsLoadBalancerLifecycle URI_TEMPLATE_ATTRIBUTE aware. Now this logic is hidden in LoadBalancerTags utility class.

private static String getPath(RequestData requestData) {
     return requestData.getUrl() != null ? requestData.getUrl().getPath() : UNKNOWN;
}

Possibly MicrometerStatsLoadBalancerLifecycle could be aware of client type.

Alternative:

Set spring.cloud.loadbalancer.stats.micrometer.enabled: false and deliver you own metrics collector as bean implementing LoadBalancerLifecycle to override LoadBalancerTags.getPath with e.g.

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;
}

JaroslawDembek avatar Nov 28 '23 16:11 JaroslawDembek

Hi @JaroslawDembek, thanks for raising the issue. Would you like to submit a PR with the changes? If yes, please keep in mind that since this introduces a possibly breaking behaviour change, then method would need to use a flag to go the old path or the new one, with the flag already being deprecated since we'd transition to the new behaviour as the only one with the next major release. If you don't want to work on the PR, no worries - we'll take care of it.

OlgaMaciaszek avatar Jan 18 '24 14:01 OlgaMaciaszek

I am already using overrided version of metric collector with SpringBoot3 so it shouldn't be a big task. I will jump on it next week.

JaroslawDembek avatar Jan 18 '24 16:01 JaroslawDembek

@OlgaMaciaszek I've created a PR to 4.0.x. I need some advice how to add a feature flag. It looks like a new @ConfigurationProperties is needed.

JaroslawDembek avatar Jan 26 '24 11:01 JaroslawDembek