[serverless] Add S3 Span Pointers by nhulston · Pull Request #4875 · DataDog/dd-trace-js

nhulston · 2024-11-11T18:34:05Z

What does this PR do?

Adds span pointers in S3 for putObject, copyObject, and completeMultipartUpload requests.

Span pointers are similar to Span Links, but for cases when it is impossible to pass the Trace ID and Span ID between the spans that need to be linked.

When the calculated hashes for the upstream and downstream lambdas match, the Datadog backend will automatically link the two traces together.

When clicking on the linked span, a new tab opens linking to the downstream Lambda function that was triggered by this S3 object update.

Motivation

This feature already exists in Python, and I'm working on adding it to all other runtimes (Node, .NET, Java, Golang).

Testing Guidelines

Easy: Checkout this span, enable the feature flag, and you will see that it's pointing to the downstream Lambda.

Run this Lambda function with the event payload

{
  "shouldPutObject": false,
  "shouldCopyObject": false,
  "shouldMultipartUpload": false,
  "useV2": true
}

and change one of the bools to true. Also, try both with AWS SDK v2 and v3. Enable the span pointers feature flag, and check Datadog to ensure that the spans are properly linked.

I also added tests:

# Unit tests
yarn test packages/dd-trace/test/util.spec.js

# Integration tests
docker run -d -p 4566:4566 localstack/localstack
PLUGINS=aws-sdk SERVICES=localstack DD_DATA_STREAMS_ENABLED=true yarn test:plugins:ci

Additional Notes

This handles the 'upstream' case, for Lambdas that make S3 requests. I have another PR in datadog-lambda-js to handle the 'downstream' case, for Lambdas that are triggered by S3 updates. Add S3 Downstream Span Pointers datadog-lambda-js#587
S3 and Dynamo are supported in Python. I will soon open another PR to add support for Dynamo.
packages/dd-trace/src/span_pointers.js is a public API, since datadog-lambda-js will reuse the exported fields.

github-actions · 2024-11-11T18:34:49Z

Overall package size

Self size: 8.02 MB
Deduped: 94.52 MB
No deduping: 94.86 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.2.2 | 29.27 MB | 29.27 MB | | @datadog/native-appsec | 8.3.0 | 19.37 MB | 19.38 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.5.0 | 2.51 MB | 2.65 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.0.1 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | msgpack-lite | 0.1.26 | 201.16 kB | 281.59 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | int64-buffer | 0.1.10 | 49.18 kB | 49.18 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | path-to-regexp | 0.1.10 | 6.38 kB | 6.38 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

pr-commenter · 2024-11-11T18:42:30Z

Benchmarks

Benchmark execution time: 2024-11-18 16:22:37

Comparing candidate commit fe72a0f in PR branch nicholas.hulston/s3-span-pointers with baseline commit 2072a1f in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 260 metrics, 6 unstable metrics.

nhulston · 2024-11-11T20:31:35Z

The port here was wrong

codecov · 2024-11-11T20:53:23Z

Codecov Report

Attention: Patch coverage is 68.75000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 58.76%. Comparing base (9794630) to head (44785f7).
Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
packages/dd-trace/src/opentelemetry/span.js	33.33%	2 Missing ⚠️
packages/dd-trace/src/opentracing/span.js	0.00%	2 Missing ⚠️
packages/dd-trace/src/noop/span.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #4875      +/-   ##
============================================
  Coverage          ?   58.76%              
============================================
  Files             0      280     +280     
  Lines             0    13017   +13017     
============================================
+ Hits              0     7649    +7649     
- Misses            0     5368    +5368

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nhulston · 2024-11-11T20:55:13Z

Fixes an issue with localstack. I followed the 'quick workaround' mentioned in this LocalStack comment

nhulston · 2024-11-12T19:32:24Z

Adding detailed comments here, since this function is public API and will be used in datadog-lambda-js

nhulston · 2024-11-12T19:36:14Z

No need to do this, since we already call

after(async () => { await resetLocalStackS3()

…ultipartUpload` requests.

apiarian-datadog

some notes, though mostly on the span pointers and their api. the javascript looks fine to my inexpert eyes

…s for trace+span id; add debug logs

nhulston · 2024-11-18T18:49:03Z

/remove

dd-devflow · 2024-11-18T18:49:09Z

Devflow running: `/remove`

View all feedbacks in Devflow UI.

2024-11-18 18:49:09 UTC ℹ️ Devflow: /remove

nhulston · 2024-11-18T18:57:53Z

/merge

dd-devflow · 2024-11-18T18:58:12Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-18 18:58:12 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-18 21:23:23 UTC ⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

apiarian-datadog

still approved!

apiarian-datadog

still approved!

apiarian-datadog

still approved!

nhulston · 2024-11-18T21:23:12Z

/merge -c

nhulston · 2024-11-18T21:24:28Z

/merge

dd-devflow · 2024-11-18T21:24:44Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-18 21:24:43 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-18 21:38:10 UTC ℹ️ MergeQueue: This merge request was already merged

This pull request was merged directly.

* Add span pointer info on S3 `putObject`, `copyObject`, and `completeMultipartUpload` requests. * Unit tests * small improvement * Create `addSpanPointer()` so we don't have to export a context with 0s for trace+span id; add debug logs * Add integration test for completeMultipartUpload; update unit test * Rename to `addSpanPointers()` * Update comments and make getting eTag more reliable * Validate parameters before calling `generateS3PointerHash` * add unit tests * Rename var to `SPAN_LINK_POINTER_KIND`; standardize the hashing function. * Set the span link kind in the `addSpanPointer()` functions so that downstream callers don't have to worry about passing it. * Move constants to constants.js; move `generatePointerHash` to util.js

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from 40640b8 to 6094019 Compare November 11, 2024 18:34

nhulston added the semver-minor label Nov 11, 2024

nhulston commented Nov 11, 2024

View reviewed changes

Comment thread packages/datadog-plugin-aws-sdk/test/s3.spec.js Outdated

Copy link
Copy Markdown

Contributor Author

nhulston Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The port here was wrong

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from 492496e to 7cba2af Compare November 11, 2024 20:37

nhulston commented Nov 11, 2024

View reviewed changes

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch 2 times, most recently from 2cee24b to f2dc82f Compare November 12, 2024 19:22

nhulston commented Nov 12, 2024

View reviewed changes

Add span pointer info on S3 putObject, copyObject, and `completeM…

bb321d2

…ultipartUpload` requests.

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from f2dc82f to 2a7a3d5 Compare November 12, 2024 19:39

Unit tests

4ca5b30

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from 2a7a3d5 to 4ca5b30 Compare November 12, 2024 20:18

nhulston marked this pull request as ready for review November 12, 2024 20:58

nhulston requested review from a team as code owners November 12, 2024 20:58

purple4reina reviewed Nov 12, 2024

View reviewed changes

Comment thread packages/datadog-plugin-aws-sdk/src/services/s3.js

nhulston and others added 2 commits November 13, 2024 09:57

small improvement

b5ef4c7

Merge branch 'master' into nicholas.hulston/s3-span-pointers

a36db0f

apiarian-datadog reviewed Nov 13, 2024

View reviewed changes

khanayan123 self-requested a review November 13, 2024 18:56

nhulston added 2 commits November 13, 2024 16:52

Create addSpanPointer() so we don't have to export a context with 0…

550f616

…s for trace+span id; add debug logs

Add integration test for completeMultipartUpload; update unit test

833d1b7

nhulston requested a review from apiarian-datadog November 13, 2024 21:53

Rename to addSpanPointers()

f11b2e6

khanayan123 reviewed Nov 14, 2024

View reviewed changes

Comment thread packages/datadog-plugin-aws-sdk/src/base.js

Update comments and make getting eTag more reliable

20bfdc2

dd-devflow Bot added the mergequeue-status: waiting label Nov 18, 2024

dd-devflow Bot added mergequeue-status: removed and removed mergequeue-status: waiting labels Nov 18, 2024

dd-devflow Bot added mergequeue-status: waiting and removed mergequeue-status: removed labels Nov 18, 2024

apiarian-datadog approved these changes Nov 18, 2024

View reviewed changes

dd-devflow Bot added mergequeue-status: removed and removed mergequeue-status: waiting labels Nov 18, 2024

dd-devflow Bot added mergequeue-status: waiting and removed mergequeue-status: removed labels Nov 18, 2024

bengl merged commit 6392a2e into master Nov 18, 2024

bengl deleted the nicholas.hulston/s3-span-pointers branch November 18, 2024 21:38

dd-devflow Bot added mergequeue-status: done and removed mergequeue-status: waiting labels Nov 18, 2024

rochdev mentioned this pull request Nov 19, 2024

v4.50.0 proposal #4887

Merged

rochdev mentioned this pull request Nov 19, 2024

v5.26.0 proposal #4888

Merged

nhulston mentioned this pull request Nov 20, 2024

chore: upgrade dd-trace to v4.50.0 DataDog/datadog-lambda-js#590

Merged

11 tasks

Conversation

nhulston commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Testing Guidelines

Additional Notes

Uh oh!

github-actions Bot commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overall package size

Uh oh!

pr-commenter Bot commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

nhulston Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nhulston Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

nhulston Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nhulston Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

apiarian-datadog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nhulston commented Nov 18, 2024

Uh oh!

dd-devflow Bot commented Nov 18, 2024

Devflow running: /remove

Uh oh!

nhulston commented Nov 18, 2024

Uh oh!

dd-devflow Bot commented Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Devflow running: /merge

Uh oh!

apiarian-datadog left a comment

Choose a reason for hiding this comment

Uh oh!

apiarian-datadog left a comment

Choose a reason for hiding this comment

Uh oh!

apiarian-datadog left a comment

Choose a reason for hiding this comment

Uh oh!

nhulston commented Nov 18, 2024

Uh oh!

nhulston commented Nov 18, 2024

Uh oh!

dd-devflow Bot commented Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Devflow running: /merge

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

nhulston commented Nov 11, 2024 •

edited

Loading

github-actions Bot commented Nov 11, 2024 •

edited

Loading

pr-commenter Bot commented Nov 11, 2024 •

edited

Loading

codecov Bot commented Nov 11, 2024 •

edited

Loading

nhulston Nov 12, 2024 •

edited

Loading

Devflow running: `/remove`

dd-devflow Bot commented Nov 18, 2024 •

edited

Loading

Devflow running: `/merge`

dd-devflow Bot commented Nov 18, 2024 •

edited

Loading

Devflow running: `/merge`