-
Notifications
You must be signed in to change notification settings - Fork 227
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
Add support for grpc aio server interceptor #1870
Add support for grpc aio server interceptor #1870
Conversation
💚 CLA has been signed |
I approved it so tests will run. |
Thanks @basepi! I saw some problems and fixed them. I also just tried to push an attempt at the tests. Not sure it will work, I haven't been able to setup my local dev environment yet, will try to do that sometime soon, but if you'd like to run the CI as it is! |
dd9d354
to
c7167fc
Compare
c7167fc
to
529a293
Compare
Turns out it was quite easy to run the tests locally, so I was able to fix some issues and now the tests are running properly. |
Ok, done |
import grpc | ||
|
||
# Check against the oldest version that I believe has the expected API | ||
if parse_version(grpc.__version__) >= parse_version("1.33.1") and hasattr(grpc, "aio"): |
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.
This is how I'm checking if we should try to instrument the async version, but I'm not sure what is the recommended way for doing this kind of thing here. I've thought of leaving just the check for the aio
module's existence (and perhaps add a check for a server
attribute inside grpc.aio
), but since I wrote the version check first, I just left it there too.
Async tracing is only supported from python 3.7.
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.
One small fix
Just let the instrumentation system attempt to import it, and abort if it fails.
@elasticmachine, run elasticsearch-ci/docs |
What does this pull request do?
Add support for grpc aio server (async) instrumentation using an interceptor mostly copied from the "regular", sync, version. For now it's still missing tests, but I'm creating the PR just to check if anything breaks in the CI.
Related issues
Closes #1869