Add S3 Downstream Span Pointers by nhulston · Pull Request #587 · DataDog/datadog-lambda-js

nhulston · 2024-11-14T17:41:49Z

What does this PR do?

Adds span pointers to spans for Lambdas triggered by putObject, copyObject, and completeMultipartUpload events.

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.

More thorough: 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.

Additional Notes

This handles the 'downstream' case, for Lambdas that are triggered S3 updates. This PR adds span pointers on the upstream case (Lambdas that make the S3 requests).
S3 and Dynamo are supported in Python. I will soon open another PR to add support for Dynamo.

Types of Changes

Bug fix
New feature
Breaking change
Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

This PR's description is comprehensive
This PR contains breaking changes that are documented in the description
This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
This PR impacts documentation, and it has been updated (or a ticket has been logged)
This PR's changes are covered by the automated tests
This PR collects user input/sensitive content into Datadog
This PR passes the integration tests (ask a Datadog member to run the tests)

apiarian-datadog

the span pointer stuff looks fine, though some aspects of the api are a little strange. we may want someone with more js experience to take a look as well.

apiarian-datadog · 2024-11-22T20:49:11Z

+  const mockPointerHash = "mock-hash-123";
+
+  beforeEach(() => {
+    jest.spyOn(util, "generatePointerHash").mockReturnValue(mockPointerHash);


why not let the generate pointer hash function actually run?

I already have tests for generatePointerHash in the tracer; i just wanted to test only the logic behind the functions created in this PR. That way, if we ever make changes to how the generatePointerHash function works in the tracer, it won't break the tests here. And the changes will be automatically propagated to here, requiring no code updates.

purple4reina · 2024-11-26T20:22:17Z

+  let util;
+  try {
+    constants = require("dd-trace/packages/dd-trace/src/constants");
+    util = require("dd-trace/packages/dd-trace/src/util");


@duncanista, can you help answer a question for me?

I recall when you added support for w3c trace propagation to datadog-lambda-js, you said that instead of directly importing the tracer from dd-trace, we create an interface to wrap it.

If that's correct, then is there any issue with us requiring other files from ddtrace here?

There might be an issue when customer is not using the tracer (some do it), it would essentially crash, ideally, we'd have a better way to get this type of data

@duncanista Could you take another look at the updated code from my latest commit? It should be safe and avoid any crashing:

let S3_PTR_KIND; let SPAN_POINTER_DIRECTION; let generatePointerHash; try { const constants = require("dd-trace/packages/dd-trace/src/constants"); const util = require("dd-trace/packages/dd-trace/src/util"); ({ S3_PTR_KIND, SPAN_POINTER_DIRECTION } = constants); ({ generatePointerHash } = util); } catch (err) { if (err instanceof Error) { logDebug("Failed to load dd-trace span pointer dependencies", err); } return spanPointerAttributesList; }

@duncanista, Can you give more details (or point me to a doc somewhere) about how "customer is not using the tracer" is implemented? Do you mean they just aren't including ddtrace in their package.json file? I'd like to be able to create a small sample app that I can test this PR with that "is not using the tracer".

Yeah correct, just the lambda library, not the tracer. Let me review the whole PR

duncanista

This should be fine – I'm approving to unblock, but this is not ideal on how we are importing it. I guess its fail-safe, yet this is not making it friendly to eventually make this package ESBuild compatible

duncanista · 2024-11-26T22:25:23Z

Please add integration tests for this, due to how you are requiring this packages from the external package

purple4reina

looks great!

datadog-datadog-prod-us1 Bot reviewed Nov 14, 2024

View reviewed changes

Comment thread src/utils/span-pointers.ts Outdated

Comment thread src/utils/span-pointers.ts

Comment thread src/trace/trigger.ts

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from d0a666e to c8aa5a8 Compare November 14, 2024 17:45

datadog-datadog-prod-us1 Bot reviewed Nov 14, 2024

View reviewed changes

Comment thread src/utils/span-pointers.ts

nhulston added 2 commits November 14, 2024 12:55

Add span pointers downstream

4ac118d

Add unit tests

a3b587b

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from c8aa5a8 to a3b587b Compare November 14, 2024 17:55

nhulston mentioned this pull request Nov 14, 2024

[serverless] Add S3 Span Pointers DataDog/dd-trace-js#4875

Merged

nhulston marked this pull request as ready for review November 14, 2024 18:46

nhulston requested a review from a team as a code owner November 14, 2024 18:46

apiarian-datadog approved these changes Nov 14, 2024

View reviewed changes

Comment thread src/utils/span-pointers.ts Outdated

Comment thread src/utils/span-pointers.ts Outdated

datadog-datadog-prod-us1 Bot reviewed Nov 15, 2024

View reviewed changes

Comment thread src/utils/span-pointers.ts Outdated

Comment thread src/trace/listener.ts Outdated

Comment thread src/trace/listener.ts Outdated

Comment thread src/trace/listener.ts Outdated

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from c9e623b to ea5594b Compare November 15, 2024 16:23

nhulston marked this pull request as draft November 15, 2024 21:50

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from 2309fdd to 1740176 Compare November 18, 2024 22:05

Code review improvement

340ac84

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from 1740176 to 340ac84 Compare November 18, 2024 22:06

nhulston and others added 4 commits November 18, 2024 17:06

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

9e02a6a

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

d923f05

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

d30e722

Only import tracer dependencies when we need it

b50916d

datadog-datadog-prod-us1 Bot reviewed Nov 22, 2024

View reviewed changes

Comment thread src/utils/span-pointers.spec.ts

Comment thread src/utils/span-pointers.spec.ts

Comment thread src/utils/span-pointers.spec.ts

nhulston marked this pull request as ready for review November 22, 2024 20:37

datadog-datadog-prod-us1 Bot reviewed Nov 22, 2024

View reviewed changes

Comment thread src/utils/span-pointers.spec.ts

Fix tests

0715e81

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from 5ffcfdb to 0715e81 Compare November 22, 2024 20:41

apiarian-datadog reviewed Nov 22, 2024

View reviewed changes

Comment thread src/utils/span-pointers.ts Outdated

Remove noisy debug

3f7858d

purple4reina reviewed Nov 26, 2024

View reviewed changes

Comment thread src/trace/listener.ts Outdated

purple4reina reviewed Nov 26, 2024

View reviewed changes

Comment thread src/trace/listener.ts Outdated

Code review improvements

0d1aecb

datadog-datadog-prod-us1 Bot reviewed Nov 26, 2024

View reviewed changes

Comment thread src/utils/span-pointers.ts

Comment thread src/utils/span-pointers.ts

purple4reina reviewed Nov 26, 2024

View reviewed changes

Comment thread src/trace/listener.ts Outdated

duncanista reviewed Nov 26, 2024

View reviewed changes

Comment thread src/utils/span-pointers.ts Outdated

duncanista approved these changes Nov 26, 2024

View reviewed changes

Code review improvements 2

44b737a

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from 6fab017 to 44b737a Compare November 26, 2024 22:52

nhulston added 2 commits November 27, 2024 16:15

Add integration tests

86bab48

snapshots

996b6f6

nhulston force-pushed the nicholas.hulston/s3-span-pointers branch from 06d82e7 to 996b6f6 Compare November 27, 2024 21:16

nhulston requested a review from purple4reina November 27, 2024 21:22

purple4reina approved these changes Dec 4, 2024

View reviewed changes

nhulston and others added 2 commits December 5, 2024 09:03

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

6c18199

Add/update Node 22 snapshots

bbf5575

nhulston merged commit 4061917 into main Dec 6, 2024

nhulston deleted the nicholas.hulston/s3-span-pointers branch December 6, 2024 14:56

Conversation

nhulston commented Nov 14, 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

Types of Changes

Check all that apply

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

apiarian-datadog Nov 22, 2024

Choose a reason for hiding this comment

Uh oh!

nhulston Nov 22, 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!

Uh oh!

Uh oh!

purple4reina Nov 26, 2024

Choose a reason for hiding this comment

Uh oh!

duncanista Nov 26, 2024

Choose a reason for hiding this comment

Uh oh!

nhulston Nov 26, 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!

purple4reina Nov 26, 2024

Choose a reason for hiding this comment

Uh oh!

duncanista Nov 26, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

duncanista left a comment

Choose a reason for hiding this comment

Uh oh!

duncanista commented Nov 26, 2024

Uh oh!

purple4reina left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nhulston commented Nov 14, 2024 •

edited

Loading

nhulston Nov 22, 2024 •

edited

Loading

nhulston Nov 26, 2024 •

edited

Loading