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

A failure when an instrumented WebClient records metrics causes the request to fail #30978

Closed
ChristianLMI opened this issue May 11, 2022 · 0 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@ChristianLMI
Copy link

I recently ran into an issue with a misconfigured MeterFilter. The filter threw exceptions causing in requests failing. I'm adding the filter like this:

@Bean
public MeterFilter taggingMeterFilter(MetricsProperties props) {
    return new TaggingMeterFilter(props);
}

static class TaggingMeterFilter implements MeterFilter {

I first observed that this is not causing issues (other than a cluttered log) when recording http.server.requests metrics. This is due to the MetricsWebFilter catching Exceptions and logging them away:

private void record(ServerWebExchange exchange, Throwable cause, long start) {
    try {
[...]
        AutoTimer.apply(this.autoTimer, this.metricName, annotations,
                (builder) -> builder.tags(tags).register(this.registry).record(duration, TimeUnit.NANOSECONDS));
    }
    catch (Exception ex) {
        logger.warn("Failed to record timer metrics", ex);
        // Allow exchange to continue, unaffected by metrics problem
    }
}

I then discovered that when the exception occurs in recording http.client.requests this is not handled similarly and I get failed requests. Apparently this is due to code in MetricsWebClientFilterFunction that doesn't do any exception handling.

private void recordTimer(Iterable<Tag> tags, Long startTime) {
    this.autoTimer.builder(this.metricName).tags(tags).description("Timer of WebClient operation")
	.register(this.meterRegistry).record(System.nanoTime() - startTime, TimeUnit.NANOSECONDS);
}

While I will add additional resilience to my code, I'd also suggest to add exception handling here. Any thoughts?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 11, 2022
@wilkinsona wilkinsona changed the title Improve resilience against exceptions thrown in Micrometer A failure when an instrumented WebClient records metrics causes the request to fail May 12, 2022
@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels May 12, 2022
@wilkinsona wilkinsona added this to the 2.5.x milestone May 12, 2022
@scottfrederick scottfrederick self-assigned this May 17, 2022
@scottfrederick scottfrederick modified the milestones: 2.5.x, 2.5.14 May 18, 2022
scottfrederick added a commit that referenced this issue May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants