From dabffec893dd48777c44aae0cdd64cb1ba95ebf8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 19:16:59 +0000 Subject: [PATCH 1/2] Omit invocationId from error/critical logs to allow aggregation invocationId is unique per Lambda invocation, so including it on error and critical log entries defeated log-aggregation/error-tracking tools that group errors by identical fields. info/warn/track logs are unaffected and still carry it for correlation. --- src/logger.ts | 13 ++++++++++--- tests/logger.property.spec.ts | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/logger.ts b/src/logger.ts index 7e5edff8..5b3ac0ad 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -19,7 +19,7 @@ export interface LogEntry { title: string; code?: string; timestamp: string; - invocationId: string; + invocationId?: string; containerId: string; trackPoints?: TrackPoint[]; stack?: string; @@ -193,6 +193,9 @@ export class RequestLogger implements Logger { private emit(level: LogLevel, title: string, context?: Record): void { const includeTrackPoints = level === "track" || level === "error" || level === "critical"; const includeStack = level === "error" || level === "critical"; + // error/critical logs omit invocationId: it's unique per invocation, so including it + // defeats log-aggregation tools that group errors by identical fields. + const includeInvocationId = level !== "error" && level !== "critical"; // Extract code from context if present let code: string | undefined; @@ -214,7 +217,7 @@ export class RequestLogger implements Logger { title, ...(code !== undefined ? { code } : {}), timestamp: DateTime.utc().toISO()!, - invocationId: this.invocationId, + ...(includeInvocationId ? { invocationId: this.invocationId } : {}), containerId: this.containerId, ...(includeTrackPoints && this.trackPoints.length > 0 ? { trackPoints: this.trackPoints } : {}), ...(includeStack ? { stack: new Error().stack ?? "" } : {}), @@ -223,7 +226,11 @@ export class RequestLogger implements Logger { // Re-assign required fields AFTER spread to guarantee context cannot overwrite them entry.level = level.toUpperCase() as unknown as LogLevel; entry.title = title; - entry.invocationId = this.invocationId; + if (includeInvocationId) { + entry.invocationId = this.invocationId; + } else { + delete entry.invocationId; + } entry.containerId = this.containerId; if (code !== undefined) entry.code = code; diff --git a/tests/logger.property.spec.ts b/tests/logger.property.spec.ts index d1812cf5..abc05ab4 100644 --- a/tests/logger.property.spec.ts +++ b/tests/logger.property.spec.ts @@ -64,7 +64,12 @@ describe("Log entry structural invariant", () => { expect(entry.level).toBe(level.toUpperCase()); expect(entry.title).toBe("test.msg"); expect(entry.containerId).toBe("test1234"); - expect(entry.invocationId).toBe("test-invocation"); + if (level === "error" || level === "critical") { + // Omitted so error-tracking tools can aggregate identical errors across invocations. + expect(entry.invocationId).toBeUndefined(); + } else { + expect(entry.invocationId).toBe("test-invocation"); + } expect(new Date(entry.timestamp as string).toISOString()).toBe(entry.timestamp); }, ); @@ -105,6 +110,16 @@ describe("Context merge preserves required fields", () => { expect(entry.invocationId).not.toBe("FAKE"); expect(entry.invocationId).toBe("test-invocation"); }); + + it("error level omits invocationId even when context supplies one", () => { + const logger = new RequestLogger({ containerId: "test1234" }); + logger.startInvocation("test-invocation"); + + callLevel(logger, "error", "real.message", { invocationId: "FAKE" }); + + const entry = lastEntryFromSpies(logSpy, warnSpy, errorSpy); + expect(entry.invocationId).toBeUndefined(); + }); }); describe("Track points included for track/error/critical levels", () => { From 873413620636762f9fe8f9b58200882cbe0e0428 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 19:24:15 +0000 Subject: [PATCH 2/2] Revert: keep invocationId on error/critical logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit invocationId was never embedded in the log message/title text, so omitting it from error/critical entries wasn't necessary to keep aggregation working — it's a separate structured field. Revert to always including it for request correlation. --- src/logger.ts | 13 +++---------- tests/logger.property.spec.ts | 17 +---------------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/src/logger.ts b/src/logger.ts index 5b3ac0ad..7e5edff8 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -19,7 +19,7 @@ export interface LogEntry { title: string; code?: string; timestamp: string; - invocationId?: string; + invocationId: string; containerId: string; trackPoints?: TrackPoint[]; stack?: string; @@ -193,9 +193,6 @@ export class RequestLogger implements Logger { private emit(level: LogLevel, title: string, context?: Record): void { const includeTrackPoints = level === "track" || level === "error" || level === "critical"; const includeStack = level === "error" || level === "critical"; - // error/critical logs omit invocationId: it's unique per invocation, so including it - // defeats log-aggregation tools that group errors by identical fields. - const includeInvocationId = level !== "error" && level !== "critical"; // Extract code from context if present let code: string | undefined; @@ -217,7 +214,7 @@ export class RequestLogger implements Logger { title, ...(code !== undefined ? { code } : {}), timestamp: DateTime.utc().toISO()!, - ...(includeInvocationId ? { invocationId: this.invocationId } : {}), + invocationId: this.invocationId, containerId: this.containerId, ...(includeTrackPoints && this.trackPoints.length > 0 ? { trackPoints: this.trackPoints } : {}), ...(includeStack ? { stack: new Error().stack ?? "" } : {}), @@ -226,11 +223,7 @@ export class RequestLogger implements Logger { // Re-assign required fields AFTER spread to guarantee context cannot overwrite them entry.level = level.toUpperCase() as unknown as LogLevel; entry.title = title; - if (includeInvocationId) { - entry.invocationId = this.invocationId; - } else { - delete entry.invocationId; - } + entry.invocationId = this.invocationId; entry.containerId = this.containerId; if (code !== undefined) entry.code = code; diff --git a/tests/logger.property.spec.ts b/tests/logger.property.spec.ts index abc05ab4..d1812cf5 100644 --- a/tests/logger.property.spec.ts +++ b/tests/logger.property.spec.ts @@ -64,12 +64,7 @@ describe("Log entry structural invariant", () => { expect(entry.level).toBe(level.toUpperCase()); expect(entry.title).toBe("test.msg"); expect(entry.containerId).toBe("test1234"); - if (level === "error" || level === "critical") { - // Omitted so error-tracking tools can aggregate identical errors across invocations. - expect(entry.invocationId).toBeUndefined(); - } else { - expect(entry.invocationId).toBe("test-invocation"); - } + expect(entry.invocationId).toBe("test-invocation"); expect(new Date(entry.timestamp as string).toISOString()).toBe(entry.timestamp); }, ); @@ -110,16 +105,6 @@ describe("Context merge preserves required fields", () => { expect(entry.invocationId).not.toBe("FAKE"); expect(entry.invocationId).toBe("test-invocation"); }); - - it("error level omits invocationId even when context supplies one", () => { - const logger = new RequestLogger({ containerId: "test1234" }); - logger.startInvocation("test-invocation"); - - callLevel(logger, "error", "real.message", { invocationId: "FAKE" }); - - const entry = lastEntryFromSpies(logSpy, warnSpy, errorSpy); - expect(entry.invocationId).toBeUndefined(); - }); }); describe("Track points included for track/error/critical levels", () => {