Skip to content
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

Please, add http.target and http.url to metric attributes #3553

Closed
mkrudele opened this issue Jan 19, 2023 · 4 comments · Fixed by #3581
Closed

Please, add http.target and http.url to metric attributes #3553

mkrudele opened this issue Jan 19, 2023 · 4 comments · Fixed by #3581

Comments

@mkrudele
Copy link

NB: Before opening a feature request against this repo, consider whether the feature should/could be implemented in the other OpenTelemetry client libraries. If so, please open an issue on opentelemetry-specification first.

Is your feature request related to a problem? Please describe.

http_server_duration is missing http_target attribute.
http_client_duration is missing the http_url attribute.
Both are crucial to build effective monitoring dashboards and alerting system.

Looking at the code I see TODOs here:

//TODO: http.url attribute, it should susbtitute any parameters to avoid high cardinality.

and here
//TODO: http.target attribute, it should susbtitute any parameters to avoid high cardinality.
.

Describe the solution you'd like

We are using prometheus exporter to send metrics to our Syddig instance. As wrote above, we need the http.target and http.url in server and client metrics to build really useful monitoring dashboards and alerting system.

Describe alternatives you've considered

We want to use OpenTelemetry, so not yet considered alternatives.

Additional context

@mkrudele
Copy link
Author

I also noticed that the http_target does not include request query parameters. Would it be possible to add an additional attribute, e.g. http_search_params for that?

@hermogenes
Copy link
Contributor

I also noticed that the http_target does not include request query parameters. Would it be possible to add an additional attribute, e.g. http_search_params for that?

Hello @mkrudele! Are you sure that is a good idea add query params to metrics? I think it would make the lib prone to high cardinality problems. Maybe an option would be introduce a hook similar to the ones available to span and allow customize the metric on demand

@mkrudele
Copy link
Author

mkrudele commented Feb 3, 2023

@hermogenes I agree, I didn't think at that aspect. A hook would work, however you added the most important piece of information actually missing, the HTTP_ROUTE, which is great. Thank you. I hope it will be published soon in a new release.

@mkrudele
Copy link
Author

@hermogenes Any idea of when will this be published in a new release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants