Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(distributed_tracing): ensure ASM standalone operates on all propagation styles #11875

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions ddtrace/propagation/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,6 @@ def _inject(span_context, headers):
log.debug("tried to inject invalid context %r", span_context)
return

# When in appsec standalone mode, only distributed traces with the `_dd.p.appsec` tag
# are propagated. If the tag is not present, we should not propagate downstream.
if asm_config._appsec_standalone_enabled and (APPSEC.PROPAGATION_HEADER not in span_context._meta):
return

if span_context.trace_id > _MAX_UINT_64BITS:
# set lower order 64 bits in `x-datadog-trace-id` header. For backwards compatibility these
# bits should be converted to a base 10 integer.
Expand Down Expand Up @@ -355,16 +350,6 @@ def _extract(headers):
if meta:
meta = validate_sampling_decision(meta)

if asm_config._appsec_standalone_enabled:
# When in appsec standalone mode, only distributed traces with the `_dd.p.appsec` tag
# are propagated downstream, however we need 1 trace per minute sent to the backend, so
# we unset sampling priority so the rate limiter decides.
if not meta or APPSEC.PROPAGATION_HEADER not in meta:
sampling_priority = None
# If the trace has appsec propagation tag, the default priority is user keep
elif meta and APPSEC.PROPAGATION_HEADER in meta:
sampling_priority = 2 # type: ignore[assignment]

return Context(
# DEV: Do not allow `0` for trace id or span id, use None instead
trace_id=trace_id or None,
Expand Down Expand Up @@ -1097,6 +1082,11 @@ def parent_call():

_inject_llmobs_parent_id(span_context)

# When in appsec standalone mode, only distributed traces with the `_dd.p.appsec` tag
# are propagated. If the tag is not present, we should not propagate downstream.
if asm_config._appsec_standalone_enabled and (APPSEC.PROPAGATION_HEADER not in span_context._meta):
return

if PROPAGATION_STYLE_DATADOG in config._propagation_style_inject:
_DatadogMultiHeader._inject(span_context, headers)
if PROPAGATION_STYLE_B3_MULTI in config._propagation_style_inject:
Expand Down Expand Up @@ -1162,6 +1152,16 @@ def my_controller(url, headers):
else:
context = baggage_context

if asm_config._appsec_standalone_enabled:
# When in appsec standalone mode, only distributed traces with the `_dd.p.appsec` tag
# are propagated downstream, however we need 1 trace per minute sent to the backend, so
# we unset sampling priority so the rate limiter decides.
if not context._meta or APPSEC.PROPAGATION_HEADER not in context._meta:
context.sampling_priority = None
# If the trace has appsec propagation tag, the default priority is user keep
elif context and APPSEC.PROPAGATION_HEADER in context._meta:
context.sampling_priority = 2

return context

except Exception:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
fixes:
- |
ASM: Fixed an issue where distributed tracing headers were improperly propagated when tracing was disabled. Now, headers are only propagated for traces with the _dd.p.appsec tag, regardless of the propagation style (e.g., TraceContext). Previously, this logic applied only to Datadog headers, potentially allowing invalid trace info to be injected or extracted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify - previously we were only propagating ASM distributed tracing headers on Datadog headers regardless if tracing was enabled/disabled? Is this because DatadogMultiHeaders._inject() doesn't account for tracing being enabled/disabled?

Loading