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

Missing functionality or Different behavior of ServerRequestObservationConvention, does not provide a way to add tags to long task timers #41747

Open
santoshdahal12 opened this issue Aug 8, 2024 · 7 comments
Labels
status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged

Comments

@santoshdahal12
Copy link

santoshdahal12 commented Aug 8, 2024

With Spring boot 2.7 and earlier, WebMvcTagsProvider had a method that allowed to add custom tags for long task timers. However, after migrating to Spring Boot 3.3 and using recommended migration strategy of extending DefaultServerRequestObservationConvention or implementing ServerRequestObservationConvention from the docs here, the behavior is not the same.

The getLowCardinalityKeyValues method cannot be used to add custom tags to long task timers or handlers annotated with for example @ Timed(value = "aws.scrape", longTask = true)

The migration guide does not speak anything about this missed functionality and I found few developers asking questions in stackoverflow not being answered. So wanted to know if there is any guidance about this ?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 8, 2024
@santoshdahal12 santoshdahal12 changed the title Missing functionality or Different behavior of ServerRequestObservationConvention, does not provide long task timers tags and a way to override them. Missing functionality or Different behavior of ServerRequestObservationConvention, does not provide a way to add tags to long task timers Aug 8, 2024
@mhalbritter
Copy link
Contributor

mhalbritter commented Aug 9, 2024

DefaultServerRequestObservationConvention or ServerRequestObservationConvention work on observations.

The @Timed annotation you used doesn't create an observation, it creates a metric directly.

To add a tag to a metric, you can use a MeterFilter:

    @Bean
    MeterFilter meterFilter() {
        return new MeterFilter() {
            @Override
            public Meter.Id map(Meter.Id id) {
                if (id.getName().equals("aws.scrape")) {
                    return id.withTag(Tag.of("additional", "tag"));
                }
                return id;
            }
        };
    }

If you want to have an observation instead, use @Observed, but AFAIK, this doesn't use long task timers.

Btw, ServerRequestObservationConvention only works for the observations created by framework itself for the http request (which is by default named http.server.requests) and is not called for your observation created by @Observed.

Does that solve your problem?

@mhalbritter mhalbritter added the status: waiting-for-feedback We need additional information before we can continue label Aug 9, 2024
@santoshdahal12
Copy link
Author

santoshdahal12 commented Aug 9, 2024

The problem statement is to add custom tags to long task timers annotated in the handlers of a controller in a centralized way without knowing specific metername

This was exactly solved by WebMvcMetricsFilter before.

The earlier implementation i.e till 2.7.18, WebMvcMetricFilter had a logic to get all Timed annotations from the handler of the controller and allowed to add custom tags for long task timers via WebMvcTagsProvider . For reference, this is the filter . This was nice because we had a centralized way of adding custom tags for handlers with long task timers.

MeterFilter does not know about request context and thus it's not straightforward to differentiate if the metric is from handlers of a controller or it's coming from any scheduled service or from somewhere else.

Any thoughts on how we could achieve the same ? or at least we could mention in migration documentation about this missed fucntionality and guidance on how to achieve the same

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 9, 2024
@mhalbritter
Copy link
Contributor

@marcingrzejszczak @jonatan-ivanov Do you know any solution to this?

@philwebb philwebb added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Sep 3, 2024
@snicoll
Copy link
Member

snicoll commented Mar 21, 2025

nudge to @marcingrzejszczak @jonatan-ivanov

@jonatan-ivanov
Copy link
Member

Sorry, I completely missed this issue (Marcin was on leave).
So based on this:

The problem statement is to add custom tags to long task timers annotated in the handlers of a controller in a centralized way without knowing specific metername

I think this is not possible anymore out of the box like it was in 2.x but bear with me, I might have a solution for you. In Boot 2.x, these annotations on the controllers were mainly handled by Boot (see: AutoTimer). This is not the case anymore and if you create an annotation Micrometer's TimedAspect will handle them which has zero idea about the controller or the http request (it can be used on any methods). On the other hand, Spring MVC (and WebFlux) were instrumented with the Observation API and (as Moritz said above) if you override DefaultServerRequestObservationConvention or implement ServerRequestObservationConvention, you can attach low (or high cardinality) key-values to the LongTaskTimers that were created from Observations of Spring Framework (not by your annotation), the name of the LongTaskTimer is http.server.requests.active. If you want to rename the Observation (LongTaskTimer), you can do that too in the convention.

If this does not work for you for some reason, you can create a LongTaskTimer or an Observation in the controller method and AutoTimer is still part of Boot so I think it still can be enabled.

DefaultServerRequestObservationConvention or implementing ServerRequestObservationConvention from the docs here, the behavior is not the same.

Maybe given that this is a major version change with a completely different API but if you can let go using @Timed, you can do what I recommended above and the behavior should be the same I think.

The migration guide does not speak anything about this missed functionality and I found few developers asking questions in stackoverflow not being answered. So wanted to know if there is any guidance about this ?

@santoshdahal12 Could you please point me to those unanswered StackOverflow questions?

@wilkinsona Do you happen to know if AutoTimer can be still enabled in Boot somehow?

@philwebb philwebb added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Mar 24, 2025
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Mar 31, 2025
@wilkinsona
Copy link
Member

Do you happen to know if AutoTimer can be still enabled in Boot somehow?

We have auto-timer support for Spring Data repository metrics but not, IIRC, for anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

No branches or pull requests

7 participants