RequestsInstrumentor: Add support for prepared requests#1040
RequestsInstrumentor: Add support for prepared requests#1040codeboten merged 5 commits intoopen-telemetry:masterfrom
Conversation
in addition to Session.request instrument Session.send to also create spans for prepared requests.
toumorokoshi
left a comment
toumorokoshi
left a comment
There was a problem hiding this comment.
LGTM! I think the test code can use a bit more of a refactor, but the execution looks good.
| currently not traced. If you find any other way to trigger an untraced HTTP | ||
| request, please report it via a GitHub issue with :code:`[requests: untraced | ||
| API]` in the title. | ||
| If you find any way to trigger an untraced HTTP request, please report it via a |
There was a problem hiding this comment.
Wondering if this comment even needs to be in the code? An uninstrumented trace feels like a self-evident issue.
| def instrumented_send(self, request, **kwargs): | ||
| def get_or_create_headers(): | ||
| request.headers = ( | ||
| request.headers if request.headers is not None else {} |
There was a problem hiding this comment.
this could also be request.headers or {}, although it comes with a little less type safety.
There was a problem hiding this comment.
an empty request.headers would actually change the type from CaseInsensitiveDict to a plain dict.
i guess in the else clause should also use a CaseInsensitiveDict
| httpretty.disable() | ||
|
|
||
|
|
||
| class TestRequestsIntegration(TestBase, RequestsIntegrationTestBase): |
There was a problem hiding this comment.
I believe you could actually incorporate this setup and teardown code into RequestsIntegrationTestBase.
abc.ABC is okay to use as a second parent class, and will still have the abstractmethod enforcing logic as if it was the first class passed.
Then you can just subclass everything from TestRequestsBase as before.
|
Hm.., |
3 participants
Description
The
RequestsInstrumentorwas currently only instrumetingSession.request. Sending a prepared request with a direct call toSession.sendwere not tracked.This PR adds instrumentation support for prepared requests by instrumenting
Session.sendin addition toSession.request.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Additional unit tests for sending prepared requests were added.
Checklist: