opentelemetry-collector
opentelemetry-collector copied to clipboard
Add stringSlice and int64Slice to pcommon, and let tests handle non-number slices
Description
This adds a string slice to pcommon, that will be used by the profiles pdata.
Link to tracking issue
Part of #10109.
Testing
This is unit-tested.
cc @mx-psi
Codecov Report
Attention: Patch coverage is 79.33884% with 25 lines in your changes are missing coverage. Please review.
Project coverage is 92.15%. Comparing base (
e7dcfcc) to head (83f82f1).
Additional details and impacted files
@@ Coverage Diff @@
## main #10148 +/- ##
==========================================
- Coverage 92.24% 92.15% -0.10%
==========================================
Files 353 357 +4
Lines 16797 16918 +121
==========================================
+ Hits 15495 15591 +96
- Misses 963 988 +25
Partials 339 339
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@dmathieu To confirm: the intent is to use these types for fields like string_table or location_indices, right?
Yes, it is. My pdata PR shows that use as well.
Thanks for confirming. I will wait until Friday before merging this to give time to others to review
Looking at #10109 I only see the new types being used in
pprofile. Is it required that the new types be inpcommon?
IMO it is required after profiling is declared stable. It's not strictly required now, but it's better in that we don't need people to update their import paths for this after profiling goes to 1.0
It's not strictly required now, but it's better in that we don't need people to update their import paths for this after profiling goes to 1.0
I guess the risk is that this goes into 1.x pcommon and then needs removed/modified. Unlikely, but how would we handle that? Deprecate it and remove in 2.0?
Deprecate it and remove in 2.0?
Yup, it would become dead code
What would we do if it needs to be modified instead of removed?
What would we do if it needs to be modified instead of removed?
We would have to deprecate it and come up with a different name. I don't think that's going to happen here though, what a StringSlice type should look like is kind of already defined by the other <Type here>Slice types we already support
Waiting for v0.101.0 to be released, will merge after that