Fix synthetics-derived contexts being ignored #856
Conversation
There was a problem hiding this comment.
This first iteration is the bare-minimum sort of implementation; a simple check and a simple integration test. As far as getting an immediate fix out, this is "okay" but really isn't net positive in terms of making the tracer code more manageable and less error prone.
It does not do a good job of identifying/addressing the source of the bug (parent IDs parsed to nil in HTTP propagation) and generally reveals some coupling and complexity that should be refactored. Some of these possible improvements are noted below.
| @@ -76,4 +76,53 @@ def lang_tag(span) | |||
| it { expect(@child_root_span).to be child_span } | |||
| end | |||
| end | |||
|
|
|||
| context 'with synthetics' do | |||
There was a problem hiding this comment.
I think it's a good idea to have a few top level integration tests like this: this bug manifested because we never actually had any basic tests to verify the most simple happy path for synthetics.
In the future, I'd like to extract integration tests like this to their own suite of sorts, separate from unit tests, as each kind of test serves an important purpose, but integration tests like this have a habit of creeping into the space unit tests should possess.
delner
force-pushed
the
fix/synthethics_distributed_tracing
branch
from
83eb348
to
ce7ba16
Compare
delner
force-pushed
the
fix/synthethics_distributed_tracing
branch
from
ce7ba16
to
14deece
Compare
|
Should be ready for final review @brettlangdon. |
| context 'with synthetics' do | ||
| context 'which applies the context from distributed tracing headers' do | ||
| let(:trace_id) { 3238677264721744442 } | ||
| let(:parent_id) { 0 } |
There was a problem hiding this comment.
Should we test when parent id header is not set at all?
There was a problem hiding this comment.
nvm, I do see one above in #extract test cases.
The following does not produce a distributed trace with the designated trace ID:
When synthetics traces are submitted via HTTP headers, they often are missing
parent_idor have a value of0, which is not a valid span ID, and thus the context'sparent_idbecomesnil.When the tracer encounters
parent_id = nil, it assumes the context is malformed and ignores its trace ID and parent ID, opting to use the ones generated from the local span instead. Ultimately this causes synthetics traces to go missing, because the trace IDs don't match what's sent by synthetics.This pull request ensures that the trace ID from synthetics is respected, regardless of what parent ID is, so that the resulting traces receive the correct trace ID.