-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[pdata] Add support for the new resource-entity references in xpdata #13264
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
[pdata] Add support for the new resource-entity references in xpdata #13264
Conversation
113ab6b
to
94e027e
Compare
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (85.44%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #13264 +/- ##
==========================================
- Coverage 91.67% 91.61% -0.06%
==========================================
Files 522 527 +5
Lines 29208 29366 +158
==========================================
+ Hits 26775 26903 +128
- Misses 1917 1944 +27
- Partials 516 519 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Part of #13264 but applied generically to all pdata types
284287b
to
cddea6a
Compare
cddea6a
to
e91922d
Compare
) | ||
|
||
// ResourceEntityRefs returns the EntityRefs associated with this Resource. | ||
func ResourceEntityRefs(res pcommon.Resource) EntityRefSlice { |
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.
s/ResourceEntityRefs/GetEntityRefs?
I think this is temporary correct until the entity is moved to stable. If that is the case we should document it.
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.
s/ResourceEntityRefs/GetEntityRefs?
Why don't mention the resource in the name? Also, Get
isn't good from the Go best practices perspective. Maybe EntityRefsFromResource
?
I think this is temporary correct until the entity is moved to stable. If that is the case we should document it.
Added a comment about that
Add support for the new resource-entity reference API added to the proto as part of open-telemetry/opentelemetry-proto#635. This required moving CopyTo implementation to the internal package for the structures that are used by other packages
e91922d
to
770f58c
Compare
) | ||
|
||
// ResourceEntityRefs returns the EntityRefs associated with this Resource. | ||
// Once EntityRefs is stabilized in the proto definition, |
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.
Once EntityRefs is stabilized in the proto definition,
FYI: Even though this is in xpdata if incompatible changes are made to the EntityRefs
definition, then we can no longer process the incoming request. Please make sure we don't break this.
627e36f
Add support for the new resource-entity reference API added to the proto as part of open-telemetry/opentelemetry-proto#635.
The API is separated into the experimental xpdata module since the new entity related protobuf definitions are in development
This is the first step towards adopting Entities in Collector which was drafted in #11958