Upgrade github.com/mostynb/go-grpc-compression and switch to nonclobbering imports #7966
Conversation
There was a problem hiding this comment.
@mostynb can you add a changelog entry to capture this change?
I suspect it will mostly be a non event for most users, but in case anyone is surprised by their compression options changing, they can reference the changelog to see what changed.
I gave it a try- I hope the formatting is OK. |
…ering imports Some consumers of this library have registered grpc compressors, and the init() order can cause their compressors to be clobbered by opentelemetry-collector's use of github.com/mostynb/go-grpc-compression, which follows the pattern from the grpc-go library (overwrite any previously registered encoders with the same name). Version 1.2.0 of github.com/mostynb/go-grpc-compression added so-called "nonclobbering" imports, which only register the codec if there is not already a codec registered with that name. This PR switches to use those. Fixes #7920.
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #7966 +/- ##
=======================================
Coverage 90.96% 90.96%
=======================================
Files 300 300
Lines 15090 15090
=======================================
Hits 13726 13726
Misses 1091 1091
Partials 273 273
☔ View full report in Codecov by Sentry. |
Description: Switch to grpc encoders that won't override the consumer's previously registered codecs.
Some consumers of this library have registered grpc compressors, and the init() order can cause their compressors to be clobbered by opentelemetry-collector's use of github.com/mostynb/go-grpc-compression, which follows the pattern from the grpc-go library (overwrite any previously registered encoders with the same name).
Version 1.2.0 of github.com/mostynb/go-grpc-compression added so-called "nonclobbering" imports, which only register the codec if there is not already a codec registered with that name. This PR switches to use those.
Link to tracking Issue: #7920
Testing: tests have been added to the upstream library.