Allow custom ScheduledTaskObservationConvention in ScheduledMethodRunnable

[hadjiski]

Currently a custom convention can be achieved only by going through the micrometer global conventions: SomeCustom implementes GlobalObservationConvention<ScheduledTaskObservationContext>
which need to be extra registered in a ObservationRegistryCustomizer bean

@Bean
    public ObservationRegistryCustomizer<ObservationRegistry> observationRegistryCustomizer() {
        return registry -> 
            registry.observationConfig()
                    .observationConvention(new SomeCustom());
    }

This though is making it impossible to re-use the default code from the DefaultScheduledTaskObservationConvention because the observation registry -> observationConfig -> observationConvention method is expecting a GlobalObservationConvention<?> whereas a DefaultScheduledTaskObservationConvention is in a slight different chain of the ObservationConvention inheritance.

Would it be feasible/possible to let the ScheduledMethodRunnable arrange its convention similar to the RestClientObservationConfiguration for example, where a simple component or a bean extending the DefaultClientRequestObservationConvention is enough to be used as a custom convention for the http.client.requests metrics.

Not sure what is the reason, but these customizations of conventions seem to not follow a consistent approach in general, see the recently fixed situation for the DefaultMongoHandlerObservationConvention, also in general:

• extending DefaultServerRequestObservationConvention // works like a charm for the http.server.requests metrics
• extending DefaultClientRequestObservationConvention // works like a charm for the http.client.requests metrics
• extending DefaultRepositoryTagsProvider // works like a charm for the spring.data.repository.invocations metrics though differs from first two ones
• extending DefaultMongoCommandTagsProvider // works like a charm for the mongodb.driver.commands metrics though differs from first two ones
• implementing GlobalObservationConvention<ScheduledTaskObservationContext> // works for the tasks.scheduled.execution metrics, but as described above - only after manually adding it as part of a custom bean configuration of the ObservationRegistryCustomizer<ObservationRegistry>...observationRegistry.observationConfig().observationConvention(...)

[bclozel]

Thanks for the report @hadjiski - I will look into this.

I agree that we are not consistent with allowing custom conventions locally, but this can be harder to achieve depending on how deep the integration is and how is it is to surface such an option into the configuration.

This though is making it impossible to re-use the default code from the DefaultScheduledTaskObservationConvention because the observation registry -> observationConfig -> observationConvention method is expecting a GlobalObservationConvention<?> whereas a DefaultScheduledTaskObservationConvention is in a slight different chain of the ObservationConvention inheritance.

To me the GlobalObservationConvention setup on the registry is meant for this - overriding a default convention configured by the Framework. I'm not sure why you think this is impossible, have you tried the following?

package io.spring.sample

import io.micrometer.common.KeyValue;
import io.micrometer.observation.GlobalObservationConvention;
import org.springframework.scheduling.support.DefaultScheduledTaskObservationConvention;
import org.springframework.scheduling.support.ScheduledTaskObservationContext;

public class CustomScheduledTaskObservationConvention extends DefaultScheduledTaskObservationConvention 
		implements GlobalObservationConvention<ScheduledTaskObservationContext> {

	@Override
	protected KeyValue codeFunction(ScheduledTaskObservationContext context) {
		return super.codeFunction(context);
	}
	
}

[hadjiski]

Thank you @bclozel for the fast reply, I was so back and forth between all the ways to utilize the observations and their customizations, that I mixed it with the situation around the DefaultMongoHandlerObservationConvention which is defined as package private class and somehow it stayed in my mind.
Yes, extends DefaultScheduledTaskObservationConvention implements GlobalObservationConvention<ScheduledTaskObservationContext> would work.

Btw. how am I supposed to propagate business data from outside to the current observation resp. the provided context, so I can use it in the above overridden customized method to take some decisions.
Looking at this description I am trying to propagate it via the ObservationRegistry.getCurrentObservation() -> getContext() -> put to map, which almost always works, just for the http.server.requests it does not, but there we can fallback to using the HttpServletRequest and its attributes.
Now doing the same, does not appear to work for the observation for the scheduled tasks. Calling getCurrentObservation() returns null, also accessing the provided ScheduledTaskObservationContext is not the one, which was available outside as the business data was propagated to the current observation context, which at that time was spring.security.http.secured.requests one (I am using a regular Spring boot 3.2.3 rest controller application).

Explaining the solution for http.server.requests in more details:
Somewhere in the rest controller I am putting business data to the HttpServletRequest attributes, cause I know that they will be later available as context.getCarrier().getAttribute(...), so in a customizer like this I could access the business data

@Component
public class ServerRequestMetricsCustomizer extends DefaultServerRequestObservationConvention {
...
    @Override
    public KeyValues getLowCardinalityKeyValues(ServerRequestObservationContext context) {
        return super.getLowCardinalityKeyValues(context)
                 // context has to be passed as the only way to access the propagated business data
                .and(someMethodToExtractKeyValuesFromCarrierAttributes(context));
    }
}

Though the ScheduledTaskObservationContext does not appear to have such a carrier or other element, which is available in the outside area.

[bclozel]

@hadjiski getting the current observation from within the scheduled method should work, we have an integration test that verifies that. If you can reproduce the problem in a minimal sample application, please create a new issue and share that application.

[hadjiski]

@bclozel my bad, I had multiple scheduled methods and only for one of them setup the propagation.
After a closer look, I can confirm it working same like the example ServerRequestObservationContext just that not the carrier's attributes are to be used, rather the map of the context itself, thanks for the clarification