feat(tracer): implement AWS payload tagging for request/response#10642
Merged
feat(tracer): implement AWS payload tagging for request/response#10642
Conversation
Contributor
|
|
BenchmarksBenchmark execution time: 2024-11-12 20:45:03 Comparing candidate commit c6c5551 in PR branch Found 0 performance improvements and 16 performance regressions! Performance is the same for 372 metrics, 2 unstable metrics. scenario:sethttpmeta-all-disabled
scenario:sethttpmeta-all-enabled
scenario:sethttpmeta-collectipvariant_exists
scenario:sethttpmeta-no-collectipvariant
scenario:sethttpmeta-no-useragentvariant
scenario:sethttpmeta-obfuscation-no-query
scenario:sethttpmeta-obfuscation-regular-case-explicit-query
scenario:sethttpmeta-obfuscation-regular-case-implicit-query
scenario:sethttpmeta-obfuscation-send-querystring-disabled
scenario:sethttpmeta-obfuscation-worst-case-explicit-query
scenario:sethttpmeta-obfuscation-worst-case-implicit-query
scenario:sethttpmeta-useragentvariant_exists_1
scenario:sethttpmeta-useragentvariant_exists_2
scenario:sethttpmeta-useragentvariant_exists_3
scenario:sethttpmeta-useragentvariant_not_exists_1
scenario:sethttpmeta-useragentvariant_not_exists_2
|
5 tasks
8bdb59a to
eea1069
Compare
bouwkast
commented
Oct 30, 2024
Mapping from Java implementation
ZStriker19
approved these changes
Nov 8, 2024
Seems that environment leaks to other tests
P403n1x87
reviewed
Nov 8, 2024
…so they don't get flagged as misspelled during doc build
P403n1x87
reviewed
Nov 11, 2024
P403n1x87
reviewed
Nov 11, 2024
P403n1x87
reviewed
Nov 11, 2024
Contributor
|
I'm slightly concerned by the new vendored libraries and by the potential performance of this solution. I'd love to see some benchmarking/profiling to get an idea of what the CPU overhead looks like, if possible 🙏 |
quinna-h
reviewed
Nov 11, 2024
2 tasks
juanjux
approved these changes
Nov 12, 2024
gnufede
approved these changes
Nov 12, 2024
quinna-h
pushed a commit
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]>
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
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.
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 and this implementation in dd-trace-java.
This feature is disabled by default.
When activated this will produce span tags such as:
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 defaultJSONPaths for redaction logic.JSONPaths to define that AWS request payload expansion is enabled for requests using the defaultJSONPaths and the user-suppliedJSONPaths 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 defaultJSONPaths for redaction logic.JSONPaths to define that AWS request payload expansion is enabled for responses using the defaultJSONPaths and the user-suppliedJSONPaths for redaction logic.DD_TRACE_CLOUD_PAYLOAD_TAGGING_MAX_DEPTH(not defined in RFC but done to match NodeJS):10DD_TRACE_CLOUD_PAYLOAD_TAGGING_MAX_TAGS(to match Java implementation)758DD_TRACE_CLOUD_PAYLOAD_TAGGING_SERVICES(to match Java implementation)s3,sns,sqs,kinesis,eventbridgeOther
jsonpath-nghas been vendoredplyhas been vendored (v3.11) (dependency ofjsonpath-ng)Checklist
Reviewer Checklist