Migrating compound extraction span link tests to weblog #3499
Conversation
mhlidd
changed the title
tests/test_distributed.py
Outdated
| assert False | ||
|
|
||
|
|
||
| def retrieve_span_links(span): |
There was a problem hiding this comment.
Can we reuse the existing retrieve span links function. We could move the original implementation out of parametric.utils and add it to a common directory.
tests/test_distributed.py
Outdated
| self.req = weblog.get("/make_distant_call", params={"url": "http://weblog:7777"}, headers=extract_headers) | ||
|
|
||
| def test_span_links_from_conflicting_contexts(self): | ||
| trace = [span for _, _, span in interfaces.library.get_spans(self.req, full_trace=True)] |
There was a problem hiding this comment.
Here can we retrieve all spans with links. We can put an if statement in the list concatenation.
tests/test_distributed.py
Outdated
| print(trace) | ||
| for span in trace: | ||
| links = retrieve_span_links(span) | ||
| if links != None: |
There was a problem hiding this comment.
Nit: There might be more than one span with span links. One with conflicting distributed tracing headers and the other with some other span relationship. I am wondering if we should check the trace id in the if statement. That way we can guard against false negatives.
mhlidd
requested review from
Kyle-Verhoog,
avara1986,
smola and
jandro996
and removed request for
a team
mhlidd
requested review from
zacharycmontoya,
cbeauchesne,
a team and
mabdinur
and removed request for
a team
mhlidd
force-pushed
the
mhlidd/migrate_span_links_to_weblog
branch
from
fb0d288
to
cb9eca1
Compare
| @@ -205,3 +206,35 @@ def get_free_port(): | |||
| except OSError: | |||
| port += 1 | |||
| raise IOError("no free ports") | |||
|
|
|||
|
|
|||
| def retrieve_span_links(span): | |||
There was a problem hiding this comment.
not sure if this is the best place to this method
There was a problem hiding this comment.
@robertomonteromiguel Do you have any suggestions on where helper functions should go? This is a function that I want to be accessible for both weblog and parametric tests.
| @@ -105,6 +105,24 @@ def all_endtoend_scenarios(test_object): | |||
| doc="Test W3C trace style", | |||
| ) | |||
|
|
|||
| trace_propagation_style_w3c_datadog_b3 = EndToEndScenario( | |||
There was a problem hiding this comment.
you should add the new scenarios here:. github/workflows/run-end-to-end.yml
Motivation
Currently, we test the results of compound extraction of inconsistent headers through parametric tests. However, using the parametric app does not truly reflect the behavior exhibited through automatic instrumentation, as we have to manually call extract on the headers passed in, create a span, and add the extracted context/span links to the span. This PR aims to migrate the parametric tests into weblog tests to remove the manual process of imitating this scenario, and have the system-tests represent what actually happens when we have compound extraction on a set of inconsistent headers.
Changes
Add 2 weblog scenarios that take in
DD_TRACE_PROPAGATION_STYLE_EXTRACTwith values of tracecontext -> datadog -> b3 and datadog -> tracecontext -> b3 respectively. The tests create a span through themake_distant_callendpoint and we verify that we get back a span that has the appropriate span links for different scenarios.APMAPI-899
Workflow
codeownersfile quickly.🚀 Once your PR is reviewed, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
[<language>], double-check that only<language>is impacted by the changebuild-XXX-imagelabel is present