open-telemetry / opentelemetry-collector

OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
4.51k stars 1.48k forks source link

Upgrade pdata to proto to 1.4.0 #11722

Open dmathieu opened 4 days ago

dmathieu commented 4 days ago

This upgrades pdata to the 1.4.0 version of protobufs.

Sorry for the very large PR. This can't really be split into smaller PRs though, as changes are not backwards compatible.

This PR includes https://github.com/open-telemetry/opentelemetry-collector/pull/11706, as the two are very close and separating them would cause weird conflicts. The two changes can be merged either as separate PRs (with 11706 first), or as one.

Closes https://github.com/open-telemetry/opentelemetry-collector/issues/11720

cc @mx-psi

codecov[bot] commented 4 days ago

Codecov Report

Attention: Patch coverage is 93.99478% with 23 lines in your changes missing coverage. Please review.

Project coverage is 91.53%. Comparing base (f74890a) to head (408fbc9).

Files with missing lines Patch % Lines
pdata/pprofile/json.go 85.18% 8 Missing :warning:
pdata/internal/generated_wrapper_int32slice.go 50.00% 6 Missing :warning:
pdata/internal/generated_wrapper_intslice.go 50.00% 6 Missing :warning:
pdata/pprofile/profile.go 0.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #11722 +/- ## ========================================== - Coverage 91.61% 91.53% -0.09% ========================================== Files 443 447 +4 Lines 23770 23720 -50 ========================================== - Hits 21776 21711 -65 - Misses 1620 1635 +15 Partials 374 374 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

mx-psi commented 4 days ago

Is there any way we can make a graceful transition for avoiding this breakage in contrib?

=== FAIL: .  (0.00s)
FAIL    github.com/open-telemetry/opentelemetry-collector-contrib/extension/encoding/otlpencodingextension [build failed]

=== Errors
Error: ./extension_test.go:205:6: im.SetStartTime undefined (type pprofile.Profile has no field or method SetStartTime)
Error: ./extension_test.go:206:6: im.SetEndTime undefined (type pprofile.Profile has no field or method SetEndTime)
dmathieu commented 4 days ago

I can bring back StartTime, with TimeNanos as the origin field name. But I can't bring back EndTime, as Duration doesn't have the same meaning.

dmathieu commented 4 days ago

I have made EndTime a noop.

mx-psi commented 4 days ago

This PR includes https://github.com/open-telemetry/opentelemetry-collector/pull/11706, as the two are very close and separating them would cause weird conflicts.

Should we close #11706 then?

dmathieu commented 4 days ago

It really depends how reviews go. If you prefer reviewing a single but bigger PR, then yes.