Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebClient's URI_TEMPLATE_ATTRIBUTE ignored by MicrometerStatsLoadBalancerLifecycle #1302

Closed
JaroslawDembek opened this issue Nov 28, 2023 · 3 comments · Fixed by #1422 · May be fixed by #1328
Closed

WebClient's URI_TEMPLATE_ATTRIBUTE ignored by MicrometerStatsLoadBalancerLifecycle #1302

JaroslawDembek opened this issue Nov 28, 2023 · 3 comments · Fixed by #1422 · May be fixed by #1328
Assignees
Labels
Milestone

Comments

@JaroslawDembek
Copy link

JaroslawDembek commented Nov 28, 2023

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;
}
@OlgaMaciaszek
Copy link
Collaborator

OlgaMaciaszek commented Jan 18, 2024

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.

@JaroslawDembek
Copy link
Author

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 pushed a commit to JaroslawDembek/spring-cloud-commons that referenced this issue Jan 26, 2024
@JaroslawDembek
Copy link
Author

JaroslawDembek commented Jan 26, 2024

@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.

@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2024.0.0-RC1 Oct 15, 2024
@OlgaMaciaszek OlgaMaciaszek added this to the 4.2.0-RC1 milestone Oct 15, 2024
@spencergibb spencergibb removed this from the 4.2.0-RC1 milestone Nov 15, 2024
@spencergibb spencergibb moved this to In Progress in 2024.0.0 Nov 15, 2024
@OlgaMaciaszek OlgaMaciaszek added this to the 4.2.0 milestone Nov 19, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in 2024.0.0 Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment