Skip to content

appsec: RFC-1012 metrics improvements (duration distributions, rule_variant, tag fixes)#3850

Open
cataphract wants to merge 17 commits intomasterfrom
glopes/metrics-improv
Open

appsec: RFC-1012 metrics improvements (duration distributions, rule_variant, tag fixes)#3850
cataphract wants to merge 17 commits intomasterfrom
glopes/metrics-improv

Conversation

@cataphract
Copy link
Copy Markdown
Contributor

@cataphract cataphract commented Apr 30, 2026

Description

Implements and completes the RFC-1012 appsec telemetry metric improvements.

Duration metrics

  • waf.duration, waf.duration_ext, rasp.duration_ext are now submitted as
    DDSketch distributions (µs).
  • _dd.appsec.waf.duration_ext is added as a span metric (µs).
  • waf.duration_ext / rasp.duration_ext are measured by the PHP extension
    (not the helper) so that the request-shutdown round-trip is included in the
    value. The extension accumulates WAF + RASP round-trip durations in
    duration_acc.c/h and submits them directly to telemetry and as span metrics
    at request shutdown — rather than passing them through the helper — so the
    helper's own processing time is excluded from the measurement.
  • All duration values — both span metrics and telemetry distributions — are in
    microseconds.

Tag fixes and additions

  • waf.requests: fix rate_limited tag value when the request was rate-limited
    despite having an event.
  • waf.config_errors: add action tag (init / update); emit the metric and
    diagnostic logs for errors in bundled rules (previously only remote-config
    updates were covered).
  • rasp.rule.{eval,match,timeout}: add event_rules_version tag.
  • rasp.rule.* metrics: add optional rule_variant tag (e.g. distinguishes
    subtypes within a rule type). Protocol extended with a new optional msgpack
    field; PHP extension passes it as a third argument to push_addresses().
  • Rename sqlisql_injection in PDO/Mysqli/Filesystem integrations for
    consistency with other tracers.
  • Ensure all tags are always emitted regardless of value (RFC-1012 requirement).

Other

  • Update libdatadog to fix flakiness in helper metric submission.
  • Integration tests: Groovy DDSketch protobuf decoder to cross-check span metric
    values against distribution bins.
  • Deprecation notes for appsec.waf.input_truncated and
    appsec.waf.truncated_value_size.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Apr 30, 2026

Benchmarks [ appsec ]

Benchmark execution time: 2026-05-05 15:24:47

Comparing candidate commit d4bf924 in PR branch glopes/metrics-improv with baseline commit 82abdf3 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics.

@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Apr 30, 2026

Benchmarks [ tracer ]

Benchmark execution time: 2026-05-05 16:02:48

Comparing candidate commit d4bf924 in PR branch glopes/metrics-improv with baseline commit 82abdf3 in branch master.

Found 0 performance improvements and 4 performance regressions! Performance is the same for 189 metrics, 1 unstable metrics.

scenario:SamplingRuleMatchingBench/benchRegexMatching1

  • 🟥 execution_time [+66.982ns; +161.818ns] or [+4.589%; +11.086%]

scenario:SamplingRuleMatchingBench/benchRegexMatching2

  • 🟥 execution_time [+77.217ns; +168.983ns] or [+5.371%; +11.754%]

scenario:SamplingRuleMatchingBench/benchRegexMatching3

  • 🟥 execution_time [+59.228ns; +117.172ns] or [+4.049%; +8.010%]

scenario:SamplingRuleMatchingBench/benchRegexMatching4

  • 🟥 execution_time [+102.240ns; +199.560ns] or [+7.171%; +13.997%]

@cataphract cataphract force-pushed the glopes/metrics-improv branch 3 times, most recently from da0489c to a6d4548 Compare May 4, 2026 13:56
@datadog-datadog-prod-us1
Copy link
Copy Markdown

datadog-datadog-prod-us1 Bot commented May 4, 2026

Tests

Fix all issues with BitsAI or with Cursor

⚠️ Warnings

❄️ 5 New flaky tests detected

rasp duration span metrics and distributions are consistent() from com.datadog.appsec.php.integration.TelemetryTests   View in Datadog   (Fix with Cursor)
java.lang.AssertionError: rasp.duration distribution metric not found. Expression: (raspDuration != null). Values: raspDuration = null

java.lang.AssertionError: rasp.duration distribution metric not found. Expression: (raspDuration != null). Values: raspDuration = null
	at org.codehaus.groovy.runtime.InvokerHelper.createAssertError(InvokerHelper.java:416)
	at com.datadog.appsec.php.integration.TelemetryTests.rasp duration span metrics and distributions are consistent(TelemetryTests.groovy:959)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
waf config errors emitted with action init for bad init rules() from com.datadog.appsec.php.integration.TelemetryTests   View in Datadog   (Fix with Cursor)
Assertion failed: 

assert configError != null
       |           |
       null        false

Assertion failed: 

assert configError != null
       |           |
...
waf duration span metrics and distributions are consistent() from com.datadog.appsec.php.integration.TelemetryTests   View in Datadog   (Fix with Cursor)
java.lang.AssertionError: waf.duration distribution metric not found. Expression: (wafDuration != null). Values: wafDuration = null

java.lang.AssertionError: waf.duration distribution metric not found. Expression: (wafDuration != null). Values: wafDuration = null
	at org.codehaus.groovy.runtime.InvokerHelper.createAssertError(InvokerHelper.java:416)
	at com.datadog.appsec.php.integration.TelemetryTests.waf duration span metrics and distributions are consistent(TelemetryTests.groovy:720)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
View all

ℹ️ Info

No other issues found (see more)

🧪 All tests passed

🎯 Code Coverage (details)
Patch Coverage: 75.97%
Overall Coverage: 60.71% (+0.01%)

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: d4bf924 | Docs | Datadog PR Page | Give us feedback!

@cataphract cataphract marked this pull request as ready for review May 4, 2026 15:57
@cataphract cataphract requested review from a team as code owners May 4, 2026 15:58
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6d4548ef6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread appsec/helper-rust/src/client.rs Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6d4548ef6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread appsec/helper-rust/src/client.rs Outdated
@cataphract cataphract force-pushed the glopes/metrics-improv branch from a6d4548 to 50a1739 Compare May 5, 2026 08:33
@cataphract cataphract requested a review from a team as a code owner May 5, 2026 08:33
@cataphract cataphract force-pushed the glopes/metrics-improv branch from 50a1739 to 435d2b6 Compare May 5, 2026 08:41
cataphract and others added 16 commits May 5, 2026 10:50
… for bundled rules

- report_diagnostics_errors now takes rc_path as Option<&rc::RcPath> and an
  explicit action tag, so both the init path (action:init) and the update path
  (action:update) submit the waf.config_errors metric.
- The init path passes None for rc_path and constructs a synthetic
  ParsedConfigKey{product:"bundled_rules", config_id:"bundled_rules"} so
  diagnostic telemetry logs are also emitted for errors in the bundled rules.
- The init-path TelemetryLogsCollector is reused as the service's
  logs_collector so those logs are flushed on the first request.
- Integration test @order(21) verifies the metric (action:init,
  event_rules_version:9.9.9) and the diagnostic log
  (rc::bundled_rules::diagnostic) in a single drain loop, since
  drainTelemetry is destructive.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces rasp.c/h with duration_acc.c/h to accumulate both WAF and RASP
round-trip times measured by the extension. Sends them in request_shutdown
(now 6 args). Rust helper emits waf.duration, waf.duration_ext, and
rasp.duration_ext as DDSketch distributions, and _dd.appsec.waf.duration_ext
as a span metric. Adds integration tests with a Groovy DDSketch protobuf
decoder to cross-check span metric values against distribution bins.
…sql_injection

Adds rule_variant as a new optional field in the request_exec protocol
(msgpack map key "rule_variant", sent only when non-empty). The PHP
extension parses an optional third argument to push_addresses() and
packs it when present.

FilesystemIntegration passes "request" for SSRF pre-hooks. PDO and
Mysqli integrations rename "sqli" to "sql_injection" for consistency
with other tracers.

In the Rust helper, WafRunType::RaspRule becomes a struct variant
{ rule_type, rule_variant: Option<String> }. The rasp_per_rule HashMap
is keyed on (rule_type, rule_variant); the rule_variant tag is emitted
only when non-empty (the sidecar rejects tags ending with a bare colon).

Also adds event_rules_version to the three RASP per-rule metrics
(rasp.rule.eval, rasp.rule.match, rasp.timeout).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
It changed the clang-tidy version we were looking for locally but this
introduces new errors reported by clang-tidy, which were not handled.
This way request shutdown roundtrip can be accounted for in the
waf.duration_ext
@cataphract cataphract force-pushed the glopes/metrics-improv branch from 435d2b6 to 1ec9641 Compare May 5, 2026 09:50
@cataphract cataphract changed the title appsec: improvements in metrics appsec: RFC-1012 metrics improvements (duration distributions, rule_variant, tag fixes) May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant