-
Notifications
You must be signed in to change notification settings - Fork 565
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
Expose instrumentation scope name #4448
Conversation
Could you please make the the PR description more detailed? You could e.g. give examples with code what is possible with the new method and what is the alternative when it is missing (changing to draft before it is done). |
@pellared How does it look now? |
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.
How does it look now?
Now, I find it clear 👍
I will first change the the title and ask others if they like the proposal.
If the proposal would be accepted then:
- We would have to do this for all instrumentation libraries.
- Changelog would have to be updated.
@open-telemetry/go-approvers PTAL Give the PR a 👍 if you like the proposal. Additionally more detailed (text) feedback is welcome. |
Codecov Report
@@ Coverage Diff @@
## main #4448 +/- ##
=====================================
Coverage 80.8% 80.8%
=====================================
Files 150 150
Lines 10371 10371
=====================================
Hits 8387 8387
Misses 1840 1840
Partials 144 144
|
1370131
to
2ab2ed5
Compare
What other libraries are you aware of? I can get to them too. |
All under https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main/instrumentation |
I think I found all that are using MeterProvider. |
The same has to be done for all |
otelhttp has a meterprovider that is why I did it. Do we have a use case for exposing the scope names for tracers? Of course I don't mind making the change. I am just curious. |
API consistency. For now, leave it as it is. I added your proposal as a topic for today's SIG meeting |
@utezduyar During SIG meeting we agreed that
I am converting the PR to draft. Please make it "Ready for review" when you update the PR. |
It is good to know the scope to apply view filters. One use case is to drop some metrics in the application.
What about |
It is back again. |
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.
I noticed that we are also missing ScopeName
in otelmacaron
and otelhttptrace
.
instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go
Outdated
Show resolved
Hide resolved
Can you point me the file name for this? I still couldn't find it. |
|
Co-authored-by: David Ashpole <dashpole@google.com>
@utezduyar Thank you for your contribution 🎉 |
Thanks for your assistance. |
It is good to know the scope to apply view filters. For example if we would like to drop all or some of the meters from this library, below code is how the filtering is applied. The scope name is used explicitly and a change in the scope name by the runtime instrumentation library will break applications.
Instead of this, get the scope name from the library itself