fix aws request/response payload tagging #9887
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🎯 Code Coverage 🔗 Commit SHA: 640bc68 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 53 metrics, 12 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.55.0-SNAPSHOT~640bc686bc, baseline=1.56.0-SNAPSHOT~1397f25f48
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.047 s) : 0, 1047282
Total [baseline] (8.622 s) : 0, 8622134
Agent [candidate] (1.048 s) : 0, 1047620
Total [candidate] (8.619 s) : 0, 8619371
section iast
Agent [baseline] (1.192 s) : 0, 1192060
Total [baseline] (9.252 s) : 0, 9251660
Agent [candidate] (1.196 s) : 0, 1195725
Total [candidate] (9.265 s) : 0, 9265458
gantt
title insecure-bank - break down per module: candidate=1.55.0-SNAPSHOT~640bc686bc, baseline=1.56.0-SNAPSHOT~1397f25f48
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.458 ms) : 0, 1458
crashtracking [candidate] (1.457 ms) : 0, 1457
BytebuddyAgent [baseline] (705.151 ms) : 0, 705151
BytebuddyAgent [candidate] (705.428 ms) : 0, 705428
GlobalTracer [baseline] (245.592 ms) : 0, 245592
GlobalTracer [candidate] (245.774 ms) : 0, 245774
AppSec [baseline] (32.382 ms) : 0, 32382
AppSec [candidate] (32.379 ms) : 0, 32379
Debugger [baseline] (6.386 ms) : 0, 6386
Debugger [candidate] (6.36 ms) : 0, 6360
Remote Config [baseline] (709.264 µs) : 0, 709
Remote Config [candidate] (723.006 µs) : 0, 723
Telemetry [baseline] (13.45 ms) : 0, 13450
Telemetry [candidate] (14.356 ms) : 0, 14356
Flare Poller [baseline] (7.447 ms) : 0, 7447
Flare Poller [candidate] (6.486 ms) : 0, 6486
section iast
crashtracking [baseline] (1.478 ms) : 0, 1478
crashtracking [candidate] (1.473 ms) : 0, 1473
BytebuddyAgent [baseline] (837.199 ms) : 0, 837199
BytebuddyAgent [candidate] (841.701 ms) : 0, 841701
GlobalTracer [baseline] (236.36 ms) : 0, 236360
GlobalTracer [candidate] (236.235 ms) : 0, 236235
IAST [baseline] (33.294 ms) : 0, 33294
IAST [candidate] (34.64 ms) : 0, 34640
AppSec [baseline] (29.122 ms) : 0, 29122
AppSec [candidate] (27.284 ms) : 0, 27284
Debugger [baseline] (6.055 ms) : 0, 6055
Debugger [candidate] (6.04 ms) : 0, 6040
Remote Config [baseline] (611.898 µs) : 0, 612
Remote Config [candidate] (610.006 µs) : 0, 610
Telemetry [baseline] (8.557 ms) : 0, 8557
Telemetry [candidate] (8.502 ms) : 0, 8502
Flare Poller [baseline] (4.185 ms) : 0, 4185
Flare Poller [candidate] (4.211 ms) : 0, 4211
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.55.0-SNAPSHOT~640bc686bc, baseline=1.56.0-SNAPSHOT~1397f25f48
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.049 s) : 0, 1049383
Total [baseline] (10.785 s) : 0, 10785389
Agent [candidate] (1.047 s) : 0, 1047292
Total [candidate] (10.795 s) : 0, 10795233
section appsec
Agent [baseline] (1.23 s) : 0, 1229938
Total [baseline] (10.913 s) : 0, 10913221
Agent [candidate] (1.226 s) : 0, 1226038
Total [candidate] (10.836 s) : 0, 10835665
section iast
Agent [baseline] (1.183 s) : 0, 1182892
Total [baseline] (11.128 s) : 0, 11128135
Agent [candidate] (1.182 s) : 0, 1181835
Total [candidate] (11.156 s) : 0, 11156203
section profiling
Agent [baseline] (1.196 s) : 0, 1196179
Total [baseline] (10.889 s) : 0, 10889158
Agent [candidate] (1.198 s) : 0, 1198438
Total [candidate] (10.909 s) : 0, 10908851
gantt
title petclinic - break down per module: candidate=1.55.0-SNAPSHOT~640bc686bc, baseline=1.56.0-SNAPSHOT~1397f25f48
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.459 ms) : 0, 1459
crashtracking [candidate] (1.462 ms) : 0, 1462
BytebuddyAgent [baseline] (706.326 ms) : 0, 706326
BytebuddyAgent [candidate] (705.317 ms) : 0, 705317
GlobalTracer [baseline] (246.309 ms) : 0, 246309
GlobalTracer [candidate] (245.418 ms) : 0, 245418
AppSec [baseline] (32.523 ms) : 0, 32523
AppSec [candidate] (32.403 ms) : 0, 32403
Debugger [baseline] (6.419 ms) : 0, 6419
Debugger [candidate] (6.401 ms) : 0, 6401
Remote Config [baseline] (706.161 µs) : 0, 706
Remote Config [candidate] (711.519 µs) : 0, 712
Telemetry [baseline] (12.843 ms) : 0, 12843
Telemetry [candidate] (15.126 ms) : 0, 15126
Flare Poller [baseline] (8.013 ms) : 0, 8013
Flare Poller [candidate] (5.761 ms) : 0, 5761
section appsec
crashtracking [baseline] (1.448 ms) : 0, 1448
crashtracking [candidate] (1.468 ms) : 0, 1468
BytebuddyAgent [baseline] (732.754 ms) : 0, 732754
BytebuddyAgent [candidate] (731.735 ms) : 0, 731735
GlobalTracer [baseline] (239.584 ms) : 0, 239584
GlobalTracer [candidate] (238.644 ms) : 0, 238644
IAST [baseline] (25.351 ms) : 0, 25351
IAST [candidate] (25.071 ms) : 0, 25071
AppSec [baseline] (176.23 ms) : 0, 176230
AppSec [candidate] (175.005 ms) : 0, 175005
Debugger [baseline] (6.122 ms) : 0, 6122
Debugger [candidate] (6.017 ms) : 0, 6017
Remote Config [baseline] (671.081 µs) : 0, 671
Remote Config [candidate] (649.813 µs) : 0, 650
Telemetry [baseline] (8.765 ms) : 0, 8765
Telemetry [candidate] (8.573 ms) : 0, 8573
Flare Poller [baseline] (4.102 ms) : 0, 4102
Flare Poller [candidate] (4.009 ms) : 0, 4009
section iast
crashtracking [baseline] (1.455 ms) : 0, 1455
crashtracking [candidate] (1.46 ms) : 0, 1460
BytebuddyAgent [baseline] (830.424 ms) : 0, 830424
BytebuddyAgent [candidate] (830.229 ms) : 0, 830229
GlobalTracer [baseline] (235.282 ms) : 0, 235282
GlobalTracer [candidate] (234.899 ms) : 0, 234899
IAST [baseline] (34.372 ms) : 0, 34372
IAST [candidate] (35.314 ms) : 0, 35314
AppSec [baseline] (27.279 ms) : 0, 27279
AppSec [candidate] (26.088 ms) : 0, 26088
Debugger [baseline] (6.062 ms) : 0, 6062
Debugger [candidate] (6.015 ms) : 0, 6015
Remote Config [baseline] (608.261 µs) : 0, 608
Remote Config [candidate] (596.39 µs) : 0, 596
Telemetry [baseline] (8.504 ms) : 0, 8504
Telemetry [candidate] (8.296 ms) : 0, 8296
Flare Poller [baseline] (4.17 ms) : 0, 4170
Flare Poller [candidate] (4.17 ms) : 0, 4170
section profiling
crashtracking [baseline] (1.448 ms) : 0, 1448
crashtracking [candidate] (1.44 ms) : 0, 1440
BytebuddyAgent [baseline] (730.861 ms) : 0, 730861
BytebuddyAgent [candidate] (733.585 ms) : 0, 733585
GlobalTracer [baseline] (222.491 ms) : 0, 222491
GlobalTracer [candidate] (222.873 ms) : 0, 222873
AppSec [baseline] (32.233 ms) : 0, 32233
AppSec [candidate] (32.471 ms) : 0, 32471
Debugger [baseline] (6.782 ms) : 0, 6782
Debugger [candidate] (8.253 ms) : 0, 8253
Remote Config [baseline] (688.979 µs) : 0, 689
Remote Config [candidate] (695.619 µs) : 0, 696
Telemetry [baseline] (16.298 ms) : 0, 16298
Telemetry [candidate] (14.511 ms) : 0, 14511
Flare Poller [baseline] (4.132 ms) : 0, 4132
Flare Poller [candidate] (4.156 ms) : 0, 4156
ProfilingAgent [baseline] (111.306 ms) : 0, 111306
ProfilingAgent [candidate] (110.383 ms) : 0, 110383
Profiling [baseline] (111.974 ms) : 0, 111974
Profiling [candidate] (111.05 ms) : 0, 111050
LoadParameters
See matching parameters
SummaryFound 3 performance improvements and 1 performance regressions! Performance is the same for 14 metrics, 18 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.55.0-SNAPSHOT~640bc686bc, baseline=1.56.0-SNAPSHOT~1397f25f48
dateFormat X
axisFormat %s
section baseline
no_agent (17.568 ms) : 17387, 17749
. : milestone, 17568,
appsec (19.734 ms) : 19533, 19935
. : milestone, 19734,
code_origins (17.712 ms) : 17537, 17886
. : milestone, 17712,
iast (18.666 ms) : 18473, 18858
. : milestone, 18666,
profiling (19.557 ms) : 19357, 19757
. : milestone, 19557,
tracing (18.762 ms) : 18572, 18952
. : milestone, 18762,
section candidate
no_agent (18.358 ms) : 18171, 18544
. : milestone, 18358,
appsec (18.782 ms) : 18589, 18975
. : milestone, 18782,
code_origins (18.864 ms) : 18675, 19053
. : milestone, 18864,
iast (17.978 ms) : 17798, 18158
. : milestone, 17978,
profiling (18.522 ms) : 18335, 18708
. : milestone, 18522,
tracing (17.876 ms) : 17699, 18053
. : milestone, 17876,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.55.0-SNAPSHOT~640bc686bc, baseline=1.56.0-SNAPSHOT~1397f25f48
dateFormat X
axisFormat %s
section baseline
no_agent (1.185 ms) : 1173, 1196
. : milestone, 1185,
iast (3.103 ms) : 3059, 3146
. : milestone, 3103,
iast_FULL (5.654 ms) : 5597, 5711
. : milestone, 5654,
iast_GLOBAL (3.603 ms) : 3550, 3657
. : milestone, 3603,
profiling (1.952 ms) : 1936, 1968
. : milestone, 1952,
tracing (1.767 ms) : 1753, 1781
. : milestone, 1767,
section candidate
no_agent (1.195 ms) : 1184, 1207
. : milestone, 1195,
iast (3.189 ms) : 3149, 3229
. : milestone, 3189,
iast_FULL (5.705 ms) : 5648, 5762
. : milestone, 5705,
iast_GLOBAL (3.543 ms) : 3499, 3588
. : milestone, 3543,
profiling (2.052 ms) : 2034, 2071
. : milestone, 2052,
tracing (1.765 ms) : 1751, 1780
. : milestone, 1765,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.55.0-SNAPSHOT~640bc686bc, baseline=1.56.0-SNAPSHOT~1397f25f48
dateFormat X
axisFormat %s
section baseline
no_agent (1.469 ms) : 1457, 1480
. : milestone, 1469,
appsec (3.633 ms) : 3419, 3846
. : milestone, 3633,
iast (2.197 ms) : 2133, 2261
. : milestone, 2197,
iast_GLOBAL (2.244 ms) : 2180, 2308
. : milestone, 2244,
profiling (2.054 ms) : 2002, 2105
. : milestone, 2054,
tracing (2.024 ms) : 1974, 2074
. : milestone, 2024,
section candidate
no_agent (1.471 ms) : 1460, 1483
. : milestone, 1471,
appsec (3.696 ms) : 3479, 3912
. : milestone, 3696,
iast (2.194 ms) : 2131, 2258
. : milestone, 2194,
iast_GLOBAL (2.243 ms) : 2179, 2306
. : milestone, 2243,
profiling (2.048 ms) : 1997, 2100
. : milestone, 2048,
tracing (2.013 ms) : 1964, 2063
. : milestone, 2013,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.55.0-SNAPSHOT~640bc686bc, baseline=1.56.0-SNAPSHOT~1397f25f48
dateFormat X
axisFormat %s
section baseline
no_agent (15.546 s) : 15546000, 15546000
. : milestone, 15546000,
appsec (14.876 s) : 14876000, 14876000
. : milestone, 14876000,
iast (18.414 s) : 18414000, 18414000
. : milestone, 18414000,
iast_GLOBAL (17.842 s) : 17842000, 17842000
. : milestone, 17842000,
profiling (15.107 s) : 15107000, 15107000
. : milestone, 15107000,
tracing (14.769 s) : 14769000, 14769000
. : milestone, 14769000,
section candidate
no_agent (15.631 s) : 15631000, 15631000
. : milestone, 15631000,
appsec (14.645 s) : 14645000, 14645000
. : milestone, 14645000,
iast (18.673 s) : 18673000, 18673000
. : milestone, 18673000,
iast_GLOBAL (17.998 s) : 17998000, 17998000
. : milestone, 17998000,
profiling (15.156 s) : 15156000, 15156000
. : milestone, 15156000,
tracing (14.788 s) : 14788000, 14788000
. : milestone, 14788000,
|
Contributor
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
mhlidd
reviewed
Oct 30, 2025
Contributor
mhlidd
left a comment
mhlidd
left a comment
There was a problem hiding this comment.
General Question: If the config is set to a non-JSON-path value, or not "all", should the feature be turned off in the Config class?
ojproductions
added
type: enhancement
Contributor
Author
|
@mhlidd, if we are to take the official documentation literally, the only time payload tagging is disabled is when the variables are empty or omitted. I do see how this could be misleading though because you can effectively turn on payload tagging by adding any value even if it is not "all". How I see it though, the paths are subtractive rather than additive. Meaning, default is to show every field, and for every valid path added, we show 1 less field. If the path is invalid its ignored |
Contributor
Just want to make sure that this is aligned with the spec. From the spec, I see the following:
This seems a little contradictory to the behavior we see in the tracer. While the redaction behavior would not be turned on unless the JSON paths passed in are valid, the configs are still when adding AWS span tags at some point. ❓ Does the RFC spec mean that we want the span tag behavior to also be disabled when non-valid JSON paths are set for the environment variables. If so, we may need to modify the logic of verifying the logic of handling the Environment Variable values. |
Contributor
Author
|
I think this comes down to which source of truth we want to honor – the spec or the public documentation on datadog website. I believe @joeyzhao2018 (author of the spec) also decided to follow the public doc (but correct me if I'm wrong Joey. Either way we should update the spec accordingly) |
Contributor
|
@ojproductions To clarify. The public documentation specified clearly on three types of values, i.e. JSON PATH, "all" and empty( or double quotes ""). It did NOT clearly state the expected behavior of any other values such as "false" "off". IMHO, treating those values as ON and tell the customers that our doc only mentioned "" as the official accepted value is not ideal. Therefore I still think it should be off. |
ojproductions
changed the title
mhlidd
reviewed
Nov 10, 2025
Contributor
mhlidd
left a comment
mhlidd
left a comment
There was a problem hiding this comment.
Overall migration/feature looks good! Just some questions/suggestions with the tests
dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/PayloadTagsProcessor.java
Show resolved
Hide resolved
internal-api/src/main/java/datadog/trace/util/json/JsonPathParser.java
Outdated
Show resolved
Hide resolved
internal-api/src/main/java/datadog/trace/util/json/JsonPathParser.java
Outdated
Show resolved
Hide resolved
Comment on lines
940
to
941
| config.@cloudRequestPayloadTagging == null | ||
| config.@cloudResponsePayloadTagging == [] |
Contributor
There was a problem hiding this comment.
I think we should be okay just checking the values from the getter here instead of also checking the field values themselves. This is because the only way to fetch the values outside of Config is through the getters.
Contributor
Author
There was a problem hiding this comment.
I did this to ensure the payload tagging was disabled when null, but itd probably be better to test that via the isCloudPayloadTaggingEnabled methods
| return JsonPath.Builder.start() | ||
| } | ||
|
|
||
| def "set cloud payload tagging config"() { |
Contributor
There was a problem hiding this comment.
Similarly as above, I would assert on the values from the getters instead of the field values since other classes would only receive that value.
To verify the difference between "all" vs "invalidPath", I would assert upon the isCloud*PayloadTaggingEnabled functions.
| if(config.@cloudRequestPayloadTagging == null){ | ||
| expectedreqconfig == null | ||
| }else{ | ||
| config.@cloudRequestPayloadTagging.toString()== expectedreqconfig.toString() |
Contributor
There was a problem hiding this comment.
❓ Haven't tested, but do you need to compare toString versions here? If you need to, can you hard-code the expected value instead of using JsonPath.Builder()?
Contributor
Author
There was a problem hiding this comment.
yes, I can probably just make the expected values strings, to avoid using the builder. unless im interpreting your suggestion wrong? lmk
Comment on lines
2924
to
2928
| if(config.@cloudResponsePayloadTagging == null){ | ||
| expectedrespconfig == null | ||
| }else{ | ||
| config.@cloudResponsePayloadTagging.toString()== expectedrespconfig.toString() | ||
| } |
internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy
Outdated
Show resolved
Hide resolved
internal-api/src/test/groovy/datadog/trace/util/json/JsonPathParserSpec.groovy
Outdated
Show resolved
Hide resolved
internal-api/src/test/groovy/datadog/trace/util/json/PathCursorSpec.groovy
Show resolved
Hide resolved
mhlidd
approved these changes
Nov 11, 2025
Contributor
mhlidd
left a comment
mhlidd
left a comment
There was a problem hiding this comment.
Great work! Thanks for the fixes!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What Does This Do
Motivation
Specifically in the lambda environment, users (or some instrumenation helpers such as datadog-cdk-constructs) may add the env without adding any value so that it defaults to disabled.
This is to make the feature visible for discovery purposes, but without forcing it as a default
read more here
APMSVLS-53