Topher c/apmsvls 52 fixing payload tagging
What does this PR do?
This fixes a couple of issues with payload tagging logic. Currently if using an invalid json path can break tracing.
After investigating the reason why the trace breaks is due to trying to write to property that doesn't exist. This cannot be evaluated at configuration time as we do not have the user's JSON schema. So I have just implemented a check to see if the the parent and parentProperty exist before trying to set this.
This is based on the stacktrace generated:
2025-09-04T19:16:13.181Z 1d5edca0-30cc-4b4f-9153-98b480162b26 ERROR [dd.trace_id=5438775504825836037 dd.span_id=5438775504825836037] Uncaught Exception
{"errorType":"TypeError","errorMessage":"Cannot set properties of null (setting 'null')","stack":["TypeError: Cannot set properties of null (setting 'null')"," at /var/task/node_modules/dd-trace/packages/dd-trace/src/payload-tagging/index.js:56:42","
at JSONPath._handleCallback (/var/task/node_modules/jsonpath-plus/dist/index-node-cjs.cjs:1660:5)"," at JSONPath._trace (/var/task/node_modules/jsonpath-plus/dist/index-node-cjs.cjs:1688:10)"," at JSONPath._trace
(/var/task/node_modules/jsonpath-plus/dist/index-node-cjs.cjs:1758:17)"," at JSONPath.evaluate
(/var/task/node_modules/jsonpath-plus/dist/index-node-cjs.cjs:1611:23)"," at new JSONPath
(/var/task/node_modules/jsonpath-plus/dist/index-node-cjs.cjs:1554:22)"," at JSONPath
(/var/task/node_modules/jsonpath-plus/dist/index-node-cjs.cjs:1514:14)"," at redact (/var/task/node_modules/dd-
trace/packages/dd-trace/src/payload-tagging/index.js:55:7)"," at computeTags (/var/task/node_modules/dd-
trace/packages/dd-trace/src/payload-tagging/index.js:84:3)"," at tagsFromRequest (/var/task/node_modules/dd-
trace/packages/dd-trace/src/payload-tagging/index.js:89:10)"]}
Motivation
I want to make the use of this feature to be more intuitive along with catching when the user makes a mistake so that they are not questioning why their payloads are redacting or in this case their whole trace disappears.
Plugin Checklist
payload-tagging
- [ ] Unit tests.
- [ ] Integration tests.
- [ ] Benchmarks.
- [ ] TypeScript definitions.
- [ ] TypeScript tests.
- [ ] API documentation.
- [ ] CI jobs/workflows.
Additional Notes
Overall package size
Self size: 4.39 MB Deduped: 5.21 MB No deduping: 5.21 MB
Dependency sizes
| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 2.0.0 | 68.46 kB | 797.03 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 84.49%. Comparing base (b00ab50) to head (2803e27).
:warning: Report is 1 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #6358 +/- ##
=======================================
Coverage 84.49% 84.49%
=======================================
Files 523 523
Lines 22447 22449 +2
=======================================
+ Hits 18966 18968 +2
Misses 3481 3481
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Benchmarks
Benchmark execution time: 2025-12-22 17:29:38
Comparing candidate commit 2803e27034bb6acd874f0c7e81f897c39ddbcee7 in PR branch TopherC/APMSVLS-52-Fixing-payload-tagging with baseline commit b00ab5001877f2142c405f9ed5812e07f6e1772a in branch master.
Found 0 performance improvements and 0 performance regressions! Performance is the same for 292 metrics, 28 unstable metrics.
The code is LGTM, we would just also need some tests. Do you know what happens in case a user passes through faulty rules?
@BridgeAR -- this will just return. Because the filter is incorrect there is nothing to redact or capture. Today we're trying to assign using a null property as a key.
Hey @BridgeAR -- I see there is payload-tagging tests residing in here. Is this sufficient or should I add more testing?
Thanks, ~Topher
⚠️ Warnings
🧪 1 Test failed
❄️ Known flaky:
tests.appsec.waf.test_telemetry.Test_TelemetryMetrics.test_waf_requests_match_traced_requests[uds-express4]fromsystem_tests_suite(Datadog) (Fix with Cursor)AssertionError: Number of requests in traces do not match waf.requests metric total assert 5 == 4 self = <tests.appsec.waf.test_telemetry.Test_TelemetryMetrics object at 0x7f7d48dcc0b0> @bug(context.library < "[email protected]", reason="APPSEC-51509") def test_waf_requests_match_traced_requests(self): """Total waf.requests metric should match the number of requests in traces.""" spans = [s for _, s in interfaces.library.get_root_spans()] spans = [ ...
ℹ️ Info
❄️ No new flaky tests detected
This comment will be updated automatically if new data arrives.🔗 Commit SHA: 2803e27 | Docs | Datadog PR Page | Was this helpful? Give us feedback!
I change the commit title to something a little cleaner. It's not clear what payload tagging is though so the title could probably use more love. At any rate when squashing please ensure the commit title is well formed.
Oh, this is the AWS payload tagging project I worked on a while ago. I'll update the title to include the text "AWS" as that seems relevant.