Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/trace/context/extractor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
EventBridgeSQSEventTraceExtractor,
HTTPEventTraceExtractor,
KinesisEventTraceExtractor,
MSKEventTraceExtractor,
SNSEventTraceExtractor,
SNSSQSEventTraceExtractor,
SQSEventTraceExtractor,
Expand Down Expand Up @@ -812,6 +813,11 @@ describe("getTraceEventExtractor", () => {
["a string", "some-value"],
["a number", 1234],
["an object which doesn't match any expected event", { custom: "event" }],
["MSK without records", { eventSource: "aws:kafka" }],
["MSK with null records", { eventSource: "aws:kafka", records: null }],
["MSK with array records", { eventSource: "aws:kafka", records: [] }],
["MSK with string records", { eventSource: "aws:kafka", records: "invalid" }],
["records without an MSK event source", { records: { "topic-0": [] } }],
])("returns undefined when event is '%s'", (_, event) => {
const tracerWrapper = new TracerWrapper();
const traceContextExtractor = new TraceContextExtractor(tracerWrapper, {} as TraceConfig);
Expand Down Expand Up @@ -894,6 +900,12 @@ describe("getTraceEventExtractor", () => {
],
},
],
[
"MSKEventTraceExtractor",
"MSK event",
MSKEventTraceExtractor,
{ eventSource: "aws:kafka", records: { "topic-0": [] } },
],
[
"KinesisEventTraceExtractor",
"Kinesis stream event",
Expand Down
3 changes: 3 additions & 0 deletions src/trace/context/extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
EventBridgeSQSEventTraceExtractor,
HTTPEventTraceExtractor,
KinesisEventTraceExtractor,
MSKEventTraceExtractor,
LambdaContextTraceExtractor,
SNSEventTraceExtractor,
SNSSQSEventTraceExtractor,
Expand Down Expand Up @@ -95,6 +96,8 @@ export class TraceContextExtractor {
if (EventValidator.isEventBridgeSQSEvent(event))
return new EventBridgeSQSEventTraceExtractor(this.tracerWrapper, this.config);
if (EventValidator.isAppSyncResolverEvent(event)) return new AppSyncEventTraceExtractor(this.tracerWrapper);
if (EventValidator.isMSKEvent(event)) return new MSKEventTraceExtractor(this.tracerWrapper);

if (EventValidator.isSQSEvent(event)) return new SQSEventTraceExtractor(this.tracerWrapper, this.config);
if (EventValidator.isKinesisStreamEvent(event))
return new KinesisEventTraceExtractor(this.tracerWrapper, this.config);
Expand Down
1 change: 1 addition & 0 deletions src/trace/context/extractors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ export { StepFunctionEventTraceExtractor } from "./step-function";
export { LambdaContextTraceExtractor } from "./lambda-context";
export { CustomTraceExtractor } from "./custom";
export { DurableExecutionEventTraceExtractor } from "./durable-execution";
export { MSKEventTraceExtractor } from "./msk";
122 changes: 122 additions & 0 deletions src/trace/context/extractors/msk.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import { MSKEvent } from "aws-lambda";
import { MSKEventTraceExtractor } from "./msk";
import { TracerWrapper } from "../../tracer-wrapper";
import { SpanContextWrapper } from "../../span-context-wrapper";
import { TraceSource } from "../../trace-context-service";

const datadogHeaders = {
"x-datadog-trace-id": "667309514221035538",
"x-datadog-parent-id": "1350735035497811828",
"x-datadog-sampling-priority": "1",
"x-datadog-tags": "_dd.p.tid=0123456789abcdef",
};
const w3cHeaders = {
traceparent: "00-0123456789abcdef0942c4f3b84c8812-12becbb3b2d5bd74-01",
tracestate: "dd=s:1;t.dm:-0",
};
const record = (headers: Record<string, string>) => ({
headers: Object.entries(headers).map(([name, value]) => ({ [name]: Array.from(Buffer.from(value)) })),
});
const event = (records: any): MSKEvent => ({ eventSource: "aws:kafka", records } as MSKEvent);

// Mock the tracer boundary so these tests exercise MSK decoding and record selection.
describe("MSKEventTraceExtractor", () => {
const spanContext = new SpanContextWrapper(
{ toTraceId: () => datadogHeaders["x-datadog-trace-id"], toSpanId: () => datadogHeaders["x-datadog-parent-id"] },
TraceSource.Event,
);
let extract: jest.Mock;
let extractor: MSKEventTraceExtractor;

beforeEach(() => {
extract = jest.fn().mockReturnValue(null);
extractor = new MSKEventTraceExtractor({ extract } as unknown as TracerWrapper);
});

it.each([
["Datadog", datadogHeaders],
["W3C", w3cHeaders],
["combined", { ...datadogHeaders, ...w3cHeaders }],
])("decodes and forwards all %s propagation headers", (_, headers) => {
extract.mockReturnValue(spanContext);
expect(extractor.extract(event({ "topic-0": [record(headers)] }))).toBe(spanContext);
expect(extract).toHaveBeenCalledWith(headers);
expect(extract).toHaveBeenCalledTimes(1);
});

it("normalizes header names and decodes UTF-8", () => {
extractor.extract(
event({ "topic-0": [record({ TraceParent: w3cHeaders.traceparent, baggage: "city=São Paulo" })] }),
);
expect(extract).toHaveBeenCalledWith({ traceparent: w3cHeaders.traceparent, baggage: "city=São Paulo" });
});

it("uses the last value for duplicate headers", () => {
const message = { headers: [...record({ traceparent: "old" }).headers, ...record(w3cHeaders).headers] };
extractor.extract(event({ "topic-0": [message] }));
expect(extract).toHaveBeenCalledWith(w3cHeaders);
});

it("skips untraced records across partitions and stops at the first valid context", () => {
extract.mockReturnValueOnce(null).mockReturnValueOnce(spanContext);
expect(
extractor.extract(
event({
"topic-0": [{ headers: [] }, record({ traceparent: "invalid" })],
"topic-1": [record(datadogHeaders), record(w3cHeaders)],
}),
),
).toBe(spanContext);
expect(extract).toHaveBeenCalledTimes(2);
expect(extract).toHaveBeenNthCalledWith(2, datadogHeaders);
});

it("never combines partial trace headers from different records", () => {
extractor.extract(
event({ "topic-0": [record({ "x-datadog-trace-id": "123" }), record({ "x-datadog-parent-id": "456" })] }),
);
expect(extract).toHaveBeenNthCalledWith(1, { "x-datadog-trace-id": "123" });
expect(extract).toHaveBeenNthCalledWith(2, { "x-datadog-parent-id": "456" });
});

it.each([null, "123", { "0": 49 }].map((value) => [value]))("skips non-array header values: %j", (value) => {
extract.mockReturnValue(spanContext);
expect(
extractor.extract(event({ "topic-0": [{ headers: [{ invalid: value }, ...record(datadogHeaders).headers] }] })),
).toBe(spanContext);
expect(extract).toHaveBeenCalledWith(datadogHeaders);
});

it.each([
undefined,
null,
"invalid",
{},
{ "topic-0": [] },
{ "topic-0": null },
{ "topic-0": "invalid" },
{ "topic-0": [null, {}, { headers: null }, { headers: "invalid" }, { headers: [null, 42, []] }] },
])("returns null for empty or malformed records: %j", (records) => {
expect(extractor.extract(event(records))).toBeNull();
expect(extract).not.toHaveBeenCalled();
});

it("returns null after an extraction error", () => {
extract.mockImplementationOnce(() => {
throw new Error("invalid carrier");
});
expect(extractor.extract(event({ "topic-0": [record(w3cHeaders), record(datadogHeaders)] }))).toBeNull();
expect(extract).toHaveBeenCalledTimes(1);
});

it("returns null when the tracer cannot extract context", () => {
expect(extractor.extract(event({ "topic-0": [record(datadogHeaders)] }))).toBeNull();
});

it("does not mutate the event", () => {
const payload = event({ "topic-0": [record(datadogHeaders)] });
const original = JSON.stringify(payload);
extractor.extract(payload);
expect(JSON.stringify(payload)).toBe(original);
});
});
56 changes: 56 additions & 0 deletions src/trace/context/extractors/msk.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { MSKEvent, MSKRecord } from "aws-lambda";
import { logDebug } from "../../../utils";
import { EventTraceExtractor } from "../extractor";
import { handleExtractionError } from "../extractor-utils";
import { TracerWrapper } from "../../tracer-wrapper";
import { SpanContextWrapper } from "../../span-context-wrapper";

export class MSKEventTraceExtractor implements EventTraceExtractor {
constructor(private tracerWrapper: TracerWrapper) {}

extract(event: MSKEvent): SpanContextWrapper | null {
if (!event?.records) {
logDebug("Failed to extract trace context from MSK event");
return null;
}

try {
// A Lambda span can have only one parent. Use the first record with valid
// trace context, without combining headers from different records.
for (const records of Object.values(event.records)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every other batch extractor takes trace context from the first record only: sqs.ts, sns.ts, sns-sqs.ts, event-bridge-sqs.ts, and kinesis.ts all use Records[0]. Their loops over every record are only for DSM checkpoints. Can we do the same here? Read the first record of the first topic-partition, try to extract from it, and return.

This keeps MSK consistent with the other event sources, so the Lambda span's parent is always the first record. It also avoids decoding headers and calling tracer.extract on up to 10k records in every untraced batch.

The test cases that expect a later record's context to win would need to change to expect null.

if (!Array.isArray(records)) continue;
for (const record of records) {
const headers = this.getParsedRecordHeaders(record);
if (!headers) continue;
const traceContext = this.tracerWrapper.extract(headers);
if (traceContext) {
logDebug("Extracted trace context from MSK event");
return traceContext;
}
}
}
} catch (error) {
handleExtractionError(error, "MSK");
}

logDebug("Failed to extract trace context from MSK event");
return null;
Comment thread
lucassarcanjo marked this conversation as resolved.
}

private getParsedRecordHeaders(record: MSKRecord): Record<string, string> | null {
if (!Array.isArray(record?.headers)) return null;

let headers: Record<string, string> | null = null;
for (const entry of record.headers) {
if (!entry || typeof entry !== "object" || Array.isArray(entry)) continue;
for (const [name, value] of Object.entries(entry)) {
// MSK serializes Kafka header bytes as integer arrays, not base64.
if (Array.isArray(value)) {
headers ??= {};
headers[name.toLowerCase()] = Buffer.from(value).toString("utf8");
}
}
}
return headers;
}
}
10 changes: 10 additions & 0 deletions src/utils/event-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
AppSyncResolverEvent,
EventBridgeEvent,
KinesisStreamEvent,
MSKEvent,
SNSEvent,
SNSMessage,
SQSEvent,
Expand Down Expand Up @@ -75,6 +76,15 @@ export class EventValidator {
return Array.isArray(event.Records) && event.Records.length > 0 && event.Records[0].kinesis !== undefined;
}

static isMSKEvent(event: any): event is MSKEvent {
return (
event?.eventSource === "aws:kafka" &&
event.records !== null &&
typeof event.records === "object" &&
!Array.isArray(event.records)
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lambda's self-managed Kafka trigger (any non-MSK cluster, e.g. on EC2 or Confluent Cloud) sends the same records and headers shape as MSK, but with eventSource: "SelfManagedKafka". Right now isMSKEvent only accepts "aws:kafka"`, so those Lambdas would still start a disconnected trace.

Could we accept both values?

static isKafkaEvent(event: any): event is MSKEvent | SelfManagedKafkaEvent {
  return (
    (event?.eventSource === "aws:kafka" || event?.eventSource === "SelfManagedKafka") &&
    event.records !== null &&
    typeof event.records === "object" &&
    !Array.isArray(event.records)
  );
}

The header-decoding logic should work for both as-is. A test fixture with eventSource: "SelfManagedKafka" would cover it. If self-managed Kafka is intentionally out of scope for this PR, that's fine too. A follow-up issue would be enough.

}

static isDurableExecutionEvent(event: any): boolean {
return typeof event.DurableExecutionArn === "string";
}
Expand Down
Loading