[core] Add WSGI http header support to HTTP propagator #522
Conversation
|
Note: We should still try to run a WSGI server in our tests to replicate this behaviour to prevent future regressions. |
tests/propagation/test_http.py
Outdated
| tracer = get_dummy_tracer() | ||
|
|
||
| headers = { | ||
| get_wsgi_header(HTTP_HEADER_TRACE_ID): '1234', |
There was a problem hiding this comment.
Use a real input and not get_wsgi_header(), otherwise if the implementation changes, this test will not fail.
tests/propagation/test_http.py
Outdated
| with tracer.trace("local_root_span") as span: | ||
| eq_(span.trace_id, 1234) | ||
| eq_(span.parent_id, 5678) | ||
| # TODO: do it for priority too |
tests/propagation/test_http.py
Outdated
| @@ -47,3 +48,22 @@ def test_extract(self): | |||
| eq_(span.trace_id, 1234) | |||
| eq_(span.parent_id, 5678) | |||
| # TODO: do it for priority too | |||
There was a problem hiding this comment.
Here too, and in all other occurrences.
ddtrace/propagation/utils.py
Outdated
| @@ -0,0 +1,7 @@ | |||
|
|
|||
There was a problem hiding this comment.
no empty lines at the beginning of our modules. Task for the future: enforce a strict flake8 ruleset
tests/propagation/test_http.py
Outdated
| HTTP_HEADER_TRACE_ID: '1234', | ||
| HTTP_HEADER_PARENT_ID: '5678', | ||
| HTTP_HEADER_SAMPLING_PRIORITY: '1', | ||
| HTTP_HEADER_TRACE_ID: "1234", |
There was a problem hiding this comment.
should we use HTTP_X_DATADOG_TRACE_ID also here?
There was a problem hiding this comment.
This test is semantically unchanged.
To test the "HTTP_X_DATADOG_TRACE_ID" key we have the test_WSGI_extract test method below.
There was a problem hiding this comment.
My point is to not use a variable but a plain string that detects any change in our code. In general the input must not be generated using our code otherwise the test is not reliable (it changes with our code changes so you don't detect if the software is different). It should be x-datadog-trace-id or similar.
There was a problem hiding this comment.
Ohhh, okay. I didn't realize that you wanted me to change the functionality on the existing test. I'll make this change.
|
@palazzem this should be good to go! |
WSGI specification states for variable HTTP headers to be in a particular format stated here.
This PR adds support to the HTTP Propagator to support if and when the Datadog headers are converted to this format.
This should fix #456.