Skip to content

Deprecation reason for the autotime enabled, percentiles, and percentiles-historgram properties is confusing #41745

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

Closed
pietro-saccani opened this issue Aug 8, 2024 · 5 comments
Assignees
Labels
type: documentation A documentation update
Milestone

Comments

@pietro-saccani
Copy link

pietro-saccani commented Aug 8, 2024

Hello, as of Spring Boot 3 the management.metrics.web.server.request.autotime configuration properties were removed and the following note was left: Should be applied at the ObservationRegistry level.

There's no explanation anywhere on how to do that.

Can something please be added to the documentation, or to the migration guide?
I've found no new alternative to configure the application to automatically measure the durations of RestTemplate exchanges. The only way I can see it being done is by using a custom interceptor that does it somehow. Otherwise I guess we have to explicitly measure times using Observations to wrap the code that uses a RestTemplate to make remote calls?

Sorry if this issue looks "StackOverflow-y" but there's honestly no info about this in the docs and the few already existing questions on StackOverflow have no answers or comments.

Edit: I've found this chapter of the documentation. By what I understand, at least for RestTemplate and WebClient, calls are observed by default, including their duration?
Anyway, for someone who had no idea about the Observations API before having to switch to Spring Boot 3, it's really time-consuming to understand where the feature that was previously granted by management.metrics.web.server.request.autotime.enabled = true ended up. In my opinion it's better to explicitly state, at least in the migration guide, that the new way is to configure an ObservationRegistry on the HTTP client of choice, and that it's done automatically for RestTemplate and WebClient (for now?). Also providing an example in the guide would be great.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 8, 2024
@mhalbritter
Copy link
Contributor

RestTemplates created through the autowired RestTemplateBuilder are automatically instrumented and publish observations with the name http.client.requests.

@Component
class MyBean implements CommandLineRunner {
    private static final Logger LOGGER = LoggerFactory.getLogger(MyBean.class);

    private final RestTemplate restTemplate;

    MyBean(RestTemplateBuilder restTemplateBuilder) {
        this.restTemplate = restTemplateBuilder.build();
    }


    @Override
    public void run(String... args) throws Exception {
        String content = this.restTemplate.getForObject(URI.create("http://example.com/"), String.class);
        LOGGER.info("Content: {}", content);
    }
}

creates

2024-08-09T09:35:30.115+02:00  INFO 30493 --- [sb-41745] [           main] i.m.o.ObservationTextPublisher           : START - name='http.client.requests', contextualName='null', error='null', lowCardinalityKeyValues=[client.name='example.com', exception='none', method='GET', outcome='UNKNOWN', status='CLIENT_ERROR', uri='none'], highCardinalityKeyValues=[http.url='http://example.com/'], map=[class io.micrometer.core.instrument.LongTaskTimer$Sample='SampleImpl{duration(seconds)=3.18238E-4, duration(nanos)=318238.0, startTimeNanos=3473343862384}', class io.micrometer.core.instrument.Timer$Sample='io.micrometer.core.instrument.Timer$Sample@691541bc'], parentObservation=null
2024-08-09T09:35:30.116+02:00  INFO 30493 --- [sb-41745] [           main] i.m.o.ObservationTextPublisher           :  OPEN - name='http.client.requests', contextualName='null', error='null', lowCardinalityKeyValues=[client.name='example.com', exception='none', method='GET', outcome='UNKNOWN', status='CLIENT_ERROR', uri='none'], highCardinalityKeyValues=[http.url='http://example.com/'], map=[class io.micrometer.core.instrument.LongTaskTimer$Sample='SampleImpl{duration(seconds)=7.31084E-4, duration(nanos)=731084.0, startTimeNanos=3473343862384}', class io.micrometer.core.instrument.Timer$Sample='io.micrometer.core.instrument.Timer$Sample@691541bc'], parentObservation=null
2024-08-09T09:35:30.317+02:00  INFO 30493 --- [sb-41745] [           main] i.m.o.ObservationTextPublisher           : CLOSE - name='http.client.requests', contextualName='null', error='null', lowCardinalityKeyValues=[client.name='example.com', exception='none', method='GET', outcome='UNKNOWN', status='CLIENT_ERROR', uri='none'], highCardinalityKeyValues=[http.url='http://example.com/'], map=[class io.micrometer.core.instrument.LongTaskTimer$Sample='SampleImpl{duration(seconds)=0.201936786, duration(nanos)=2.01936786E8, startTimeNanos=3473343862384}', class io.micrometer.core.instrument.Timer$Sample='io.micrometer.core.instrument.Timer$Sample@691541bc'], parentObservation=null
2024-08-09T09:35:30.317+02:00  INFO 30493 --- [sb-41745] [           main] i.m.o.ObservationTextPublisher           :  STOP - name='http.client.requests', contextualName='http get', error='null', lowCardinalityKeyValues=[client.name='example.com', exception='none', method='GET', outcome='SUCCESS', status='200', uri='none'], highCardinalityKeyValues=[http.url='http://example.com/'], map=[class io.micrometer.core.instrument.LongTaskTimer$Sample='SampleImpl{duration(seconds)=0.20269521, duration(nanos)=2.0269521E8, startTimeNanos=3473343862384}', class io.micrometer.core.instrument.Timer$Sample='io.micrometer.core.instrument.Timer$Sample@691541bc'], parentObservation=null

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
@pietro-saccani
Copy link
Author

pietro-saccani commented Aug 9, 2024

Sorry, I was editing the post and about to save just as you posted yours.

Yes, thank you, I had found out about this earlier. But the fact still stands that it's not clear for people who switched from Spring Boot 2 that the feature they previously activated with management.metrics.web.server.request.autotime.enabled = true is now active by default when using RestTemplate or WebClient. The sentence Should be applied at the ObservationRegistry level. is pretty cryptic for someone who never knew about the Observations API.

@mhalbritter
Copy link
Contributor

@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

mhalbritter commented Aug 9, 2024

The sentence Should be applied at the ObservationRegistry level. is pretty cryptic for someone who never knew about the Observations API.

Yeah, i agree with that. I think we should reword this to something like "Is now enabled automatically".

@mhalbritter mhalbritter changed the title Clarify documentation on replacing the removed "management.metrics.web.server.request.autotime" configuration properties Deprecation reason for the autotime properties is confusing Aug 9, 2024
@mhalbritter mhalbritter added type: documentation A documentation update and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Aug 9, 2024
@mhalbritter mhalbritter added this to the 3.2.x milestone Aug 9, 2024
@mhalbritter mhalbritter changed the title Deprecation reason for the autotime properties is confusing Deprecation reason for the autotime enabled properties is confusing Aug 9, 2024
@pietro-saccani
Copy link
Author

pietro-saccani commented Aug 9, 2024

Did you read https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-3.0-Migration-Guide#micrometer-and-metrics-changes and it's still unclear?

The paragraph itself is clear but, since it doesn't explicitly mention that Observers can record times too, someone who never used the Observations API and just had put management.metrics.web.server.request.autotime.enabled = true in their YAML to measure the duration of outgoing calls could be confused about what they should do with ObservationRegistries to replace it (and that's assuming that they did see that "cryptic" sentence I mentioned).

But anyway, I see you agree with me on that sentence being too vague. Changing that would be enough to save other people the time I spent on this >_>

@snicoll snicoll self-assigned this Sep 9, 2024
@snicoll snicoll changed the title Deprecation reason for the autotime enabled properties is confusing Deprecation reason for the autotime enabled, percentiles, and percentiles-historgram properties is confusing Sep 9, 2024
@snicoll snicoll modified the milestones: 3.2.x, 3.2.10 Sep 9, 2024
@snicoll snicoll closed this as completed in b1db3ad Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

4 participants