Skip to content

Commit c552877

Browse files
committed
refactor: simplify MSK header parsing per review
- Guard event.records with an early return instead of an empty fallback - Wrap the record loop in a single try/catch so errors log once - Return null from getParsedRecordHeaders when nothing decodes, lazily creating the headers map - Drop per-byte validation and rely on Buffer.from
1 parent f3620ec commit c552877

2 files changed

Lines changed: 34 additions & 31 deletions

File tree

‎src/trace/context/extractors/msk.spec.ts‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -79,20 +79,18 @@ describe("MSKEventTraceExtractor", () => {
7979
expect(extract).toHaveBeenNthCalledWith(2, { "x-datadog-parent-id": "456" });
8080
});
8181

82-
it.each([null, "123", [256], [-1], [1.5], ["49"], [null], [true], { "0": 49 }].map((value) => [value]))(
83-
"ignores invalid byte values: %j",
84-
(value) => {
85-
extract.mockReturnValue(spanContext);
86-
expect(
87-
extractor.extract(event({ "topic-0": [{ headers: [{ invalid: value }, ...record(datadogHeaders).headers] }] })),
88-
).toBe(spanContext);
89-
expect(extract).toHaveBeenCalledWith(datadogHeaders);
90-
},
91-
);
82+
it.each([null, "123", { "0": 49 }].map((value) => [value]))("skips non-array header values: %j", (value) => {
83+
extract.mockReturnValue(spanContext);
84+
expect(
85+
extractor.extract(event({ "topic-0": [{ headers: [{ invalid: value }, ...record(datadogHeaders).headers] }] })),
86+
).toBe(spanContext);
87+
expect(extract).toHaveBeenCalledWith(datadogHeaders);
88+
});
9289

9390
it.each([
9491
undefined,
9592
null,
93+
"invalid",
9694
{},
9795
{ "topic-0": [] },
9896
{ "topic-0": null },
@@ -103,13 +101,12 @@ describe("MSKEventTraceExtractor", () => {
103101
expect(extract).not.toHaveBeenCalled();
104102
});
105103

106-
it("continues after an extraction error", () => {
107-
extract
108-
.mockImplementationOnce(() => {
109-
throw new Error("invalid carrier");
110-
})
111-
.mockReturnValueOnce(spanContext);
112-
expect(extractor.extract(event({ "topic-0": [record(w3cHeaders), record(datadogHeaders)] }))).toBe(spanContext);
104+
it("returns null after an extraction error", () => {
105+
extract.mockImplementationOnce(() => {
106+
throw new Error("invalid carrier");
107+
});
108+
expect(extractor.extract(event({ "topic-0": [record(w3cHeaders), record(datadogHeaders)] }))).toBeNull();
109+
expect(extract).toHaveBeenCalledTimes(1);
113110
});
114111

115112
it("returns null when the tracer cannot extract context", () => {

‎src/trace/context/extractors/msk.ts‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,38 +9,44 @@ export class MSKEventTraceExtractor implements EventTraceExtractor {
99
constructor(private tracerWrapper: TracerWrapper) {}
1010

1111
extract(event: MSKEvent): SpanContextWrapper | null {
12-
// A Lambda span can have only one parent. Use the first record with valid
13-
// trace context, without combining headers from different records.
14-
for (const records of Object.values(event.records ?? {})) {
15-
if (!Array.isArray(records)) continue;
16-
for (const record of records) {
17-
try {
12+
if (!event?.records) {
13+
logDebug("Failed to extract trace context from MSK event");
14+
return null;
15+
}
16+
17+
try {
18+
// A Lambda span can have only one parent. Use the first record with valid
19+
// trace context, without combining headers from different records.
20+
for (const records of Object.values(event.records)) {
21+
if (!Array.isArray(records)) continue;
22+
for (const record of records) {
1823
const headers = this.getParsedRecordHeaders(record);
19-
if (Object.keys(headers).length === 0) continue;
24+
if (!headers) continue;
2025
const traceContext = this.tracerWrapper.extract(headers);
2126
if (traceContext) {
2227
logDebug("Extracted trace context from MSK event");
2328
return traceContext;
2429
}
25-
} catch (error) {
26-
handleExtractionError(error, "MSK");
2730
}
2831
}
32+
} catch (error) {
33+
handleExtractionError(error, "MSK");
2934
}
35+
3036
logDebug("Failed to extract trace context from MSK event");
3137
return null;
3238
}
3339

34-
private getParsedRecordHeaders(record: MSKRecord): Record<string, string> {
35-
const headers: Record<string, string> = Object.create(null);
36-
if (!Array.isArray(record?.headers)) return headers;
40+
private getParsedRecordHeaders(record: MSKRecord): Record<string, string> | null {
41+
if (!Array.isArray(record?.headers)) return null;
3742

43+
let headers: Record<string, string> | null = null;
3844
for (const entry of record.headers) {
3945
if (!entry || typeof entry !== "object" || Array.isArray(entry)) continue;
4046
for (const [name, value] of Object.entries(entry)) {
4147
// MSK serializes Kafka header bytes as integer arrays, not base64.
42-
// Validate before decoding because Buffer.from silently coerces invalid bytes.
43-
if (Array.isArray(value) && value.every((byte) => Number.isInteger(byte) && byte >= 0 && byte <= 255)) {
48+
if (Array.isArray(value)) {
49+
headers ??= {};
4450
headers[name.toLowerCase()] = Buffer.from(value).toString("utf8");
4551
}
4652
}

0 commit comments

Comments
 (0)