-
Notifications
You must be signed in to change notification settings - Fork 36
Add S3 Downstream Span Pointers #587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
4ac118d
a3b587b
340ac84
9e02a6a
d923f05
d30e722
b50916d
0715e81
3f7858d
0d1aecb
44b737a
86bab48
996b6f6
6c18199
bbf5575
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
import { getSpanPointerAttributes } from "./span-pointers"; | ||
import { eventTypes } from "../trace/trigger"; | ||
|
||
// tslint:disable-next-line:no-var-requires | ||
nhulston marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const { S3_PTR_KIND, SPAN_POINTER_DIRECTION } = require("dd-trace/packages/dd-trace/src/constants"); | ||
nhulston marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// tslint:disable-next-line:no-var-requires | ||
nhulston marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const util = require("dd-trace/packages/dd-trace/src/util"); | ||
nhulston marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Mock the external dependencies | ||
jest.mock("./log", () => ({ | ||
logDebug: jest.fn(), | ||
})); | ||
|
||
interface SpanPointerAttributes { | ||
pointerKind: string; | ||
pointerDirection: string; | ||
pointerHash: string; | ||
} | ||
|
||
describe("span-pointers utils", () => { | ||
const mockPointerHash = "mock-hash-123"; | ||
|
||
beforeEach(() => { | ||
jest.spyOn(util, "generatePointerHash").mockReturnValue(mockPointerHash); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not let the generate pointer hash function actually run? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I already have tests for |
||
}); | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
describe("getSpanPointerAttributes", () => { | ||
it("returns undefined when eventSource is undefined", () => { | ||
const result = getSpanPointerAttributes(undefined, {}); | ||
expect(result).toBeUndefined(); | ||
}); | ||
|
||
it("returns undefined for unsupported event types", () => { | ||
const result = getSpanPointerAttributes("unsupported" as eventTypes, {}); | ||
expect(result).toBeUndefined(); | ||
}); | ||
|
||
describe("S3 event processing", () => { | ||
it("processes single S3 record correctly", () => { | ||
const event = { | ||
Records: [ | ||
{ | ||
s3: { | ||
bucket: { name: "test-bucket" }, | ||
object: { | ||
key: "test-key", | ||
eTag: "test-etag", | ||
}, | ||
}, | ||
}, | ||
], | ||
}; | ||
|
||
const expected: SpanPointerAttributes[] = [ | ||
{ | ||
pointerKind: S3_PTR_KIND, | ||
pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM, | ||
pointerHash: mockPointerHash, | ||
}, | ||
]; | ||
|
||
const result = getSpanPointerAttributes(eventTypes.s3, event); | ||
expect(result).toEqual(expected); | ||
expect(util.generatePointerHash).toHaveBeenCalledWith(["test-bucket", "test-key", "test-etag"]); | ||
}); | ||
|
||
it("processes multiple S3 records correctly", () => { | ||
const event = { | ||
Records: [ | ||
{ | ||
s3: { | ||
bucket: { name: "bucket1" }, | ||
object: { | ||
key: "key1", | ||
eTag: "etag1", | ||
}, | ||
}, | ||
}, | ||
{ | ||
s3: { | ||
bucket: { name: "bucket2" }, | ||
object: { | ||
key: "key2", | ||
eTag: "etag2", | ||
}, | ||
}, | ||
}, | ||
], | ||
}; | ||
|
||
const expected: SpanPointerAttributes[] = [ | ||
{ | ||
pointerKind: S3_PTR_KIND, | ||
pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM, | ||
pointerHash: mockPointerHash, | ||
}, | ||
{ | ||
pointerKind: S3_PTR_KIND, | ||
pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM, | ||
pointerHash: mockPointerHash, | ||
}, | ||
]; | ||
|
||
const result = getSpanPointerAttributes(eventTypes.s3, event); | ||
expect(result).toEqual(expected); | ||
}); | ||
|
||
it("handles empty Records array", () => { | ||
const event = { Records: [] }; | ||
const result = getSpanPointerAttributes(eventTypes.s3, event); | ||
expect(result).toEqual([]); | ||
}); | ||
|
||
it("handles missing Records property", () => { | ||
const event = {}; | ||
const result = getSpanPointerAttributes(eventTypes.s3, event); | ||
expect(result).toEqual([]); | ||
}); | ||
|
||
it("skips invalid records but processes valid ones", () => { | ||
const event = { | ||
Records: [ | ||
{ | ||
// Invalid record missing s3 property | ||
}, | ||
{ | ||
s3: { | ||
bucket: { name: "valid-bucket" }, | ||
object: { | ||
key: "valid-key", | ||
eTag: "valid-etag", | ||
}, | ||
}, | ||
}, | ||
], | ||
}; | ||
|
||
const expected: SpanPointerAttributes[] = [ | ||
{ | ||
pointerKind: S3_PTR_KIND, | ||
pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM, | ||
pointerHash: mockPointerHash, | ||
}, | ||
]; | ||
|
||
const result = getSpanPointerAttributes(eventTypes.s3, event); | ||
expect(result).toEqual(expected); | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
import { eventTypes } from "../trace/trigger"; | ||
import { logDebug } from "./log"; | ||
|
||
interface SpanPointerAttributes { | ||
pointerKind: string; | ||
pointerDirection: string; | ||
pointerHash: string; | ||
nhulston marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
* Computes span pointer attributes | ||
* | ||
* @param {eventTypes} eventSource - The type of event being processed (e.g., S3, DynamoDB). | ||
* @param {any} event - The event object containing source-specific data. | ||
* @returns {SpanPointerAttributes[] | undefined} An array of span pointer attribute objects, or undefined if none could be computed. | ||
*/ | ||
export function getSpanPointerAttributes( | ||
eventSource: eventTypes | undefined, | ||
event: any, | ||
nhulston marked this conversation as resolved.
Show resolved
Hide resolved
|
||
): SpanPointerAttributes[] | undefined { | ||
if (!eventSource) { | ||
return; | ||
} | ||
|
||
switch (eventSource) { | ||
case eventTypes.s3: | ||
return processS3Event(event); | ||
default: | ||
logDebug(`Event type ${eventSource} not supported by span pointers.`); | ||
nhulston marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return; | ||
} | ||
} | ||
|
||
function processS3Event(event: any): SpanPointerAttributes[] { | ||
nhulston marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const records = event.Records || []; | ||
const spanPointerAttributesList: SpanPointerAttributes[] = []; | ||
|
||
// Get dependencies from tracer only when needed | ||
let constants; | ||
let util; | ||
try { | ||
constants = require("dd-trace/packages/dd-trace/src/constants"); | ||
util = require("dd-trace/packages/dd-trace/src/util"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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 was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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;
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah correct, just the lambda library, not the tracer. Let me review the whole PR |
||
} catch (err) { | ||
if (err instanceof Error) { | ||
logDebug("Failed to load dd-trace span pointer dependencies", err); | ||
} | ||
return spanPointerAttributesList; | ||
} | ||
|
||
const { S3_PTR_KIND, SPAN_POINTER_DIRECTION } = constants; | ||
const { generatePointerHash } = util; | ||
|
||
for (const record of records) { | ||
const eventName = record.eventName; | ||
if (!eventName.startsWith("ObjectCreated")) { | ||
continue; | ||
} | ||
// Values are stored in the same place, regardless of AWS SDK v2/v3 or the event type. | ||
// https://docs.aws.amazon.com/AmazonS3/latest/userguide/notification-content-structure.html | ||
const s3Event = record?.s3; | ||
const bucketName = s3Event?.bucket?.name; | ||
const objectKey = s3Event?.object?.key; | ||
let eTag = s3Event?.object?.eTag; | ||
|
||
if (!bucketName || !objectKey || !eTag) { | ||
logDebug("Unable to calculate span pointer hash because of missing parameters."); | ||
continue; | ||
} | ||
|
||
// https://github.com/DataDog/dd-span-pointer-rules/blob/main/AWS/S3/Object/README.md | ||
if (eTag.startsWith('"') && eTag.endsWith('"')) { | ||
eTag = eTag.slice(1, -1); | ||
} | ||
const pointerHash = generatePointerHash([bucketName, objectKey, eTag]); | ||
const spanPointerAttributes: SpanPointerAttributes = { | ||
pointerKind: S3_PTR_KIND, | ||
pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM, | ||
pointerHash, | ||
}; | ||
spanPointerAttributesList.push(spanPointerAttributes); | ||
} | ||
|
||
return spanPointerAttributesList; | ||
} |
Uh oh!
There was an error while loading. Please reload this page.