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

Document that Micrometer's "error" tag should be preferred vs. legacy "exception" tag #31514

Closed
chicobento opened this issue Dec 21, 2022 · 5 comments
Assignees
Labels
theme: observability An issue related to observability and tracing type: documentation A documentation task
Milestone

Comments

@chicobento
Copy link

chicobento commented Dec 21, 2022

On Spring Boot 3 besides exception, method, outcome, status and uri, the error tag is also being generated:

http_server_requests_count{error="none" ... }

This tag is not on official docs neither ServerHttpObservationDocumentation class and overlaps with exception tag.
Looking at the code it seems that this behavior is added by micrometer's DefaultMeterObservationHandler which is configured by ObservationAutoConfiguration.

A possible quick-fix would be to override DefaultMeterObservationHandler#createErrorTags to return Tags.empty().

On a side note, an additional undocumented behavior added by DefaultMeterObservationHandler is the http.server.requests.active metrics. Should this be documented ?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 21, 2022
@chicobento
Copy link
Author

Thread on micrometer slack on this issue: https://micrometer-metrics.slack.com/archives/C030GTHE4P6/p1672670045511959

@bclozel
Copy link
Member

bclozel commented Jan 4, 2023

I agree with @jonatan-ivanov 's assessment in this Slack thread.

I see 3 solutions for this:

  1. Removing the "extra" exception tag contributied by Spring MVC. This would break a lot of existing applications and dashboards.
  2. Removing the error tag contributed by Micrometer itself, since we're using the Micrometer Observation API. This doesn't make any sense as Micrometer can be used without Spring and should enforce such common tags
  3. Leave things as is and live with the fact that we have duplicate tags.

I think 3) is the best solution. We could revisit this in the future, but I can't think of a proper milestone for this change. Typically, metrics dashboards deal with multiple applications, built with various Spring Boot versions. Unless there's a strong driver for this removal, I don't think this is worth breaking people.

I'm going to turn this into a documentation improvement and call out in our reference documentation that the error tag should be preferred in dashboards and that exception might be removed in the future.

@bclozel bclozel added type: documentation A documentation task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 4, 2023
@bclozel bclozel changed the title http.server.requests error tag Document that Micrometer's "error" tag should be preferred vs. legacy "exception" tag Jan 4, 2023
@chicobento
Copy link
Author

@bclozel fair enough, sounds like a plan. just a heads up that http.server.requests.active metric will have the exception tag but not the error tag - not that either one or the other are relevant for this metric.
not sure about the behavior of other metrics though, will add here if I find anything.

@jonatan-ivanov
Copy link
Member

@bclozel 👍🏼 Maybe it makes sense to provide the way in the docs to remove the exception tag if users want to:

@Bean
MeterFilter exceptionTagRemover() {
    return MeterFilter.ignoreTags("exception");
} 

A step forward could be (if users asks for it?) something similar that we have with common tags: if management.metrics.tags present, Boot creates a MeterFilter that adds those tags to every meter. We could do something very similar to ignore tags.

@jonatan-ivanov
Copy link
Member

@chicobento As you mentioned neither exception nor error are relevant for http.server.requests.active. You probably already figured it out but these .active time series will never have error tag and the exception tag should always be none. The reason behind this is that they track operations that are in-progress (LongTaskTimer), once the operation failed, it is already terminated. So .active looks in the present and there you don't know if an operation failed or not since it is still in progress. The "normal" (w/o .active) time series look into the past, where you have the data if an operation failed or not.

Please let us know if you found any issues.

@bclozel bclozel transferred this issue from spring-projects/spring-boot Oct 27, 2023
@bclozel bclozel self-assigned this Oct 27, 2023
@bclozel bclozel added the theme: observability An issue related to observability and tracing label Oct 27, 2023
@bclozel bclozel added this to the 6.1.0 milestone Oct 27, 2023
@bclozel bclozel closed this as completed in 86bb8a0 Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: observability An issue related to observability and tracing type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

4 participants