Repository navigation
fix(appsec): stop counting WAF and RASP timeouts as errors - #5828
Conversation
One WAF or RASP timeout was counted two times. `RegisterWafRun` counted
it as a timeout (`rasp.timeout`, `waf.requests{waf_timeout:true}`). Then
`IncWafError` also counted it as an error (`rasp.error{waf_error:4}`,
`waf.error{waf_error:4}`) and stored code 4 in `RASPErrorCodes` and
`WAFErrorCode`. These codes feed the `_dd.appsec.rasp.error` and
`_dd.appsec.waf.error` span tags. A timeout code could also hide a real
error code in these tags.
In `IncWafError`, skip only the `raspError` and `wafError` calls when
the error matches `waferrors.ErrTimeout`. Keep the scope and rule type
checks, so a timeout with an unknown scope or an unknown RASP rule type
still records an instrumentation exception. `run.go` and
`RegisterWafRun` do not change.
Add tests for: bare and wrapped timeouts through `runWAF`; real errors
that are still counted; an error and a timeout in the same request, in
both orders; the unknown scope and unknown rule type cases; and the
span tags.
APPSEC-71527
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 97d62fc | Docs | View more details | Give us feedback! |
BenchmarksBenchmark execution time: 2026-11-05 16:10:56 Comparing candidate commit 97d62fc in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 413 metrics, 1 unstable metrics.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97d62fc3c1
ℹ️ 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".
![datadog-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Bare and wrapped timeouts leave error counters and error-code span tags unset, while genuine errors remain recorded when a timeout occurs in the same request. Scope and rule-type validation remain intact.
🤖 Bits Code Review · Commit 97d62fc
Resolve the conflicts with the timeout rework of #5828: - keep main's IncWafError, and the A2 code of this branch - keep main's TestIncWafErrorTimeout and the shared test helpers, and drop the tests of this branch that duplicate them - update the tests of #5828 that assert the old positive error codes
Replace the section with the A2 content only: timeouts are handled by #5828, so the section no longer describes them.
What does this PR do?
Stops counting a WAF or RASP timeout as an error in Go AppSec telemetry.
Before, one timeout was counted two times:
RegisterWafRuncounted it as a timeout:rasp.timeout(RASP) orwaf.requests{waf_timeout:true}(WAF scope).IncWafErrorcounted it again as an error:rasp.error{waf_error:4}orwaf.error{waf_error:4}. It also stored code4inRASPErrorCodes/WAFErrorCode. These codes set the_dd.appsec.rasp.errorand_dd.appsec.waf.errorspan tags. The code4of a timeout could also hide the code of a real error in these tags.Now,
IncWafErrorskips only theraspError/wafErrorcalls whenerrors.Is(err, waferrors.ErrTimeout). There is no early return. The scope and rule type checks stay, so a timeout with an unknown scope or an unknown RASP rule type still records an instrumentation exception.run.goandRegisterWafRundo not change.Tests:
TestRunWAFTimeoutIsNotAnError(throughrunWAF): bare and wrapped timeouts for RASP and WAF scope; unknown rule type and unknown scope with a timeout;ErrInternaland a non-RunErrorare still counted.TestRunWAFErrorAndTimeoutOrder: an error and a timeout in the same request, in both orders, for RASP and WAF scope.TestIncWafErrorTimeout: direct test ofIncWafError.TestWAFErrorCodeSpanTag: a timeout sets no error tag; a timeout does not hide a real error.Motivation
Action 3 of the DD-17 report (internal). Jira: APPSEC-71527.
Known limit, not changed here: on
main, a RASP timeout with an unknown rule type is not counted inrasp.timeout, becauseRegisterWafRunreturns before the timeout count. The instrumentation exception is still recorded.Out of scope: the 2 ms default timeout and a separate RASP budget (APPSEC-62654), and the docs fix.
Release note
AppSec telemetry: a WAF or RASP timeout is no longer also counted as an error.
rasp.error{waf_error:4}(RASP) orwaf.error{waf_error:4}(WAF scope).rasp.timeout; WAF-scope timeouts markwaf.requests{waf_timeout:true}. They no longer incrementrasp.error/waf.error. The span tags_dd.appsec.waf.errorand_dd.appsec.rasp.errorare no longer set for a timeout._dd.appsec.waf.timeoutsand_dd.appsec.rasp.timeoutdo not change.rasp.errorandwaf.errordrops. Thewaf_error:4series goes to zero. Monitors and dashboards that use these metrics will change when the services upgrade. To follow timeouts, userasp.timeout(RASP) andwaf.requests{waf_timeout:true}(WAF scope).Jira
APPSEC-71527
Red → green proof
Command (same in both runs):
The cases with a valid scope and rule type that contain a timeout fail without the fix. The unknown-scope and unknown-rule-type cases pass in both runs: they guard the instrumentation exception.
Red: new tests on
main, without the fix inmetrics.goFailure reasons:
rasp.error/waf.error= 1 instead of 0; error code4instead of 0 (or instead of1when anErrInternalwas also seen);_dd.appsec.waf.error/_dd.appsec.rasp.errorset to4.Green: same tests with the fix
Reviewer's Checklist
IncWafError. The new unit tests check the classification, the submitted metric values and the span tag values. The code that sends telemetry and span tags does not change. No existing system test readsrasp.error,waf.error,_dd.appsec.waf.erroror_dd.appsec.rasp.error; those tests do not check the corrected metrics. A cross-tracer system test is a possible follow-up inDataDog/system-tests.golangci-lint run ./internal/appsec/...reports 0 issues.go test ./internal/appsec/...passes, with and withoutDD_APPSEC_ENABLED=true.appsec)make generatelocally. (No generated file is affected.)make fix-moduleslocally. (No go.mod change.)