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

Align http and undici metric name #4805

Closed
mbrevda opened this issue Jun 18, 2024 · 2 comments
Closed

Align http and undici metric name #4805

mbrevda opened this issue Jun 18, 2024 · 2 comments

Comments

@mbrevda
Copy link

mbrevda commented Jun 18, 2024

Currently, opentelemetry-instrumentation-http exports metrics under the name http.client.duration, while instrumentation-undici exports metrics as http.client.request.duration. It would seem that this is the same metric, and should be named as such (unless I'm misunderstanding something here?).

I'm not sure which is "correct" or which package needs to change, but it would seem like one of them should be changed to match the other. Same for their context. Guessing this can be hacked via a View or something, although a native solution would be way more elegant.

Additional context

The difference I see between how these these packages are being utilized is via "fake fetch" packages, such as node-fetch, Axios, and other packages that use node: HTTP under the hood, vs. packages using native fetch (or importing undici). It might be interesting to note the client in the context.

/cc @david-luna (open-telemetry/opentelemetry-js-contrib#1951) and @hectorhdzg (#3149) who seemingly added these two versions. Thanks guys!

@pichlermarc
Copy link
Member

Hi thanks for reaching out 🙂 - I think this may be a duplicate of #4572, see reasons below:

A while ago there were changes in the semantic conventions for HTTP and we have not updated @opentelemetry/semantic-conventions yet to match the latest semconv. This package contains the constants for metric and attribute names. These there were quite a few challenges to overcome but this update is on-track now. @opentelemetry/instrumentation-undici is using a later version of the semconv which it has locally for now as it's a new instrumentation.

@dyladan is currently working on updating the semantic conventions package (@opentelemetry/semantic-conventions) #4690 so that we can update @opentelemetry/instrumentation-http and all other instrumentations to the latest semconv version. This update will include metrics names and align them with what you see today in @opentelemetry/instrumentation-undici. See #4572 as the tracking this update.

I'm closing this feature request as a duplicate of #4572.

@pichlermarc pichlermarc closed this as not planned Won't fix, can't repro, duplicate, stale Jun 18, 2024
@mbrevda
Copy link
Author

mbrevda commented Jun 18, 2024

great, thanks so much, all!

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

No branches or pull requests

2 participants