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

gRPC Stream Interceptors #901

Closed
jpopadak opened this issue Feb 17, 2021 · 5 comments · Fixed by #918
Closed

gRPC Stream Interceptors #901

jpopadak opened this issue Feb 17, 2021 · 5 comments · Fixed by #918
Labels
agent-go enhancement New feature or request

Comments

@jpopadak
Copy link

jpopadak commented Feb 17, 2021

Is your feature request related to a problem? Please describe.
There currently exists within apmgrpc UnaryClientInterceptor and UnaryServerInterceptor.

I see within the documentation it says no Stream support exists for gRPC.

There is currently no support for intercepting at the stream level. Please file an issue and/or send a pull request if this is something you need.

Well, here I am asking for a Client and Server Stream Interceptor for gRPC use. 🙃
We have many services that use gRPC Stream APIs, and this is sadly one of our blockers for a quick migration.

Describe the solution you'd like
A similar interceptor (just as easy) as the UnaryClientInterceptor and UnaryServerInterceptor.

Describe alternatives you've considered

Additional context
Unrelated, but gRPC specific: Would be wonderful if we could also get #900 included.

@jpopadak jpopadak changed the title gRPC Stream Interceptor gRPC Stream Interceptors Feb 17, 2021
@axw
Copy link
Member

axw commented Feb 17, 2021

What would you expect the stream interceptors to measure? Beginning to end of stream? Anything in between?

@jpopadak
Copy link
Author

jpopadak commented Feb 17, 2021

Something like how the Unary interceptors work.

  • Timing
  • Deadline if any
  • Response code
  • Error if any
    I don't think we need the whole message body (most people probably won't use it).

The Logrus interceptor is a good basic example of how to retrieve the data.

@axw
Copy link
Member

axw commented Feb 18, 2021

Thanks @jpopadak. Seems reasonable to create a transaction/span for the entire stream. Maybe in the future we could extend this to log events for individual messages written to and read from the stream, if we implement something like #818.

@axw axw added the enhancement New feature or request label Feb 18, 2021
@jpopadak
Copy link
Author

Yeah, the only thing I worry about is the max number of spans. It might be hit quicker for Stream APIs.
For example, one of our services streams its data using thousands of messages on a "single stream".

@axw
Copy link
Member

axw commented Mar 25, 2021

@jpopadak if you have a chance to test this out some time soon I'd appreciate any feedback you may have, so we can make changes before the next release as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-go enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants