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 integration #476

Merged
merged 68 commits into from
Mar 31, 2020
Merged

gRPC integration #476

merged 68 commits into from
Mar 31, 2020

Conversation

c24t
Copy link
Member

@c24t c24t commented Mar 12, 2020

This is a port of grpcio-opentracing, and borrows from opencensus-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 pattern
  • route_guide (source, docs) which includes examples for unary-unary, client-streaming, server-streaming, and bidirectional streaming

Running the examples

% cd ext/opentelemetry-ext-grpc/examples
% ./hello_world_server.py
Span(name="/helloworld.Greeter/SayHello", context=SpanContext(trace_id=0xdc68d08b983225bf3b86eabfc9a9f02c, span_id=0x12aa95fc91338970, trace_state={}), kind=SpanKind.SERVER, parent=<opentelemetry.trace.DefaultSpan object at 0x10943d190>, start_time=2020-03-25T00:23:31.605991Z, end_time=2020-03-25T00:23:31.606155Z)
% ./hello_world_client.py
Span(name="/helloworld.Greeter/SayHello", context=SpanContext(trace_id=0xdc68d08b983225bf3b86eabfc9a9f02c, span_id=0xb4766ea78457996c, trace_state={}), kind=SpanKind.CLIENT, parent=None, start_time=2020-03-25T00:23:31.574011Z, end_time=2020-03-25T00:23:31.606996Z)
Greeter client received: Hello, YOU!

Note that the client's trace ID gets propagated to the server.

You should see something similar running ./route_guide_server.py and route_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 from grpcio-opentracing. It implemented interceptors before grpc-python had proper support for them. grpc-python has since added interceptors, but grpcio-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

@andrewhsu andrewhsu mentioned this pull request Mar 16, 2020
@c24t c24t marked this pull request as ready for review March 25, 2020 00:17
@c24t c24t requested a review from a team March 25, 2020 00:17
Copy link
Contributor

@codeboten codeboten left a 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.

@johananl
Copy link
Contributor

I've experimented with the code by running a Python gRPC server with a Go gRPC client (code is here and here). Seems to work nicely 👍 Hope this helps.

@c24t
Copy link
Member Author

c24t commented Mar 30, 2020

Thanks @johananl and @codeboten!

@toumorokoshi
Copy link
Member

grpc.txt

trying to figure out how to send PRs over, but here's a patch for tox directives.

Copy link
Member

@toumorokoshi toumorokoshi left a 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!

@c24t c24t merged commit 54b390f into open-telemetry:master Mar 31, 2020
@c24t c24t deleted the grpc-again branch March 31, 2020 01:04
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.

9 participants