opentelemetry-dotnet icon indicating copy to clipboard operation
opentelemetry-dotnet copied to clipboard

Add http.url to the HTTP metrics instrumentation

Open xavirec opened this issue 2 years ago • 1 comments

Feature Request

Besides metrics specs being marked as Experimental, I think that it is useful to include http.url.

I'm fine to send a PR if you want. It should be a very simple change.

xavirec avatar Jun 20 '22 14:06 xavirec

Will it create a histogram for every unique URL?

I'd expect to be able to

  • exclude URLs unless a non-error status code was returned (to prevent a potential cardinality explosion)
  • group URLs together based on subpaths (to prevent a potential cardinality explosion, and also to aggregate metrics over URLs that are logically the same "endpoint")
    • For instance, given an endpoint for file downloads /download/{fileId}, I don't want a histogram for every file downloaded. Instead, all requests should be counted towards the /download/ value of tag http.url (or a different one, since /download/ is but a segment).

I'm looking into migrating from Prometheus-net, where the request middleware looks like this.

Perhaps there should be some way to inject (or change, should the instrumentation library have the capability to derive this on its own) grouping tags on a per request basis? Maybe one could just write their own processor for this?

kerams avatar Jun 23 '22 19:06 kerams

I'm wondering if there were any updates here? I'm also wondering if there's a way to inject http.url or at the very least the name of the asp net core controller action into the default provided asp net core metrics. I know that with traces, you can do something like Activity.SetTag("Key", "Value") to add an extra column based on an http request object, but I'm wondering if something similar would be possible for metrics?

jnazaren avatar Sep 29 '22 16:09 jnazaren

https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md#metric-httpclientrequestduration

vishweshbankwar avatar Nov 17 '23 19:11 vishweshbankwar