search

micrometer-metrics / micrometer

An application observability facade for the most popular observability tools. Think SLF4J, but for observability.
https://micrometer.io
Apache License 2.0
4.45k stars 980 forks source link

Potential incosistency between httpclient and okhttp metrics #1701

Open kubamarchwicki opened 4 years ago

kubamarchwicki commented 4 years ago

In OkHttp metrics event listener, the host tag is used to denote the target url / host (https://github.com/micrometer-metrics/micrometer/blob/master/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java#L123) which is slightly different to apache httpclient metrics executor (https://github.com/micrometer-metrics/micrometer/blob/master/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java#L137-L141) where target.host tag is being used.

I personally find the latter better (especially many - including myself) use host tag to denote the machine code is running. I think target.host work better to describe the target host

Wouldn't it be nice'ish to to try to keep the tags coherent between different (two) http clients. I've noticed this while migrating httpclient to okhttp in one of the projects.

mweirauch commented 4 years ago

I don't mean to hijack this issue, but I always wondered why the http client implementations don't share the same meric name and tags? For the http.server.requests Spring WebMVC and Jersey2 use the same set of metrics and tags so it doesn't matter what technology you use. So the same name and tags could have been used from the RestTemplate-instrumentation: http.client.requests. Perhaps worthwhile to note for v2.

shakuzen commented 4 years ago

Let's use this issue to make things consistent out-of-the-box for 2.0. We don't want to break existing users, though, so let's try to work in a backwards compatible solution before 2.0.

marcingrzejszczak commented 9 months ago

How should we proceed since we're not planning to do 2.0 any time soon? Maybe we should start creating multiple tags (old and new)?

kubamarchwicki commented 9 months ago

Multiple tags make sense. I can raise a PR. Thank you for digging out such a legacy :)