-
Notifications
You must be signed in to change notification settings - Fork 810
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
feat(instrumentation-http): Add API for adding custom metric attributes #4108
feat(instrumentation-http): Add API for adding custom metric attributes #4108
Conversation
db98bf7
to
d335f6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this!
experimental/packages/opentelemetry-instrumentation-http/src/http.ts
Outdated
Show resolved
Hide resolved
experimental/packages/opentelemetry-instrumentation-http/src/types.ts
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## main #4108 +/- ##
==========================================
- Coverage 92.31% 90.53% -1.78%
==========================================
Files 330 159 -171
Lines 9417 3763 -5654
Branches 1993 838 -1155
==========================================
- Hits 8693 3407 -5286
+ Misses 724 356 -368 |
d335f6a
to
0525942
Compare
039228a
to
a410833
Compare
a410833
to
6097474
Compare
@legendecas does it look good to you now? |
6097474
to
bc47738
Compare
@legendecas Hi, can we merge this please? 🙏 |
👻 Guess I got ghosted, lets close this. |
Hey @legendecas, How can I achieve this functionality with the current implementation of the library? |
@klippx @legendecas what happened here? |
Never mind me. I see the changes relate to static label values only and I was looking for a dynamic one. |
I'd like to add the service name as an attribute on http metrics - seems like it isn't possible without something like this. |
The marked answer has been updated to reflect how we solved this: #4088 (reply in thread) This fixes the problem across all metrics, not only http metrics. |
Which problem is this PR solving?
Add a clear and intuitive API to configure custom metric attributes that a user may inject (unrelated to span context).
Fixes #4107
Related discussion: #4088
Short description of the changes
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
There are only two metrics recorded, they should both get the custom attributes:
Checklist: