AWS Payload Tagging (deprecated) #7312
Closed
Conversation
This file contains 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
ygree
added
tag: do not merge
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 54 metrics, 9 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.41.0-SNAPSHOT~3105869721, baseline=1.41.0-SNAPSHOT~a1c2f48c91
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.066 s) : 0, 1066371
Total [baseline] (10.33 s) : 0, 10329665
Agent [candidate] (1.069 s) : 0, 1069332
Total [candidate] (10.564 s) : 0, 10563938
section appsec
Agent [baseline] (1.205 s) : 0, 1205481
Total [baseline] (10.546 s) : 0, 10545728
Agent [candidate] (1.205 s) : 0, 1204786
Total [candidate] (10.571 s) : 0, 10570503
section iast
Agent [baseline] (1.201 s) : 0, 1200562
Total [baseline] (10.911 s) : 0, 10911481
Agent [candidate] (1.194 s) : 0, 1194413
Total [candidate] (10.851 s) : 0, 10850800
section profiling
Agent [baseline] (1.262 s) : 0, 1262360
Total [baseline] (10.61 s) : 0, 10609808
Agent [candidate] (1.274 s) : 0, 1273667
Total [candidate] (10.67 s) : 0, 10670252
gantt
title petclinic - break down per module: candidate=1.41.0-SNAPSHOT~3105869721, baseline=1.41.0-SNAPSHOT~a1c2f48c91
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (681.203 ms) : 0, 681203
BytebuddyAgent [candidate] (681.096 ms) : 0, 681096
GlobalTracer [baseline] (309.794 ms) : 0, 309794
GlobalTracer [candidate] (312.741 ms) : 0, 312741
AppSec [baseline] (53.62 ms) : 0, 53620
AppSec [candidate] (53.758 ms) : 0, 53758
Remote Config [baseline] (662.244 µs) : 0, 662
Remote Config [candidate] (655.605 µs) : 0, 656
Telemetry [baseline] (7.514 ms) : 0, 7514
Telemetry [candidate] (7.461 ms) : 0, 7461
section appsec
BytebuddyAgent [baseline] (701.46 ms) : 0, 701460
BytebuddyAgent [candidate] (699.204 ms) : 0, 699204
GlobalTracer [baseline] (308.182 ms) : 0, 308182
GlobalTracer [candidate] (310.587 ms) : 0, 310587
AppSec [baseline] (162.911 ms) : 0, 162911
AppSec [candidate] (162.272 ms) : 0, 162272
Remote Config [baseline] (633.12 µs) : 0, 633
Remote Config [candidate] (629.622 µs) : 0, 630
Telemetry [baseline] (8.859 ms) : 0, 8859
Telemetry [candidate] (8.431 ms) : 0, 8431
IAST [baseline] (19.474 ms) : 0, 19474
IAST [candidate] (20.052 ms) : 0, 20052
section iast
BytebuddyAgent [baseline] (800.52 ms) : 0, 800520
BytebuddyAgent [candidate] (794.302 ms) : 0, 794302
GlobalTracer [baseline] (299.768 ms) : 0, 299768
GlobalTracer [candidate] (300.897 ms) : 0, 300897
AppSec [baseline] (52.763 ms) : 0, 52763
AppSec [candidate] (55.341 ms) : 0, 55341
Remote Config [baseline] (598.424 µs) : 0, 598
Remote Config [candidate] (595.011 µs) : 0, 595
Telemetry [baseline] (7.037 ms) : 0, 7037
Telemetry [candidate] (6.96 ms) : 0, 6960
IAST [baseline] (26.136 ms) : 0, 26136
IAST [candidate] (22.73 ms) : 0, 22730
section profiling
BytebuddyAgent [baseline] (674.082 ms) : 0, 674082
BytebuddyAgent [candidate] (678.972 ms) : 0, 678972
GlobalTracer [baseline] (391.654 ms) : 0, 391654
GlobalTracer [candidate] (397.505 ms) : 0, 397505
AppSec [baseline] (54.514 ms) : 0, 54514
AppSec [candidate] (54.232 ms) : 0, 54232
Remote Config [baseline] (647.907 µs) : 0, 648
Remote Config [candidate] (645.932 µs) : 0, 646
Telemetry [baseline] (7.325 ms) : 0, 7325
Telemetry [candidate] (7.424 ms) : 0, 7424
ProfilingAgent [baseline] (95.653 ms) : 0, 95653
ProfilingAgent [candidate] (95.923 ms) : 0, 95923
Profiling [baseline] (95.677 ms) : 0, 95677
Profiling [candidate] (95.947 ms) : 0, 95947
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.41.0-SNAPSHOT~3105869721, baseline=1.41.0-SNAPSHOT~a1c2f48c91
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.073 s) : 0, 1073004
Total [baseline] (8.534 s) : 0, 8533973
Agent [candidate] (1.076 s) : 0, 1076231
Total [candidate] (8.545 s) : 0, 8545466
section iast
Agent [baseline] (1.198 s) : 0, 1197941
Total [baseline] (9.066 s) : 0, 9066036
Agent [candidate] (1.213 s) : 0, 1213365
Total [candidate] (9.105 s) : 0, 9105342
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.198 s) : 0, 1198284
Total [baseline] (9.074 s) : 0, 9074436
Agent [candidate] (1.198 s) : 0, 1197690
Total [candidate] (9.111 s) : 0, 9110795
section iast_TELEMETRY_OFF
Agent [baseline] (1.19 s) : 0, 1189842
Total [baseline] (9.055 s) : 0, 9055436
Agent [candidate] (1.193 s) : 0, 1193456
Total [candidate] (9.057 s) : 0, 9056868
gantt
title insecure-bank - break down per module: candidate=1.41.0-SNAPSHOT~3105869721, baseline=1.41.0-SNAPSHOT~a1c2f48c91
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (686.114 ms) : 0, 686114
BytebuddyAgent [candidate] (685.711 ms) : 0, 685711
GlobalTracer [baseline] (311.232 ms) : 0, 311232
GlobalTracer [candidate] (314.574 ms) : 0, 314574
AppSec [baseline] (53.779 ms) : 0, 53779
AppSec [candidate] (53.978 ms) : 0, 53978
Remote Config [baseline] (664.167 µs) : 0, 664
Remote Config [candidate] (668.475 µs) : 0, 668
Telemetry [baseline] (7.467 ms) : 0, 7467
Telemetry [candidate] (7.531 ms) : 0, 7531
section iast
BytebuddyAgent [baseline] (800.183 ms) : 0, 800183
BytebuddyAgent [candidate] (807.113 ms) : 0, 807113
GlobalTracer [baseline] (298.089 ms) : 0, 298089
GlobalTracer [candidate] (305.279 ms) : 0, 305279
AppSec [baseline] (57.179 ms) : 0, 57179
AppSec [candidate] (56.364 ms) : 0, 56364
Remote Config [baseline] (595.413 µs) : 0, 595
Remote Config [candidate] (600.938 µs) : 0, 601
Telemetry [baseline] (7.083 ms) : 0, 7083
Telemetry [candidate] (7.123 ms) : 0, 7123
IAST [baseline] (21.112 ms) : 0, 21112
IAST [candidate] (23.042 ms) : 0, 23042
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (798.15 ms) : 0, 798150
BytebuddyAgent [candidate] (796.025 ms) : 0, 796025
GlobalTracer [baseline] (299.66 ms) : 0, 299660
GlobalTracer [candidate] (302.075 ms) : 0, 302075
AppSec [baseline] (55.861 ms) : 0, 55861
AppSec [candidate] (56.146 ms) : 0, 56146
Remote Config [baseline] (623.258 µs) : 0, 623
Remote Config [candidate] (613.003 µs) : 0, 613
Telemetry [baseline] (7.209 ms) : 0, 7209
Telemetry [candidate] (7.087 ms) : 0, 7087
IAST [baseline] (23.07 ms) : 0, 23070
IAST [candidate] (22.069 ms) : 0, 22069
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (792.561 ms) : 0, 792561
BytebuddyAgent [candidate] (792.879 ms) : 0, 792879
GlobalTracer [baseline] (298.374 ms) : 0, 298374
GlobalTracer [candidate] (301.744 ms) : 0, 301744
AppSec [baseline] (53.091 ms) : 0, 53091
AppSec [candidate] (54.587 ms) : 0, 54587
Remote Config [baseline] (616.686 µs) : 0, 617
Remote Config [candidate] (604.938 µs) : 0, 605
Telemetry [baseline] (6.832 ms) : 0, 6832
Telemetry [candidate] (6.946 ms) : 0, 6946
IAST [baseline] (24.717 ms) : 0, 24717
IAST [candidate] (23.054 ms) : 0, 23054
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 17 unstable metrics. Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.41.0-SNAPSHOT~3105869721, baseline=1.41.0-SNAPSHOT~a1c2f48c91
dateFormat X
axisFormat %s
section baseline
no_agent (378.624 µs) : 359, 399
. : milestone, 379,
iast (485.683 µs) : 464, 507
. : milestone, 486,
iast_FULL (559.462 µs) : 538, 581
. : milestone, 559,
iast_GLOBAL (514.273 µs) : 492, 536
. : milestone, 514,
iast_HARDCODED_SECRET_DISABLED (492.157 µs) : 471, 514
. : milestone, 492,
iast_INACTIVE (454.69 µs) : 433, 476
. : milestone, 455,
iast_TELEMETRY_OFF (471.992 µs) : 451, 493
. : milestone, 472,
tracing (451.796 µs) : 431, 473
. : milestone, 452,
section candidate
no_agent (373.415 µs) : 354, 393
. : milestone, 373,
iast (487.067 µs) : 465, 509
. : milestone, 487,
iast_FULL (554.443 µs) : 533, 576
. : milestone, 554,
iast_GLOBAL (502.628 µs) : 481, 524
. : milestone, 503,
iast_HARDCODED_SECRET_DISABLED (488.318 µs) : 466, 510
. : milestone, 488,
iast_INACTIVE (451.749 µs) : 431, 473
. : milestone, 452,
iast_TELEMETRY_OFF (474.415 µs) : 453, 496
. : milestone, 474,
tracing (447.354 µs) : 426, 468
. : milestone, 447,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.41.0-SNAPSHOT~3105869721, baseline=1.41.0-SNAPSHOT~a1c2f48c91
dateFormat X
axisFormat %s
section baseline
no_agent (1.34 ms) : 1321, 1359
. : milestone, 1340,
appsec (1.721 ms) : 1697, 1744
. : milestone, 1721,
appsec_no_iast (1.728 ms) : 1705, 1751
. : milestone, 1728,
iast (1.489 ms) : 1466, 1512
. : milestone, 1489,
profiling (1.51 ms) : 1485, 1536
. : milestone, 1510,
tracing (1.469 ms) : 1445, 1493
. : milestone, 1469,
section candidate
no_agent (1.349 ms) : 1329, 1369
. : milestone, 1349,
appsec (1.721 ms) : 1697, 1745
. : milestone, 1721,
appsec_no_iast (1.711 ms) : 1686, 1735
. : milestone, 1711,
iast (1.485 ms) : 1462, 1507
. : milestone, 1485,
profiling (1.485 ms) : 1460, 1509
. : milestone, 1485,
tracing (1.477 ms) : 1453, 1501
. : milestone, 1477,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.41.0-SNAPSHOT~3105869721, baseline=1.41.0-SNAPSHOT~a1c2f48c91
dateFormat X
axisFormat %s
section baseline
no_agent (15.246 s) : 15246000, 15246000
. : milestone, 15246000,
appsec (15.15 s) : 15150000, 15150000
. : milestone, 15150000,
iast (19.25 s) : 19250000, 19250000
. : milestone, 19250000,
iast_GLOBAL (18.097 s) : 18097000, 18097000
. : milestone, 18097000,
profiling (15.749 s) : 15749000, 15749000
. : milestone, 15749000,
tracing (15.233 s) : 15233000, 15233000
. : milestone, 15233000,
section candidate
no_agent (15.744 s) : 15744000, 15744000
. : milestone, 15744000,
appsec (15.016 s) : 15016000, 15016000
. : milestone, 15016000,
iast (18.741 s) : 18741000, 18741000
. : milestone, 18741000,
iast_GLOBAL (18.172 s) : 18172000, 18172000
. : milestone, 18172000,
profiling (15.711 s) : 15711000, 15711000
. : milestone, 15711000,
tracing (15.151 s) : 15151000, 15151000
. : milestone, 15151000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.41.0-SNAPSHOT~3105869721, baseline=1.41.0-SNAPSHOT~a1c2f48c91
dateFormat X
axisFormat %s
section baseline
no_agent (1.469 ms) : 1458, 1481
. : milestone, 1469,
appsec (2.338 ms) : 2297, 2379
. : milestone, 2338,
iast (2.079 ms) : 2027, 2131
. : milestone, 2079,
iast_GLOBAL (2.13 ms) : 2077, 2182
. : milestone, 2130,
profiling (1.943 ms) : 1902, 1984
. : milestone, 1943,
tracing (1.919 ms) : 1880, 1958
. : milestone, 1919,
section candidate
no_agent (1.466 ms) : 1455, 1478
. : milestone, 1466,
appsec (2.33 ms) : 2289, 2370
. : milestone, 2330,
iast (2.07 ms) : 2019, 2121
. : milestone, 2070,
iast_GLOBAL (2.108 ms) : 2056, 2159
. : milestone, 2108,
profiling (1.942 ms) : 1901, 1983
. : milestone, 1942,
tracing (1.919 ms) : 1879, 1958
. : milestone, 1919,
|
ygree
force-pushed
the
ygree/aws-payload-tagging
branch
from
c9a799d to
1e180f3
Compare
vandonr
reviewed
Aug 21, 2024
vandonr
reviewed
Aug 22, 2024
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
...va-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/ResponseBodyStreamWrapper.java
Outdated
Show resolved
Hide resolved
...va-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/ResponseBodyStreamWrapper.java
Outdated
Show resolved
Hide resolved
ygree
force-pushed
the
ygree/aws-payload-tagging
branch
6 times, most recently
from
83f1bef to
7252b43
Compare
dd-trace-core/src/main/java/datadog/trace/payloadtags/JsonToTags.java
Outdated
Show resolved
Hide resolved
ygree
force-pushed
the
ygree/aws-payload-tagging
branch
2 times, most recently
from
37eed2b to
bdb7eaf
Compare
dd-trace-core/src/main/java/datadog/trace/payloadtags/json/JsonPath.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/payloadtags/json/JsonPathParser.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/payloadtags/json/JsonPath.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/payloadtags/json/JsonPath.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/payloadtags/JsonTagsCollector.java
Outdated
Show resolved
Hide resolved
…slf4j-api dependency in the agent
…PAYLOAD_TAGGING config settings
…ath to jsurfer-core
…nsumption. Add support for reading array of bytes.
ygree
force-pushed
the
ygree/aws-payload-tagging
branch
from
22921eb to
15a3056
Compare
amarziali
reviewed
Oct 3, 2024
...-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java
Outdated
Show resolved
Hide resolved
| ResponseBodyStreamWrapper wrapper = (ResponseBodyStreamWrapper) body; | ||
| ByteArrayInputStream bodyStream = wrapper.asByteArrayInputStream(); | ||
| if (bodyStream != null) { | ||
| AgentTracer.get().addTagsFromResponseBody(span, bodyStream, "aws.response.body"); |
There was a problem hiding this comment.
I've some concerns regarding rebuffering the response and parse part of it during the advice. Apart from holding additional memory (and risk to corrupt it) and adding processing to parse the json we actually assume that the encoding would be plain json while aws sdk is supposed to support other transport encoding like cbor and xml. This will not work in those cases.
The sdk v2 has a POJO structure in the response that offers an object traversal facility that's independent from its representation. In particular each SdkResponse is an instance of SdkPojo who as a method sdkFields returning a list of SdkFields holding a reference to the location and its value. Those objects are already available when intercepting afterExecution so we can just use them without adding extra efforts.
To debug I put this method on the interceptor:
void printSdkFields(String root, SdkPojo pojo) {
for (SdkField<?> f : pojo.sdkFields()) {
Object value = f.getValueOrDefault(pojo);
String key = root + "." + f.locationName();
if (value instanceof Collection) {
int i = 0;
for (Object v : (Collection<?>) value) {
if (v instanceof SdkPojo) {
printSdkFields(key + "[" + i + "]", (SdkPojo) v);
} else {
System.err.println(key + "[" + i + "]: " + v);
}
i++;
}
} else if (value instanceof SdkPojo) {
printSdkFields(key, (SdkPojo) value);
} else {
System.err.println(key + ": " + value);
}
}
}
I call this from afterExecution like this printSdkFields("$", context.response()); and the output I have is quite good
Example for kinesis:
$.Records[0].SequenceNumber: 21269319989652663814458848515492872193
$.Records[0].ApproximateArrivalTimestamp: +56726-05-21T03:34:19Z
$.Records[0].Data: SdkBytes(bytes=0x5f3c646174613e5f30)
$.Records[0].PartitionKey: partitionKey
$.Records[0].EncryptionType: null
$.Records[1].SequenceNumber: 21269319989652663814458848515492872193
$.Records[1].ApproximateArrivalTimestamp: +56726-05-21T03:50:59Z
$.Records[1].Data: SdkBytes(bytes=0x5f3c646174613e5f30)
$.Records[1].PartitionKey: partitionKey
$.Records[1].EncryptionType: null
$.NextShardIterator: AAA
$.MillisBehindLatest: 2100
I think this approach can save some time and also support other body encoding other than json
There was a problem hiding this comment.
According to the RFC, the idea was to generalize to any service (not just AWS) and be able to convert request and response body to tags for observability purposes.
I totally agree with your concerns about overhead, so I tried to limit parsing as much as possible by
- Avoiding materializing JSON and instead using event-based parsing to traverse in one go.
- Buffering only payloads that start with '{'.
- Having an upper limit of 256Kb for the buffer.
- Disabled by default.
- Limit the number of services it's enabled for (now only SQS, ApiGateway and EventBridge).
Also, so far we are going to release it internally for the Cloud Observability team and there is no intention to release it to GA at all.
Regarding your comment about services not using JSON, this is exactly what came up in testing I did recently. The format used depends not only on the service, but even on the minor version of the SDK. For example, SNS uses Http Form Params and XML, some earlier versions of the SDK for SQS also use XML instead of JSON. And as you mentioned, Kinesis uses CBOR.
I appreciate your efforts to experiment with an alternative approach and provide a very detailed idea of how to implement it. I think we should have a specialized version for AWS SDK to use the provided POJOs as you suggested. That way we can optimize drastically for AWS SDK. And at the same time expand the visibility to AWS services that don't use JSON.
Thank you!
amarziali
approved these changes
Oct 16, 2024
There was a problem hiding this comment.
There are improvements we can do specifically for AWS that would not require any json parsing that we should consider. Also, the json parsing can be do lazily. Since the feature is experimental and not enabled by default, I would not block from approval. However we should consider to implement such improvements.
| if (config.isCloudRequestPayloadTaggingEnabled() | ||
| && config.isCloudPayloadTaggingEnabledFor(awsServiceName)) { | ||
| InputStream body = requestBody.get().contentStreamProvider().newStream(); | ||
| AgentTracer.get().addTagsFromRequestBody(span, body, "aws.request.body"); |
There was a problem hiding this comment.
Theoretically we could have done it lazily in the agent thread (as tag postprocessor) so it would not affect the same thread of the aws request (i.e. only attach the buffer to the span and process on a TagPostProcessor). It would also have avoided the fact to expose those methods on the agent tracer
There was a problem hiding this comment.
Yeah, you're right. I just wasn't sure how much we wanted to optimize it at all. Now I think we should go that extra mile to make it work without keeping buffers but using SdkPojos with all necessary parsing (still needed for inner strings/bytes that contain JSON) done outside of the main thread.
There was a problem hiding this comment.
Started rework in a separate PR #7811
manuel-alvarez-alvarez
requested review from
manuel-alvarez-alvarez
and removed request for
manuel-alvarez-alvarez
2 tasks
|
Closing this in favor #7811 |
bouwkast
added a commit
to DataDog/dd-trace-py
that referenced
this pull request
Nov 12, 2024
) ## Overview This pull request adds the ability to expand AWS request/response payloads as span tags. This matches our lambda offerings and provides useful information to developers when debugging communication between various AWS services. This is based on the AWS Payload Tagging RFC and this implementation in [dd-trace-node](DataDog/dd-trace-js#4309) and this implementation in [dd-trace-java](DataDog/dd-trace-java#7312). This feature is _disabled_ by default. When activated this will produce span tags such as: ``` "aws.request.body.PublishBatchRequestEntries.0.Id": "1", "aws.request.body.PublishBatchRequestEntries.0.Message": "ironmaiden", "aws.request.body.PublishBatchRequestEntries.1.Id": "2", "aws.request.body.PublishBatchRequestEntries.1.Message": "megadeth" "aws.response.body.HTTPStatusCode": "200", ``` ## Configuration There are five new configuration options: - `DD_TRACE_CLOUD_REQUEST_PAYLOAD_TAGGING`: - `""` by default to indicate that AWS request payload expansion is **disabled** for _requests_. - `"all"` to define that AWS request payload expansion is **enabled** for _requests_ using the default `JSONPath`s for redaction logic. - a comma-separated list of user-supplied `JSONPath`s to define that AWS request payload expansion is **enabled** for _requests_ using the default `JSONPath`s and the user-supplied `JSONPath`s for redaction logic. - `DD_TRACE_CLOUD_RESPONSE_PAYLOAD_TAGGING`: - `""` by default to indicate that AWS response payload expansion is **disabled** for _responses_. - `"all"` to define that AWS response payload expansion is **enabled** for _responses_ using the default `JSONPath`s for redaction logic. - a comma-separated list of user-supplied `JSONPath`s to define that AWS request payload expansion is **enabled** for _responses_ using the default `JSONPath`s and the user-supplied `JSONPath`s for redaction logic. - `DD_TRACE_CLOUD_PAYLOAD_TAGGING_MAX_DEPTH` (not defined in RFC but done to match NodeJS): - sets the depth after which we stop creating tags from a payload - defaults to a value of `10` - `DD_TRACE_CLOUD_PAYLOAD_TAGGING_MAX_TAGS` (to match Java implementation) - sets the maximum number of tags allowed to be expanded - defaults to a value of `758` - `DD_TRACE_CLOUD_PAYLOAD_TAGGING_SERVICES` (to match Java implementation) - a comma-separated list of supported AWS services - defaults to ` s3,sns,sqs,kinesis,eventbridge` ## Other - [`jsonpath-ng` has been vendored](https://github.com/h2non/jsonpath-ng/blob/master/jsonpath_ng/jsonpath.py) - [`ply` has been vendored (v3.11) (dependency of `jsonpath-ng`)](https://github.com/dabeaz/ply/releases/tag/3.11) ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: erikayasuda <[email protected]>
quinna-h
pushed a commit
to DataDog/dd-trace-py
that referenced
this pull request
Nov 13, 2024
) ## Overview This pull request adds the ability to expand AWS request/response payloads as span tags. This matches our lambda offerings and provides useful information to developers when debugging communication between various AWS services. This is based on the AWS Payload Tagging RFC and this implementation in [dd-trace-node](DataDog/dd-trace-js#4309) and this implementation in [dd-trace-java](DataDog/dd-trace-java#7312). This feature is _disabled_ by default. When activated this will produce span tags such as: ``` "aws.request.body.PublishBatchRequestEntries.0.Id": "1", "aws.request.body.PublishBatchRequestEntries.0.Message": "ironmaiden", "aws.request.body.PublishBatchRequestEntries.1.Id": "2", "aws.request.body.PublishBatchRequestEntries.1.Message": "megadeth" "aws.response.body.HTTPStatusCode": "200", ``` ## Configuration There are five new configuration options: - `DD_TRACE_CLOUD_REQUEST_PAYLOAD_TAGGING`: - `""` by default to indicate that AWS request payload expansion is **disabled** for _requests_. - `"all"` to define that AWS request payload expansion is **enabled** for _requests_ using the default `JSONPath`s for redaction logic. - a comma-separated list of user-supplied `JSONPath`s to define that AWS request payload expansion is **enabled** for _requests_ using the default `JSONPath`s and the user-supplied `JSONPath`s for redaction logic. - `DD_TRACE_CLOUD_RESPONSE_PAYLOAD_TAGGING`: - `""` by default to indicate that AWS response payload expansion is **disabled** for _responses_. - `"all"` to define that AWS response payload expansion is **enabled** for _responses_ using the default `JSONPath`s for redaction logic. - a comma-separated list of user-supplied `JSONPath`s to define that AWS request payload expansion is **enabled** for _responses_ using the default `JSONPath`s and the user-supplied `JSONPath`s for redaction logic. - `DD_TRACE_CLOUD_PAYLOAD_TAGGING_MAX_DEPTH` (not defined in RFC but done to match NodeJS): - sets the depth after which we stop creating tags from a payload - defaults to a value of `10` - `DD_TRACE_CLOUD_PAYLOAD_TAGGING_MAX_TAGS` (to match Java implementation) - sets the maximum number of tags allowed to be expanded - defaults to a value of `758` - `DD_TRACE_CLOUD_PAYLOAD_TAGGING_SERVICES` (to match Java implementation) - a comma-separated list of supported AWS services - defaults to ` s3,sns,sqs,kinesis,eventbridge` ## Other - [`jsonpath-ng` has been vendored](https://github.com/h2non/jsonpath-ng/blob/master/jsonpath_ng/jsonpath.py) - [`ply` has been vendored (v3.11) (dependency of `jsonpath-ng`)](https://github.com/dabeaz/ply/releases/tag/3.11) ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: erikayasuda <[email protected]>
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
Adds functionality to capture AWS Json response/request payload and convert it to span tags while applying expansion and redaction defined rules.
Motivation
Having the ability to see data that was passed into an HTTPS payload from one service to the other.
Help customers (especially those who are using serverless architecture) reproduce and resolve bugs in their serverless compute code or configuration.
Additional Notes
Jira ticket: AIDM-174
NodeJS: DataDog/dd-trace-js#4309
Python: DataDog/dd-trace-py#10642
NOTE: This only works for services that use the JSON protocol, which depends on the service and the AWS SDK version. There is an alternative hybrid approach in this PR that provides better overall visibility and performance.