fix(grpc): handle NotImplementedError from add_done_callback in aio client#4429
fix(grpc): handle NotImplementedError from add_done_callback in aio client#4429alliasgher wants to merge 5 commits into
Conversation
|
This is missing a test for checking this is working fine now and conflict handling in the CHANGELOG |
alliasgher
force-pushed
the
fix-grpc-aio-add-done-callback
branch
from
fe37463 to
c788d60
Compare
|
I think the comment about #47040 was meant for a different PR (it's a collector-contrib PR fixing the azurefunctions config round-trip, unrelated to this grpc aio fix). Happy to proceed here if that's correct. |
|
Added a regression test in 2555c4d that patches |
MikeGoldsmith
left a comment
MikeGoldsmith
left a comment
There was a problem hiding this comment.
Change looks good. I've left a suggestion for updating the changelog entry.
Also, please can you update the PR description to use the template we have for consistency.
…open-telemetry#3915 Signed-off-by: Ali <alliasgher123@gmail.com>
alliasgher
force-pushed
the
fix-grpc-aio-add-done-callback
branch
from
3b9d229 to
f21c4a7
Compare
MikeGoldsmith
left a comment
MikeGoldsmith
left a comment
There was a problem hiding this comment.
Looks good - thanks @alliasgher.
github-project-automation
Bot
moved this from Reviewed PRs that need fixes
to Approved PRs
in Python PR digest
|
This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment. |
@MikeGoldsmith approved but still marked stale |
|
Thanks for the PR! Just a heads-up: we no longer update Please add the appropriate changelog fragment for this change instead of editing |
aabmass
moved this from Approved PRs
to Reviewed PRs that need fixes
in Python PR digest
|
This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment. |
|
Superseded by #4672 - fixed formatting and lint issues. |
…lient When a user provides a custom grpc.aio interceptor that awaits the call object (e.g. `call = await continuation(...); return await call`), the resulting call object may not implement add_done_callback, raising NotImplementedError. Catch the exception and invoke the callback immediately in that case. The code and details are already available at that point so the span can still be finalised correctly. Fixes open-telemetry#3915 Signed-off-by: alliasgher <alliasgher123@gmail.com>
Verifies that spans are correctly finished when the grpc.aio call object raises NotImplementedError from add_done_callback, exercising the fallback path that calls the callback directly. Signed-off-by: alliasgher <alliasgher123@gmail.com>
CHANGELOG.md is generated by towncrier now; replace the manual edit with .changelog/4429.fixed per CONTRIBUTING.md. Signed-off-by: Ali <alliasgher123@gmail.com>
alliasgher
force-pushed
the
fix-grpc-aio-add-done-callback
branch
from
a7747b7 to
e3cec30
Compare
6 participants
Description
When a user-provided
grpc.aiointerceptor awaits the call object, the resulting type may not implementadd_done_callback— calling it raisesNotImplementedError. Catch the exception and invoke the callback immediately with the already-availablecodeanddetails, which matches the synchronous interceptor path and keeps span finalisation working.Fixes #3915
Type of change
How Has This Been Tested?
Added
test_done_callback_not_implementedintest_aio_client_interceptor.pywhich replacesadd_done_callbackwith a function that raisesNotImplementedErrorand asserts the span still completes with the correct status.Checklist
Unreleased / Fixed, linking to PR fix(grpc): handle NotImplementedError from add_done_callback in aio client #4429