Skip to content
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

Support proto v2 message format, drop v1 format #2647

Merged
merged 1 commit into from Oct 10, 2022

Conversation

ash2k
Copy link
Contributor

@ash2k ash2k commented Aug 12, 2022

Fixes #1028.

Currently, the size is only calculated for v1 messages and not for v2. This is because type assertion checks for the v1 interface.

We have two options:

  • Add type check for v2 interface and support both v1 and v2 messages.
  • Drop support for v1 and support v2 only.

This PR proposes to drop v1 support as the API has been deprecated for quite a while and most users should have migrated to v2 already. This should fix the performance problem by never calculating the size for v1 messages. Instead, the size will be efficiently calculated for v2 messages.

@codecov
Copy link

codecov bot commented Oct 8, 2022

Codecov Report

Merging #2647 (b443037) into main (773172b) will increase coverage by 0.0%.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #2647   +/-   ##
=====================================
  Coverage   69.2%   69.2%           
=====================================
  Files        145     145           
  Lines       6709    6712    +3     
=====================================
+ Hits        4648    4651    +3     
  Misses      1947    1947           
  Partials     114     114           
Impacted Files Coverage Δ
...ion/google.golang.org/grpc/otelgrpc/interceptor.go 83.6% <100.0%> (+0.1%) ⬆️

@ash2k
Copy link
Contributor Author

ash2k commented Oct 8, 2022

Rebased to resolve conflicts. Ready for merge.

@Aneurysm9 Aneurysm9 merged commit 9fd4fa8 into open-telemetry:main Oct 10, 2022
@ash2k ash2k deleted the support-msg-v2 branch October 11, 2022 01:36
@MrAlias MrAlias mentioned this pull request Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Performance] gRPC instrumentation: interceptor causes full payload to marshal into memory
4 participants