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 server instrumentation docs are inadequate and misleading #163

Closed
alertedsnake opened this issue Sep 16, 2020 · 1 comment
Closed

Comments

@alertedsnake
Copy link
Contributor

I recently submitted open-telemetry/opentelemetry-python#1113 to fix a missing line from the docs, but I've been digging more into this and I'm not sure that was the right choice.

The docs have this sequence:

grpc_server_instrumentor = GrpcInstrumentorServer()
grpc_server_instrumentor.instrument()

I added this line in open-telemetry/opentelemetry-python#1113:

server = intercept_server(server, server_interceptor())

It turns out that the instrument() method there does some magic by wrapping grpc.server in the same way my added line does. The reason I added this line in the docs, was that I wasn't calling grpc.server() after this instrument() call, I'd already created the server some lines before. And then nothing worked until I specifically called the wrapper.

So as the docs are written, this probably needs a revert, but also as they're written, nothing makes it clear what hidden effects are actually going on here, and so it's really easy to miss. Like I did.

So my bug report is that this very very basic example isn't enough for someone to go on for anything but running the basic example itself, especially when there isn't really any other useful documentation on how these components work, and there are many hidden behind-the-scenes effects that aren't immediately clear.

srikanthccv referenced this issue in srikanthccv/opentelemetry-python Nov 1, 2020
@lzchen lzchen transferred this issue from open-telemetry/opentelemetry-python Nov 9, 2020
@alertedsnake
Copy link
Contributor Author

As far as I'm concerned, the primary issue I was referring to got fixed in open-telemetry/opentelemetry-python#1171, so it this can probably just be closed.

@aabmass aabmass closed this as completed Mar 17, 2021
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

2 participants