-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Upgrade pdata to v1.5.0 #11932
base: main
Are you sure you want to change the base?
Upgrade pdata to v1.5.0 #11932
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11932 +/- ##
==========================================
+ Coverage 91.59% 91.62% +0.03%
==========================================
Files 448 447 -1
Lines 23759 23737 -22
==========================================
- Hits 21761 21749 -12
+ Misses 1623 1613 -10
Partials 375 375 ☔ View full report in Codecov by Sentry. |
pdata/pprofile/profile.go
Outdated
// Attributes returns the Attributes associated with this Profile. | ||
// | ||
// Deprecated: [v0.117.0] This field has been removed, and replaced with AttributeIndices | ||
func (ms Profile) Attributes() pcommon.Map { |
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.
This method prevents panics on contrib. It's not a great solution either though, as it makes attributes readable, but not writable (and we can't make them writable, since we can't look for changes in pcommon.Map
).
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 wonder if it makes more sense to just remove this. It's a bigger breakage, but it's more noticeable that writes silently failing
Unfortunately, due to a change in the way profile attributes are handled, this brings a breaking change to contrib, which isn't really fixable (we can read the data from the old method, but not write). I'd suggest a coordinated merge of the two PRs. |
f90033d
to
2a2feff
Compare
Description
Proto 1.5.0 has been released. This upgrades pdata to use that version.
See https://github.com/open-telemetry/opentelemetry-proto/releases/tag/v1.5.0