-
Notifications
You must be signed in to change notification settings - Fork 647
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 integration #476
gRPC integration #476
Conversation
ext/opentelemetry-ext-grpc/src/opentelemetry/ext/grpc/client_interceptor.py
Outdated
Show resolved
Hide resolved
ext/opentelemetry-ext-grpc/src/opentelemetry/ext/grpc/client_interceptor.py
Outdated
Show resolved
Hide resolved
ext/opentelemetry-ext-grpc/src/opentelemetry/ext/grpc/_server.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of questions regarding some code that's commented out. I'm still trying to figure out if there's an underlying bug re. the context but that doesn't have to block this PR.
need a lock when initializing the context
Thanks @johananl and @codeboten! |
trying to figure out how to send PRs over, but here's a patch for tox directives. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not super knowledgeable on GRPC, but verified behavior worked as expected. Thanks!
This is a port of
grpcio-opentracing
, and borrows fromopencensus-ext-grpc
. It adds client and server interceptors that wrap each request in a span and use the new context API to inject/extract the traceparent header to/from gRPC metadata.The PR includes two examples copied from grpc-python itself:
helloworld
(source, docs) which shows how to instrument the client and server in a basic unary-unary call patternroute_guide
(source, docs) which includes examples for unary-unary, client-streaming, server-streaming, and bidirectional streamingRunning the examples
Note that the client's trace ID gets propagated to the server.
You should see something similar running
./route_guide_server.py
androute_guide_client.py
.Comparing to grpcio-opentracing
88e9467 copied some files from grpcio-opentracing. To see what changed between OpenTracing and OpenTelemetry, check the diff against 88e94672. The important files are
_client.py
and_server.py
.The
grpcext
package is copied fromgrpcio-opentracing
. It implemented interceptors beforegrpc-python
had proper support for them.grpc-python
has since added interceptors, butgrpcio-opentracing
still uses its own implementation.This PR still uses
grpcext
, but we should replace this with the official interceptor API in a follow up PR.See for background:
How to review
Please ignore the generated
_pb2.py
and_pb2_grpc.py
files. The meat of the PR is in_client.py
and_server.py
.Try running the tests and examples.
Still TODO
grpcext
.