-
Notifications
You must be signed in to change notification settings - Fork 583
feat: add get_ref to retrieve Resource values by reference #3198
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?
feat: add get_ref to retrieve Resource values by reference #3198
Conversation
…nnecessary cloning
…nnecessary cloning
…eu-christian/opentelemetry-rust into feature/resource-get-ref
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3198 +/- ##
=====================================
Coverage 80.8% 80.8%
=====================================
Files 128 128
Lines 23090 23096 +6
=====================================
+ Hits 18676 18682 +6
Misses 4414 4414 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 @leghadjeu-christian thanks for raising this! Looks like a straightforward performance improvement with minimal side effects to me.
There's not really an established pattern of .get
and .get_ref
in the SDKs API, but I see no cleaner option to do this without a breaking change, which as you've pointed out we don't want.
The one other thing I note is that here:
opentelemetry-rust/stress/src/logs.rs
Line 71 in 3b2f751
.get(&Key::from_static_str("service.name")) |
... could also be changed to use .as_ref
!
Co-authored-by: Scott Gerring <[email protected]>
…eu-christian/opentelemetry-rust into feature/resource-get-ref
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.
Thanks - LGTM!
Fixes #3163
Changes
Adds a new
get_ref
method toopentelemetry_sdk::Resource
that returns a reference to the stored value instead of cloning it. This improves performance and avoids unnecessary allocations for read-only access, while keeping the existingget
method for backward compatibility.Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes