[chore] [pdata] Change CopyTo to operate directly on proto #13267
Conversation
dmitryax
requested review from
mx-psi,
dmathieu,
a team and
bogdandrutu
as code owners
dmitryax
force-pushed
the
internalize-copy-to
branch
from
02ff0d3 to
1b77214
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13267 +/- ##
==========================================
- Coverage 91.57% 91.56% -0.02%
==========================================
Files 522 522
Lines 29089 29145 +56
==========================================
+ Hits 26639 26687 +48
- Misses 1933 1939 +6
- Partials 517 519 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dmitryax
force-pushed
the
internalize-copy-to
branch
from
1b77214 to
72db77c
Compare
| sort.SliceStable(*es.orig, func(i, j int) bool { return less(es.At(i), es.At(j)) }) | ||
| } | ||
|
|
||
| func copyOrigResourceSpansSlice(dest, src []*otlptrace.ResourceSpans) []*otlptrace.ResourceSpans { |
There was a problem hiding this comment.
Not related to your change since you preserve existing logics, we should consolidate this logic with the one that we have on the primitive slices that uses append.
Probably we need some benchmarks and decide on one of them.
| if src.Min_ == nil { | ||
| dest.Min_ = nil | ||
| } else { | ||
| dest.Min_ = &otlpmetrics.HistogramDataPoint_Min{Min: src.GetMin()} |
There was a problem hiding this comment.
If destination has min, can avoid a new allocation here and re-use that.
There was a problem hiding this comment.
Can be done in separate PR since is an optimization.
If destination has an optional field set to some value but source has it unset, we need to unset the field on the destination instead of keeping the old value. Found this bug while working on #13267
This was missed in #13267
Part of #13264 but applied generically to all pdata types