Skip to content

Commit

Permalink
Merge branch '2.9' into backport-10021-to-2.9
Browse files Browse the repository at this point in the history
  • Loading branch information
vitor-de-araujo authored Aug 2, 2024
2 parents f02c60b + bf946b7 commit 78cfe57
Show file tree
Hide file tree
Showing 10 changed files with 169 additions and 70 deletions.
25 changes: 16 additions & 9 deletions ddtrace/appsec/_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def _get_rate_limiter() -> RateLimiter:

@dataclasses.dataclass(eq=False)
class AppSecSpanProcessor(SpanProcessor):
rules: str = dataclasses.field(default_factory=get_rules)
rule_filename: str = dataclasses.field(default_factory=get_rules)
obfuscation_parameter_key_regexp: bytes = dataclasses.field(
default_factory=get_appsec_obfuscation_parameter_key_regexp
)
Expand All @@ -159,28 +159,35 @@ def __post_init__(self) -> None:
from ddtrace.appsec._ddwaf import DDWaf

try:
with open(self.rules, "r") as f:
rules = json.load(f)
with open(self.rule_filename, "r") as f:
self._rules = json.load(f)

except EnvironmentError as err:
if err.errno == errno.ENOENT:
log.error("[DDAS-0001-03] ASM could not read the rule file %s. Reason: file does not exist", self.rules)
log.error(
"[DDAS-0001-03] ASM could not read the rule file %s. Reason: file does not exist",
self.rule_filename,
)
else:
# TODO: try to log reasons
log.error("[DDAS-0001-03] ASM could not read the rule file %s.", self.rules)
log.error("[DDAS-0001-03] ASM could not read the rule file %s.", self.rule_filename)
raise
except JSONDecodeError:
log.error("[DDAS-0001-03] ASM could not read the rule file %s. Reason: invalid JSON file", self.rules)
log.error(
"[DDAS-0001-03] ASM could not read the rule file %s. Reason: invalid JSON file", self.rule_filename
)
raise
except Exception:
# TODO: try to log reasons
log.error("[DDAS-0001-03] ASM could not read the rule file %s.", self.rules)
log.error("[DDAS-0001-03] ASM could not read the rule file %s.", self.rule_filename)
raise
try:
self._ddwaf = DDWaf(rules, self.obfuscation_parameter_key_regexp, self.obfuscation_parameter_value_regexp)
self._ddwaf = DDWaf(
self._rules, self.obfuscation_parameter_key_regexp, self.obfuscation_parameter_value_regexp
)
if not self._ddwaf._handle or self._ddwaf.info.failed:
stack_trace = "DDWAF.__init__: invalid rules\n ruleset: %s\nloaded:%s\nerrors:%s\n" % (
rules,
self._rules,
self._ddwaf.info.loaded,
self._ddwaf.info.errors,
)
Expand Down
17 changes: 11 additions & 6 deletions ddtrace/appsec/_remoteconfiguration.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ def _add_rules_to_list(features: Mapping[str, Any], feature: str, message: str,
if rules is not None:
try:
if ruleset.get(feature) is None:
ruleset[feature] = []
ruleset[feature] += rules
ruleset[feature] = rules
else:
ruleset[feature] = ruleset[feature] + rules
log.debug("Reloading Appsec %s: %s", message, str(rules)[:20])
except json.JSONDecodeError:
log.error("ERROR Appsec %s: invalid JSON content from remote configuration", message)
Expand All @@ -114,14 +115,18 @@ def _appsec_rules_data(features: Mapping[str, Any], test_tracer: Optional[Tracer

if features and tracer._appsec_processor:
ruleset = {} # type: dict[str, Optional[list[Any]]]
_add_rules_to_list(features, "rules_data", "rules data", ruleset)
if features.get("rules", None) == []:
# if rules is empty, we need to switch back to the default rules
ruleset = tracer._appsec_processor._rules.copy() or {}
_add_rules_to_list(features, "actions", "actions", ruleset)
_add_rules_to_list(features, "custom_rules", "custom rules", ruleset)
_add_rules_to_list(features, "rules", "Datadog rules", ruleset)
_add_rules_to_list(features, "exclusions", "exclusion filters", ruleset)
_add_rules_to_list(features, "exclusion_data", "exclusion data", ruleset)
_add_rules_to_list(features, "processors", "processors", ruleset)
_add_rules_to_list(features, "rules", "Datadog rules", ruleset)
_add_rules_to_list(features, "rules_data", "rules data", ruleset)
_add_rules_to_list(features, "rules_override", "rules override", ruleset)
_add_rules_to_list(features, "scanners", "scanners", ruleset)
_add_rules_to_list(features, "processors", "processors", ruleset)
_add_rules_to_list(features, "actions", "actions", ruleset)
if ruleset:
return tracer._appsec_processor._update_rules({k: v for k, v in ruleset.items() if v is not None})

Expand Down
29 changes: 22 additions & 7 deletions ddtrace/opentelemetry/_span.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from opentelemetry.trace.span import TraceState

from ddtrace import config
from ddtrace import tracer as ddtracer
from ddtrace.constants import ERROR_MSG
from ddtrace.constants import ERROR_STACK
from ddtrace.constants import ERROR_TYPE
Expand Down Expand Up @@ -136,13 +137,27 @@ def kind(self):
def get_span_context(self):
# type: () -> SpanContext
"""Returns an OpenTelemetry SpanContext"""
ts = None
tf = TraceFlags.DEFAULT
if self._ddspan.context:
ts_str = w3c_tracestate_add_p(self._ddspan.context._tracestate, self._ddspan.span_id)
ts = TraceState.from_header([ts_str])
if self._ddspan.context.sampling_priority and self._ddspan.context.sampling_priority > 0:
tf = TraceFlags.SAMPLED
if self._ddspan.context.sampling_priority is None:
# With the introduction of lazy sampling, spans are now sampled on serialization. With this change
# a spans trace flags could be propagated before a sampling
# decision is made. Since the default sampling decision is to unsample spans this can result
# in missing spans. To resolve this issue, a sampling decision must be made the first time
# the span context is accessed.
ddtracer.sample(self._ddspan._local_root or self._ddspan)

if self._ddspan.context.sampling_priority is None:
tf = TraceFlags.DEFAULT
log.warning(
"Span context is missing a sampling decision, defaulting to unsampled: %s", str(self._ddspan.context)
)
elif self._ddspan.context.sampling_priority > 0:
tf = TraceFlags.SAMPLED
else:
tf = TraceFlags.DEFAULT

# Evaluate the tracestate header after the sampling decision has been made
ts_str = w3c_tracestate_add_p(self._ddspan.context._tracestate, self._ddspan.span_id)
ts = TraceState.from_header([ts_str])

return SpanContext(self._ddspan.trace_id, self._ddspan.span_id, False, tf, ts)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
fixes:
- |
ASM: This fix resolves an issue where the WAF could be disabled if the ASM_DD rule file was not found in Remote Config.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
fixes:
- |
opentelemetry: Resolves an edge case where distributed tracing headers could be generated before a sampling decision is made,
resulting in dropped spans in downstream services.
2 changes: 1 addition & 1 deletion tests/appsec/appsec/test_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def test_enable_custom_rules():
processor = AppSecSpanProcessor()

assert processor.enabled
assert processor.rules == rules.RULES_GOOD_PATH
assert processor.rule_filename == rules.RULES_GOOD_PATH


def test_ddwaf_ctx(tracer_appsec):
Expand Down
12 changes: 12 additions & 0 deletions tests/appsec/appsec/test_remoteconfiguration.py
Original file line number Diff line number Diff line change
Expand Up @@ -1033,3 +1033,15 @@ def test_rc_rules_data_error_ddwaf(tracer):
"rules": [{"invalid": mock.MagicMock()}],
}
assert not _appsec_rules_data(config, tracer)


def test_rules_never_empty(tracer):
with override_global_config(dict(_asm_enabled=True)):
tracer.configure(appsec_enabled=True, api_version="v0.4")
with mock.patch("ddtrace.appsec._processor.AppSecSpanProcessor._update_rules", autospec=True) as mock_update:
mock_update.reset_mock()
_appsec_rules_data({"rules": []}, tracer)
call = mock_update.mock_calls
args = call[-1][1][1]
assert "rules" in args
assert args["rules"], "empty rules should not be possible, it must switch to default."
58 changes: 53 additions & 5 deletions tests/opentelemetry/test_trace.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import mock
import opentelemetry
from opentelemetry.trace import set_span_in_context
from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator
import opentelemetry.version
import pytest

Expand Down Expand Up @@ -50,7 +52,7 @@ def test_otel_start_span_without_default_args(oteltracer):
root = oteltracer.start_span("root-span")
otel_span = oteltracer.start_span(
"test-start-span",
context=opentelemetry.trace.set_span_in_context(root),
context=set_span_in_context(root),
kind=opentelemetry.trace.SpanKind.CLIENT,
attributes={"start_span_tag": "start_span_val"},
links=None,
Expand Down Expand Up @@ -117,7 +119,7 @@ def test_otel_start_current_span_without_default_args(oteltracer):
with oteltracer.start_as_current_span("root-span") as root:
with oteltracer.start_as_current_span(
"test-start-current-span-no-defualts",
context=opentelemetry.trace.set_span_in_context(root),
context=set_span_in_context(root),
kind=opentelemetry.trace.SpanKind.SERVER,
attributes={"start_current_span_tag": "start_cspan_val"},
links=[],
Expand All @@ -138,6 +140,50 @@ def test_otel_start_current_span_without_default_args(oteltracer):
otel_span.end()


def test_otel_get_span_context_sets_sampling_decision(oteltracer):
with oteltracer.start_span("otel-server") as otelspan:
# Sampling priority is not set on span creation
assert otelspan._ddspan.context.sampling_priority is None
# Ensure the sampling priority is always consistent with traceflags
span_context = otelspan.get_span_context()
# Sampling priority is evaluated when the SpanContext is first accessed
sp = otelspan._ddspan.context.sampling_priority
assert sp is not None
if sp > 0:
assert span_context.trace_flags == 1
else:
assert span_context.trace_flags == 0
# Ensure the sampling priority is always consistent
for _ in range(1000):
otelspan.get_span_context()
assert otelspan._ddspan.context.sampling_priority == sp


def test_distributed_trace_inject(oteltracer): # noqa:F811
with oteltracer.start_as_current_span("test-otel-distributed-trace") as span:
headers = {}
TraceContextTextMapPropagator().inject(headers, set_span_in_context(span))
sp = span.get_span_context()
assert headers["traceparent"] == f"00-{sp.trace_id:032x}-{sp.span_id:016x}-{sp.trace_flags:02x}"
assert headers["tracestate"] == sp.trace_state.to_header()


def test_distributed_trace_extract(oteltracer): # noqa:F811
headers = {
"traceparent": "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01",
"tracestate": "congo=t61rcWkgMzE,dd=s:2",
}
context = TraceContextTextMapPropagator().extract(headers)
with oteltracer.start_as_current_span("test-otel-distributed-trace", context=context) as span:
sp = span.get_span_context()
assert sp.trace_id == int("0af7651916cd43dd8448eb211c80319c", 16)
assert span._ddspan.parent_id == int("b7ad6b7169203331", 16)
assert sp.trace_flags == 1
assert sp.trace_state.get("congo") == "t61rcWkgMzE"
assert "s:2" in sp.trace_state.get("dd")
assert sp.is_remote is False


@flaky(1717428664)
@pytest.mark.parametrize(
"flask_wsgi_application,flask_env_arg,flask_port,flask_command",
Expand All @@ -164,10 +210,12 @@ def test_otel_start_current_span_without_default_args(oteltracer):
"with_opentelemetry_instrument",
],
)
@pytest.mark.snapshot(ignores=["metrics.net.peer.port", "meta.traceparent", "meta.flask.version"])
@pytest.mark.snapshot(ignores=["metrics.net.peer.port", "meta.traceparent", "meta.tracestate", "meta.flask.version"])
def test_distributed_trace_with_flask_app(flask_client, oteltracer): # noqa:F811
with oteltracer.start_as_current_span("test-otel-distributed-trace"):
resp = flask_client.get("/otel")
with oteltracer.start_as_current_span("test-otel-distributed-trace") as span:
headers = {}
TraceContextTextMapPropagator().inject(headers, set_span_in_context(span))
resp = flask_client.get("/otel", headers=headers)

assert resp.text == "otel"
assert resp.status_code == 200
Loading

0 comments on commit 78cfe57

Please sign in to comment.