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

Follow-up investigation of GRPC compression library #4342

Closed
hyunuk opened this issue Nov 2, 2021 · 2 comments
Closed

Follow-up investigation of GRPC compression library #4342

hyunuk opened this issue Nov 2, 2021 · 2 comments

Comments

@hyunuk
Copy link
Contributor

hyunuk commented Nov 2, 2021

Is your feature request related to a problem? Please describe.

For issues #2548 and the PR #4088 , it imports the library github.com/mostynb/go-grpc-compression in Collector Core

Background discussion: Bogdan raised a concern that this library may not be reliable since it is maintained by a single maintainer and not widely used. However, Anthony (@Aneurysm9) felt this library is fairly reliable since it does have a maintainer and has been released regularly over a year.

In the long run, we can consider replicating the code and internalize using this library if needed. Currently, using this library as-is should not pose a risk.

Next steps include merging PR #4088 and also tracking this discussion with this issue.

Describe the solution you'd like

I think taking a dependency on the library is a better option.

  • Although the library is not widely used, it is frequently maintained over a year and has 10+ releases.
  • Since the library is just an implementation of the interface, the code is simple and less vulnerable to create a bug.
  • If we internalize the code in the core, it should be maintained.
  • We can internalize it anytime later if it causes a problem.

Describe alternatives you've considered

Another solution we can take is implement the wrapper code directly in our collector/core.

Additional context

I did a research on the Jaeger, AWS SDK for Go v2, and other repositories about how grpc encoding is used in those libraries. Followings are what I found.

  • Jaeger only uses ‘gzip’ and not supports any other compression methods. The relevant PR is [here] (Enable gzip compression for collector grpc endpoint. jaegertracing/jaeger#3236)

  • AWS SDK for Go v2 does not use any gRPC compression methods.

  • The library github.com/mostynb/go-grpc-compression is not widely used. bazel-remote and bazel-cache is the most famous repositories that depends on this library.

The basic compression library given from grpc, google.golang.org/grpc/encoding/gzip is imported 283 times in pkg.go.dev. grafana/dskit and honeycombio/refinery are using gzip with other compression methods. While Grafana uses snappy, Honeycomb uses zstd. Both of them implemented a wrapper code of snappy/zstd internally.

cc: @alolita

@bogdandrutu
Copy link
Member

@hyunuk for AWS SDK for Go v2 does not use any gRPC compression methods. I understand they don't use gRPC but for HTTP requests what compression do they use?

So I see a need for snappy and maybe zstd, besides that no need. Also we need to add support for HTTP as well correct? So may not be un-reasonable to wrap them directly, with this package we also get lz4 which I don't want to maintain because nobody needs it.

@codeboten
Copy link
Contributor

codeboten commented Nov 3, 2021

http is captured in this issue #2766

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

No branches or pull requests

3 participants