-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Checked if instrument enabled before measuring in oteltracegrpc
#7825
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7825 +/- ##
=======================================
- Coverage 81.7% 81.7% -0.1%
=======================================
Files 304 304
Lines 23252 23260 +8
=======================================
+ Hits 19009 19015 +6
- Misses 3860 3862 +2
Partials 383 383
🚀 New features to boost your workflow:
|
exporters/otlp/otlptrace/otlptracegrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlptrace/otlptracegrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
| success := successful(e.nSpans, err) | ||
| addOpt := get[metric.AddOption](addOptPool) | ||
| defer put(addOptPool, addOpt) | ||
| *addOpt = append(*addOpt, e.inst.addOpt) |
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.
Preserve the existing code flow. Changing this is not needed and a distraction for this PR.
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.
Hey so I got stuck here because if the if block in which success is there doesn't run the other block where success is used will panick that's why I changed structure.
| } | ||
|
|
||
| recOpt := get[metric.RecordOption](recordOptPool) | ||
| defer put(recordOptPool, recOpt) |
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.
Why is this changed?
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 changed it because I wanted to put back the slice manually each time. I will change it to defer if it is better but I thought putting it back instantly would be better.
oteltracegrpc
…entation.go Co-authored-by: Tyler Yahn <[email protected]>
…entation.go Co-authored-by: Tyler Yahn <[email protected]>
This pr updates otltracegrpc for checking if the operation is enabled before adding any metric or performing operation.
tracked in #7800