Provide a way to give custom metric name to MicrometerHttpRequestExecutor

[beatfreaker]

Please describe the feature request. Currently the metric name for MicrometerHttpRequestExecutor is fixed to httpcomponents.httpclient.request, requesting to provide a way to give custom metric name like we have such option in OkHttpMetricsEventListener

Rationale We have different libraries which use different HTTP client libraries to make API calls i.e RestTemplate, OkHttp and HttpClient

Now we want to make sure that all the metrics generated by all these three clients are reported with same metric name. I know we can customize spring-boot's RestTemplate and OkHttp client metric name to httpcomponents.httpclient.request so that all metrics are reported under httpcomponents.httpclient.request but we like to name the metric as http.client.requests so need an option to provide custom metric name to MicrometerHttpRequestExecutor

Additional context

[jonatan-ivanov]

Since okhttp has it I think it would make sense to add it to this one too. Are you up to filing a PR for this?

In the meantime: you can customize any Meter using a MeterFilter, in your case:

new MeterFilter() {
    @Override
    public Meter.Id map(Meter.Id id) {
       if (id.getName().equals("httpcomponents.httpclient.request")) {
          return id.withName("http.client.requests");
       }
       return id;
    }
}

See the docs: https://micrometer.io/docs/concepts#_transforming_metrics

[beatfreaker]

@jonatan-ivanov Sure, you can assign this issue to me. I will work on a PR to resolve it.

[beatfreaker]

Hi @jonatan-ivanov ,

Should I create PR against main branch?

Also is it fine to implement a similar capability in MicrometerHttpClientInterceptor?

[jonatan-ivanov]

Should I create PR against main branch?

Yes, since this is a new functionality (it's not really a bug: a MeterFilter can be used to change the name).

Also is it fine to implement a similar capability in MicrometerHttpClientInterceptor?

I've just took a deeper look and it seems MicrometerHttpRequestExecutor is using the ObservationOrTimerCompatibleInstrumentation so it can create an Observation or a Timer. If an Observation is created (and the meter handler is registered) the created Timer will get its name from the ObservationConvention but if ObservationOrTimerCompatibleInstrumentation creates a Timer instead of an Observation, it gets its name from METER_NAME. I'm thinking adding a meterName(...) method to the builder would make this somewhat confusing since it will not have any effect in case an Observation is created. Though the same is true for the convention (does not have any effect if a Timer is created directly) so maybe adding some javadoc that describes this is fine.

MicrometerHttpClientInterceptor seems to be a simpler case, I think it's ok to add a new ctor for this.

@shakuzen What do you think?

[cachescrubber]

Exactly what I was looking for. Is there a chance the PR will make it into 1.11?

[cachescrubber]

Just a note to other people investigating how to combine different instrumentations under a unified metric name: The suggested MeterFilter to change the metric name lead me directly to #877, which is basically an unresolved micrometer / prometheus integration issue. Both instrumentations must use the exact same tags / dimensions, otherwise the data of one of them is not exported to prometheus. I wanted to combine cxf.client.requests and httpcomponents.httpclient.request which are both Timer for http request durations.

[marcingrzejszczak]

Sorry for the gigantic delay, we are refining our backlog and will do our best to give some feedback on the PR and this issue ASAP

[cachescrubber]

Hi @marcingrzejszczak,

the PR linked to this issue targets the now depreciated MicrometerHttpClientInterceptor and MicrometerHttpRequestExecutor. You probably would not add features to deprecated implementations I guess.

The current implementation could be customized, albeit not very user-friendly. On order to use a custom metric, DefaultApacheHttpClientObservationConvention has to be extended:

    static class CustomApacheHttpClientObservationConvention extends DefaultApacheHttpClientObservationConvention {
        @Override
        public String getName() {
            return "custom.httpclient.request";
        }
    }
    private final ObservationExecChainHandler customHandler = new ObservationExecChainHandler(observationRegistry, new CustomApacheHttpClientObservationConvention());

I'm happy to come up with a PR providing a more easily customizable solution. Is there a best practice / convention on how to implement customizable ObservationConvention which would integrate nicely in the project?

[marcingrzejszczak]

Actually what you did with the observation conventions is exactly what we suggest. You can read about that in the docs.

[cachescrubber]

Ok. I asked because there are still ObservationConvention having for example a constructor parameter for the metric name. DefaultOkHttpObservationConvention for example.

Would It be feasible to have a CustomizableApacheHttpClientObservationConvention extends DefaultApacheHttpClientObservationConvention with support for a custom metric name, extra tags and some options like exportTagsForRote?

[marcingrzejszczak]

Ah ok - yes, I understand now, IMO it makes sense, WDYT @shakuzen @jonatan-ivanov ?