Skip to content

Commit 340ac84

Browse files
nhulstonnhulston
committed
Code review improvement
1 parent a3b587b commit 340ac84

File tree

3 files changed

+55
-47
lines changed

3 files changed

+55
-47
lines changed

‎src/trace/listener.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ export interface TraceConfig {
7171
coldStartTraceSkipLib: string;
7272
}
7373

74+
interface SpanPointerAttributes {
75+
pointerKind: string;
76+
pointerDirection: string;
77+
pointerHash: string;
78+
}
79+
7480
export class TraceListener {
7581
private contextService: TraceContextService;
7682
private context?: Context;
@@ -81,7 +87,7 @@ export class TraceListener {
8187
private wrappedCurrentSpan?: SpanWrapper;
8288
private triggerTags?: { [key: string]: string };
8389
private lambdaSpanParentContext?: SpanContext;
84-
private spanPointerAttributesList: object[] = [];
90+
private spanPointerAttributesList: SpanPointerAttributes[] = [];
8591

8692
public get currentTraceHeaders() {
8793
return this.contextService.currentTraceHeaders;
@@ -206,7 +212,11 @@ export class TraceListener {
206212

207213
if (this.wrappedCurrentSpan) {
208214
for (const attributes of this.spanPointerAttributesList) {
209-
this.wrappedCurrentSpan.span.addSpanPointer(attributes);
215+
this.wrappedCurrentSpan.span.addSpanPointer(
216+
attributes.pointerKind,
217+
attributes.pointerDirection,
218+
attributes.pointerHash,
219+
);
210220
}
211221
}
212222
return false;

‎src/utils/span-pointers.spec.ts

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,24 @@
11
import { getSpanPointerAttributes } from "./span-pointers";
22
import { eventTypes } from "../trace/trigger";
3-
import { SPAN_LINK_KIND, S3_PTR_KIND, SPAN_POINTER_DIRECTION } from "dd-trace/packages/dd-trace/src/span_pointers";
4-
import * as spanPointers from "dd-trace/packages/dd-trace/src/span_pointers";
3+
import { S3_PTR_KIND, SPAN_POINTER_DIRECTION } from "dd-trace/packages/dd-trace/src/constants";
4+
import * as util from "dd-trace/packages/dd-trace/src/util";
55

66
// Mock the external dependencies
77
jest.mock("./log", () => ({
88
logDebug: jest.fn(),
99
}));
1010

11+
interface SpanPointerAttributes {
12+
pointerKind: string;
13+
pointerDirection: string;
14+
pointerHash: string;
15+
}
16+
1117
describe("span-pointers utils", () => {
12-
const mockS3PointerHash = "mock-hash-123";
18+
const mockPointerHash = "mock-hash-123";
1319

1420
beforeEach(() => {
15-
// Mock the generateS3PointerHash function
16-
jest.spyOn(spanPointers, "generateS3PointerHash").mockReturnValue(mockS3PointerHash);
21+
jest.spyOn(util, "generatePointerHash").mockReturnValue(mockPointerHash);
1722
});
1823

1924
afterEach(() => {
@@ -47,18 +52,17 @@ describe("span-pointers utils", () => {
4752
],
4853
};
4954

50-
const expected = [
55+
const expected: SpanPointerAttributes[] = [
5156
{
52-
"ptr.kind": S3_PTR_KIND,
53-
"ptr.dir": SPAN_POINTER_DIRECTION.UPSTREAM,
54-
"ptr.hash": mockS3PointerHash,
55-
"link.kind": SPAN_LINK_KIND,
57+
pointerKind: S3_PTR_KIND,
58+
pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM,
59+
pointerHash: mockPointerHash,
5660
},
5761
];
5862

5963
const result = getSpanPointerAttributes(eventTypes.s3, event);
6064
expect(result).toEqual(expected);
61-
expect(spanPointers.generateS3PointerHash).toHaveBeenCalledWith("test-bucket", "test-key", "test-etag");
65+
expect(util.generatePointerHash).toHaveBeenCalledWith(["test-bucket", "test-key", "test-etag"]);
6266
});
6367

6468
it("processes multiple S3 records correctly", () => {
@@ -85,18 +89,16 @@ describe("span-pointers utils", () => {
8589
],
8690
};
8791

88-
const expected = [
92+
const expected: SpanPointerAttributes[] = [
8993
{
90-
"ptr.kind": S3_PTR_KIND,
91-
"ptr.dir": SPAN_POINTER_DIRECTION.UPSTREAM,
92-
"ptr.hash": mockS3PointerHash,
93-
"link.kind": SPAN_LINK_KIND,
94+
pointerKind: S3_PTR_KIND,
95+
pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM,
96+
pointerHash: mockPointerHash,
9497
},
9598
{
96-
"ptr.kind": S3_PTR_KIND,
97-
"ptr.dir": SPAN_POINTER_DIRECTION.UPSTREAM,
98-
"ptr.hash": mockS3PointerHash,
99-
"link.kind": SPAN_LINK_KIND,
99+
pointerKind: S3_PTR_KIND,
100+
pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM,
101+
pointerHash: mockPointerHash,
100102
},
101103
];
102104

@@ -134,12 +136,11 @@ describe("span-pointers utils", () => {
134136
],
135137
};
136138

137-
const expected = [
139+
const expected: SpanPointerAttributes[] = [
138140
{
139-
"ptr.kind": S3_PTR_KIND,
140-
"ptr.dir": SPAN_POINTER_DIRECTION.UPSTREAM,
141-
"ptr.hash": mockS3PointerHash,
142-
"link.kind": SPAN_LINK_KIND,
141+
pointerKind: S3_PTR_KIND,
142+
pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM,
143+
pointerHash: mockPointerHash,
143144
},
144145
];
145146

‎src/utils/span-pointers.ts

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,12 @@
11
import { eventTypes } from "../trace/trigger";
22
import { logDebug } from "./log";
3-
import {
4-
SPAN_LINK_KIND,
5-
S3_PTR_KIND,
6-
SPAN_POINTER_DIRECTION,
7-
generateS3PointerHash,
8-
} from "dd-trace/packages/dd-trace/src/span_pointers";
3+
import { S3_PTR_KIND, SPAN_POINTER_DIRECTION } from "dd-trace/packages/dd-trace/src/constants";
4+
import { generatePointerHash } from "dd-trace/packages/dd-trace/src/util";
95

106
interface SpanPointerAttributes {
11-
"ptr.kind": string;
12-
"ptr.dir": string;
13-
"ptr.hash": string;
14-
"link.kind": string;
7+
pointerKind: string;
8+
pointerDirection: string;
9+
pointerHash: string;
1510
}
1611

1712
/**
@@ -40,32 +35,34 @@ export function getSpanPointerAttributes(
4035

4136
function processS3Event(event: any): SpanPointerAttributes[] {
4237
const records = event.Records || [];
43-
const spanPointerAttributesList = [];
44-
const linkKind = SPAN_LINK_KIND;
38+
const spanPointerAttributesList: SpanPointerAttributes[] = [];
4539

4640
for (const record of records) {
4741
const eventName = record.eventName;
48-
if (!["ObjectCreated:Put", "ObjectCreated:Copy", "ObjectCreated:CompleteMultipartUpload"].includes(eventName)) {
42+
if (!eventName.startsWith("ObjectCreated")) {
4943
continue;
5044
}
5145
// Values are stored in the same place, regardless of AWS SDK v2/v3 or the event type.
5246
// https://docs.aws.amazon.com/AmazonS3/latest/userguide/notification-content-structure.html
5347
const s3Event = record?.s3;
5448
const bucketName = s3Event?.bucket?.name;
5549
const objectKey = s3Event?.object?.key;
56-
const eTag = s3Event?.object?.eTag;
50+
let eTag = s3Event?.object?.eTag;
5751

5852
if (!bucketName || !objectKey || !eTag) {
5953
logDebug("Unable to calculate span pointer hash because of missing parameters.");
6054
continue;
6155
}
6256

63-
const pointerHash = generateS3PointerHash(bucketName, objectKey, eTag);
64-
const spanPointerAttributes = {
65-
"ptr.kind": S3_PTR_KIND,
66-
"ptr.dir": SPAN_POINTER_DIRECTION.UPSTREAM,
67-
"ptr.hash": pointerHash,
68-
"link.kind": linkKind,
57+
// https://github.com/DataDog/dd-span-pointer-rules/blob/main/AWS/S3/Object/README.md
58+
if (eTag.startsWith('"') && eTag.endsWith('"')) {
59+
eTag = eTag.slice(1, -1);
60+
}
61+
const pointerHash = generatePointerHash([bucketName, objectKey, eTag]);
62+
const spanPointerAttributes: SpanPointerAttributes = {
63+
pointerKind: S3_PTR_KIND,
64+
pointerDirection: SPAN_POINTER_DIRECTION.UPSTREAM,
65+
pointerHash,
6966
};
7067
spanPointerAttributesList.push(spanPointerAttributes);
7168
}

0 commit comments

Comments
 (0)