From 1740176dde0b4c5f215b7b1bbedd4f34f52d9b25 Mon Sep 17 00:00:00 2001 From: Nicholas Hulston Date: Fri, 15 Nov 2024 10:36:52 -0500 Subject: [PATCH] Code review improvement --- src/trace/listener.ts | 14 +++++++-- src/utils/span-pointers.spec.ts | 51 +++++++++++++++++---------------- src/utils/span-pointers.ts | 37 +++++++++++------------- 3 files changed, 55 insertions(+), 47 deletions(-) diff --git a/src/trace/listener.ts b/src/trace/listener.ts index d067ebcb0..3f90b18e1 100644 --- a/src/trace/listener.ts +++ b/src/trace/listener.ts @@ -71,6 +71,12 @@ export interface TraceConfig { coldStartTraceSkipLib: string; } +interface SpanPointerAttributes { + pointerKind: string; + pointerDirection: string; + pointerHash: string; +} + export class TraceListener { private contextService: TraceContextService; private context?: Context; @@ -81,7 +87,7 @@ export class TraceListener { private wrappedCurrentSpan?: SpanWrapper; private triggerTags?: { [key: string]: string }; private lambdaSpanParentContext?: SpanContext; - private spanPointerAttributesList: object[] = []; + private spanPointerAttributesList: SpanPointerAttributes[] = []; public get currentTraceHeaders() { return this.contextService.currentTraceHeaders; @@ -206,7 +212,11 @@ export class TraceListener { if (this.wrappedCurrentSpan) { for (const attributes of this.spanPointerAttributesList) { - this.wrappedCurrentSpan.span.addSpanPointer(attributes); + this.wrappedCurrentSpan.span.addSpanPointer( + attributes.pointerKind, + attributes.pointerDirection, + attributes.pointerHash, + ); } } return false; diff --git a/src/utils/span-pointers.spec.ts b/src/utils/span-pointers.spec.ts index 8cccb69b9..1156392b2 100644 --- a/src/utils/span-pointers.spec.ts +++ b/src/utils/span-pointers.spec.ts @@ -1,19 +1,24 @@ import { getSpanPointerAttributes } from "./span-pointers"; import { eventTypes } from "../trace/trigger"; -import { SPAN_LINK_KIND, S3_PTR_KIND, SPAN_POINTER_DIRECTION } from "dd-trace/packages/dd-trace/src/span_pointers"; -import * as spanPointers from "dd-trace/packages/dd-trace/src/span_pointers"; +import { S3_PTR_KIND, SPAN_POINTER_DIRECTION } from "dd-trace/packages/dd-trace/src/constants"; +import * as util from "dd-trace/packages/dd-trace/src/util"; // Mock the external dependencies jest.mock("./log", () => ({ logDebug: jest.fn(), })); +interface SpanPointerAttributes { + pointerKind: string; + pointerDirection: string; + pointerHash: string; +} + describe("span-pointers utils", () => { - const mockS3PointerHash = "mock-hash-123"; + const mockPointerHash = "mock-hash-123"; beforeEach(() => { - // Mock the generateS3PointerHash function - jest.spyOn(spanPointers, "generateS3PointerHash").mockReturnValue(mockS3PointerHash); + jest.spyOn(util, "generatePointerHash").mockReturnValue(mockPointerHash); }); afterEach(() => { @@ -47,18 +52,17 @@ describe("span-pointers utils", () => { ], }; - const expected = [ + const expected: SpanPointerAttributes[] = [ { - "ptr.kind": S3_PTR_KIND, - "ptr.dir": SPAN_POINTER_DIRECTION.UPSTREAM, - "ptr.hash": mockS3PointerHash, - "link.kind": SPAN_LINK_KIND, + pointerKind: S3_PTR_KIND, + pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM, + pointerHash: mockPointerHash, }, ]; const result = getSpanPointerAttributes(eventTypes.s3, event); expect(result).toEqual(expected); - expect(spanPointers.generateS3PointerHash).toHaveBeenCalledWith("test-bucket", "test-key", "test-etag"); + expect(util.generatePointerHash).toHaveBeenCalledWith(["test-bucket", "test-key", "test-etag"]); }); it("processes multiple S3 records correctly", () => { @@ -85,18 +89,16 @@ describe("span-pointers utils", () => { ], }; - const expected = [ + const expected: SpanPointerAttributes[] = [ { - "ptr.kind": S3_PTR_KIND, - "ptr.dir": SPAN_POINTER_DIRECTION.UPSTREAM, - "ptr.hash": mockS3PointerHash, - "link.kind": SPAN_LINK_KIND, + pointerKind: S3_PTR_KIND, + pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM, + pointerHash: mockPointerHash, }, { - "ptr.kind": S3_PTR_KIND, - "ptr.dir": SPAN_POINTER_DIRECTION.UPSTREAM, - "ptr.hash": mockS3PointerHash, - "link.kind": SPAN_LINK_KIND, + pointerKind: S3_PTR_KIND, + pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM, + pointerHash: mockPointerHash, }, ]; @@ -134,12 +136,11 @@ describe("span-pointers utils", () => { ], }; - const expected = [ + const expected: SpanPointerAttributes[] = [ { - "ptr.kind": S3_PTR_KIND, - "ptr.dir": SPAN_POINTER_DIRECTION.UPSTREAM, - "ptr.hash": mockS3PointerHash, - "link.kind": SPAN_LINK_KIND, + pointerKind: S3_PTR_KIND, + pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM, + pointerHash: mockPointerHash, }, ]; diff --git a/src/utils/span-pointers.ts b/src/utils/span-pointers.ts index 393e84c87..92ddbd88c 100644 --- a/src/utils/span-pointers.ts +++ b/src/utils/span-pointers.ts @@ -1,17 +1,12 @@ import { eventTypes } from "../trace/trigger"; import { logDebug } from "./log"; -import { - SPAN_LINK_KIND, - S3_PTR_KIND, - SPAN_POINTER_DIRECTION, - generateS3PointerHash, -} from "dd-trace/packages/dd-trace/src/span_pointers"; +import { S3_PTR_KIND, SPAN_POINTER_DIRECTION } from "dd-trace/packages/dd-trace/src/constants"; +import { generatePointerHash } from "dd-trace/packages/dd-trace/src/util" interface SpanPointerAttributes { - "ptr.kind": string; - "ptr.dir": string; - "ptr.hash": string; - "link.kind": string; + pointerKind: string; + pointerDirection: string; + pointerHash: string; } /** @@ -40,12 +35,11 @@ export function getSpanPointerAttributes( function processS3Event(event: any): SpanPointerAttributes[] { const records = event.Records || []; - const spanPointerAttributesList = []; - const linkKind = SPAN_LINK_KIND; + const spanPointerAttributesList: SpanPointerAttributes[] = []; for (const record of records) { const eventName = record.eventName; - if (!["ObjectCreated:Put", "ObjectCreated:Copy", "ObjectCreated:CompleteMultipartUpload"].includes(eventName)) { + if (!eventName.startsWith("ObjectCreated")) { continue; } // Values are stored in the same place, regardless of AWS SDK v2/v3 or the event type. @@ -53,19 +47,22 @@ function processS3Event(event: any): SpanPointerAttributes[] { const s3Event = record?.s3; const bucketName = s3Event?.bucket?.name; const objectKey = s3Event?.object?.key; - const eTag = s3Event?.object?.eTag; + let eTag = s3Event?.object?.eTag; if (!bucketName || !objectKey || !eTag) { logDebug("Unable to calculate span pointer hash because of missing parameters."); continue; } - const pointerHash = generateS3PointerHash(bucketName, objectKey, eTag); - const spanPointerAttributes = { - "ptr.kind": S3_PTR_KIND, - "ptr.dir": SPAN_POINTER_DIRECTION.UPSTREAM, - "ptr.hash": pointerHash, - "link.kind": linkKind, + // 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); }