zio-grpc
zio-grpc copied to clipboard
Propagate current gRPC Context inside call environment
Hello, @thesamet
I've encountered a problem while integrating my service with a third-party (opentracing) ServerCallInterceptor
, which propagates actual Span
via gRPC Context
mechanism.
Unfortunately, Context.current()
is unavailable inside service implementation because it's backed by ThreadLocal
inside gRPC and needs to be propagated from calling gRPC thread.
This tiny change allows us to capture current Context
inside RequestContext
environment.
I didn't know what to do with Scala.js dependency and added a dummy definition for it just like you did for Metadata
in scalapb-grpcweb
.
Hope you can look into it soon.
Cheers, Andrey
Hey Andrey, thanks for putting this together. I looked into io.grpc.Context
in the past, and if I recall correctly it does require special handling with ZIO due to it relying on thread local storage. For this reason, I would prefer to avoid introducing it to zio-grpc public API.
In the past, @frekw reported success integrating with zio-telementry, see some example code in #59 - can you look into this and check if this approach will be useful?
Hey Andrey, thanks for putting this together. I looked into
io.grpc.Context
in the past, and if I recall correctly it does require special handling with ZIO due to it relying on thread local storage. For this reason, I would prefer to avoid introducing it to zio-grpc public API.In the past, @frekw reported success integrating with zio-telementry, see some example code in #59 - can you look into this and check if this approach will be useful?
Actually, default Context.Storage
is backed by ThreadLocal
, but Context
itself is not. Context
is an effectively immutable storage for propagating custom payloads like tracing information or security principals. I'm aware of meaningless of using static Context.current()
inside of user code of zio-grpc, but I'm proposing to capture current state inside ZServerCallHandler.startCall
(which is the last point that is getting executed inside gRPC thread pool) and propagate an effectively immutable instance of current Context
to environment of user code.
We may also introduce a wrapper like SafeMetadata
if it would be appropriate. I'll look into it today.
@thesamet I've made it even better.
- Introduced package-private
SafeContext
trait for interop with JS (goodbye, dummyio.grpc.Context
!) - Made an
SafeContext.Key[R, E, A]
which should carry an effect for the default value (may be useful if you wish to compute value from an environment like falling back to root span in telemetry or indicate an appropriate error if needed).
What do you think about this approach?
Taking a step back - I assume you need the context so you can extract the span and add additional traces from within your service - is that right? I prefer that we also evaluate a solution where we can integrate with zio-telemetry so we end up with a solution that tracing can be done more consistently in the ZIO ecosystem.
Taking a step back - I assume you need the context so you can extract the span and add additional traces from within your service - is that right? I prefer that we also evaluate a solution where we can integrate with zio-telemetry so we end up with a solution that tracing can be done more consistently in the ZIO ecosystem.
Well, that's not the only purpose to use Context
. If we rely somehow on grpc-java
, it may be useful to have benefits of low-level access to protocol flow via interceptors. Context
is a just a most convenient way to propagate something from them.
I think zio-telemetry
integration must be done separately and may rely on requirement of SafeContext
.
@thesamet It's been a while since we've looked into this issue.
Do you still think it's a bad idea to introduce abstract API over which we may implement zio-telemetry
integration or maybe this change is good enough to go?
I am hesitant about introducing this change. The value add is that it enables using existing grpc-java interceptors, on other other hand, it introduces a class that only partially works and we need to make some effort to hide it.
The criteria I am looking for is whether there are real-world use cases that absolutely requires access to the Context. I haven't had a chance to look into it yet, but for opentracing, I think we get away by implementing our own ZServerInterceptor in zio.