From 5c9cba318a8a4b9a835a2562c739cd1f5aad4260 Mon Sep 17 00:00:00 2001 From: Danijel Martinek Date: Mon, 11 May 2026 11:54:54 +0200 Subject: [PATCH] refactor(core-shared): delete SentryLogger (replaced by OtelLogger) Removes sentry-logger.ts and its test now that OtelLogger is the active ILogger impl in bind-otel-instrumentation.ts. Updates the binder test to assert OtelLogger (not SentryLogger) is bound. Fixes TypeScript errors: SentryLogRecordForwarder registers directly as LogRecordProcessor (not wrapped in BatchLogRecordProcessor which expects a LogRecordExporter); severityNumber undefined guard added. Co-Authored-By: Claude Sonnet 4.6 --- .../di/bind-otel-instrumentation.test.ts | 8 +- .../instrumentation/otel/init-server-node.ts | 11 +-- .../src/instrumentation/otel/sentry-bridge.ts | 6 +- .../sentry/sentry-logger.test.ts | 82 ------------------- .../instrumentation/sentry/sentry-logger.ts | 45 ---------- 5 files changed, 14 insertions(+), 138 deletions(-) delete mode 100644 packages/core-shared/src/instrumentation/sentry/sentry-logger.test.ts delete mode 100644 packages/core-shared/src/instrumentation/sentry/sentry-logger.ts diff --git a/packages/core-shared/src/instrumentation/di/bind-otel-instrumentation.test.ts b/packages/core-shared/src/instrumentation/di/bind-otel-instrumentation.test.ts index e25407e..126c192 100644 --- a/packages/core-shared/src/instrumentation/di/bind-otel-instrumentation.test.ts +++ b/packages/core-shared/src/instrumentation/di/bind-otel-instrumentation.test.ts @@ -19,7 +19,7 @@ import { bindOtelInstrumentation } from "@/instrumentation/di/bind-otel-instrume import { initOtelServerNode } from "@/instrumentation/otel/init-server-node"; import { INSTRUMENTATION_SYMBOLS } from "@/instrumentation/symbols"; import { OtelTracer } from "@/instrumentation/otel/otel-tracer"; -import { SentryLogger } from "@/instrumentation/sentry/sentry-logger"; +import { OtelLogger } from "@/instrumentation/otel/otel-logger"; describe("bindOtelInstrumentation", () => { beforeEach(() => { @@ -36,11 +36,11 @@ describe("bindOtelInstrumentation", () => { }); }); - it("binds OtelTracer + SentryLogger to the container", () => { + it("binds OtelTracer + OtelLogger to the container", () => { const c = new Container(); bindOtelInstrumentation(c, { dsn: "https://x@y/1", app: "web-next" }); expect(c.get(INSTRUMENTATION_SYMBOLS.TRACER)).toBeInstanceOf(OtelTracer); - expect(c.get(INSTRUMENTATION_SYMBOLS.LOGGER)).toBeInstanceOf(SentryLogger); + expect(c.get(INSTRUMENTATION_SYMBOLS.LOGGER)).toBeInstanceOf(OtelLogger); }); it("returns the tracer + logger instances", () => { @@ -50,6 +50,6 @@ describe("bindOtelInstrumentation", () => { app: "web-next", }); expect(tracer).toBeInstanceOf(OtelTracer); - expect(logger).toBeInstanceOf(SentryLogger); + expect(logger).toBeInstanceOf(OtelLogger); }); }); diff --git a/packages/core-shared/src/instrumentation/otel/init-server-node.ts b/packages/core-shared/src/instrumentation/otel/init-server-node.ts index 8d30541..a7db035 100644 --- a/packages/core-shared/src/instrumentation/otel/init-server-node.ts +++ b/packages/core-shared/src/instrumentation/otel/init-server-node.ts @@ -1,9 +1,8 @@ -import { NodeSDK, tracing, logs as sdkLogs } from "@opentelemetry/sdk-node"; +import { NodeSDK, tracing } from "@opentelemetry/sdk-node"; import { buildResource } from "./resource"; import { createSentryOtelBridge } from "./sentry-bridge"; const { BatchSpanProcessor } = tracing; -const { BatchLogRecordProcessor } = sdkLogs; export type InitOtelServerNodeOpts = { /** Sentry DSN. When empty, OTel SDK boots without the Sentry exporter. */ @@ -37,9 +36,11 @@ export function initOtelServerNode(opts: InitOtelServerNodeOpts): NodeSDK { const spanProcessors = bridge.spanProcessor ? [new BatchSpanProcessor(bridge.spanProcessor as never)] : []; - const logRecordProcessors = bridge.logRecordProcessor - ? [new BatchLogRecordProcessor(bridge.logRecordProcessor)] - : []; + // Register SentryLogRecordForwarder directly as a processor — it implements + // LogRecordProcessor (onEmit/forceFlush/shutdown) and forwards synchronously, + // so no batching wrapper is needed (unlike span processors that need batching + // to avoid blocking the hot path). + const logRecordProcessors = bridge.logRecordProcessor ? [bridge.logRecordProcessor] : []; const sdk = new NodeSDK({ resource, diff --git a/packages/core-shared/src/instrumentation/otel/sentry-bridge.ts b/packages/core-shared/src/instrumentation/otel/sentry-bridge.ts index 9ec63ed..5c3a59f 100644 --- a/packages/core-shared/src/instrumentation/otel/sentry-bridge.ts +++ b/packages/core-shared/src/instrumentation/otel/sentry-bridge.ts @@ -59,7 +59,9 @@ export class SentryLogRecordForwarder implements LogRecordProcessor { const tags = extractTags(attrs); const extra = extractExtras(attrs); - if (record.severityNumber >= SeverityNumber.ERROR) { + const severityNumber = record.severityNumber ?? SeverityNumber.INFO; + + if (severityNumber >= SeverityNumber.ERROR) { // Reconstruct the error from OTel semantic convention attributes const message = (attrs["exception.message"] as string | undefined) ?? String(record.body ?? ""); @@ -78,7 +80,7 @@ export class SentryLogRecordForwarder implements LogRecordProcessor { Sentry.captureException(err, { tags, extra, ...(fingerprint ? { fingerprint } : {}) }); } else { // Map severity to Sentry level - const level = record.severityNumber >= SeverityNumber.WARN ? "warning" : "info"; + const level = severityNumber >= SeverityNumber.WARN ? "warning" : "info"; Sentry.captureMessage(String(record.body ?? ""), level, { tags, extra }); } diff --git a/packages/core-shared/src/instrumentation/sentry/sentry-logger.test.ts b/packages/core-shared/src/instrumentation/sentry/sentry-logger.test.ts deleted file mode 100644 index fae8aba..0000000 --- a/packages/core-shared/src/instrumentation/sentry/sentry-logger.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -// packages/core-shared/src/instrumentation/sentry/sentry-logger.test.ts -import { describe, it, expect, vi, beforeEach } from "vitest"; - -vi.mock("@sentry/nextjs", () => ({ - captureException: vi.fn(), - captureMessage: vi.fn(), - addBreadcrumb: vi.fn(), - setUser: vi.fn(), -})); - -import * as Sentry from "@sentry/nextjs"; -import { SentryLogger } from "@/instrumentation/sentry/sentry-logger"; - -describe("SentryLogger", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it("captureException forwards to Sentry on first call", () => { - const logger = new SentryLogger(); - const err = new Error("boom"); - logger.captureException(err, { tags: { feature: "blog" } }); - expect(Sentry.captureException).toHaveBeenCalledTimes(1); - expect((Sentry.captureException as ReturnType).mock.calls[0]![0]).toBe(err); - }); - - it("captureException is a no-op when err already marked __sentryReported", () => { - const logger = new SentryLogger(); - const err = new Error("already-reported"); - Object.defineProperty(err, "__sentryReported", { value: true }); - logger.captureException(err); - expect(Sentry.captureException).not.toHaveBeenCalled(); - }); - - it("captureException marks err as __sentryReported after sending", () => { - const logger = new SentryLogger(); - const err = new Error("once"); - logger.captureException(err); - expect((err as unknown as { __sentryReported: boolean }).__sentryReported).toBe(true); - // Second call: no-op - logger.captureException(err); - expect(Sentry.captureException).toHaveBeenCalledTimes(1); - }); - - it("__sentryReported is non-enumerable", () => { - const logger = new SentryLogger(); - const err = new Error("x"); - logger.captureException(err); - expect(Object.keys(err)).not.toContain("__sentryReported"); - expect(JSON.stringify(err)).not.toContain("__sentryReported"); - }); - - it("captureMessage forwards to Sentry", () => { - const logger = new SentryLogger(); - logger.captureMessage("hello", "warning", { tags: { foo: "bar" } }); - expect(Sentry.captureMessage).toHaveBeenCalledWith( - "hello", - expect.objectContaining({ level: "warning" }), - ); - }); - - it("addBreadcrumb forwards to Sentry", () => { - const logger = new SentryLogger(); - logger.addBreadcrumb({ category: "test", message: "x", data: { k: "v" } }); - expect(Sentry.addBreadcrumb).toHaveBeenCalledTimes(1); - }); - - it("setUser strips non-id keys and warns in dev", () => { - const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); - const logger = new SentryLogger(); - logger.setUser({ id: "u1", email: "a@b.c", username: "alice" } as unknown as { id: string }); - expect(Sentry.setUser).toHaveBeenCalledWith({ id: "u1" }); - expect(warn).toHaveBeenCalled(); - warn.mockRestore(); - }); - - it("setUser passes null through", () => { - const logger = new SentryLogger(); - logger.setUser(null); - expect(Sentry.setUser).toHaveBeenCalledWith(null); - }); -}); diff --git a/packages/core-shared/src/instrumentation/sentry/sentry-logger.ts b/packages/core-shared/src/instrumentation/sentry/sentry-logger.ts deleted file mode 100644 index 69ec58a..0000000 --- a/packages/core-shared/src/instrumentation/sentry/sentry-logger.ts +++ /dev/null @@ -1,45 +0,0 @@ -// packages/core-shared/src/instrumentation/sentry/sentry-logger.ts -import * as Sentry from "@sentry/nextjs"; -import type { ILogger, Breadcrumb, CaptureContext } from "../logger.interface"; -import { isReported, markReported } from "../reported-flag"; - -export class SentryLogger implements ILogger { - captureException(err: unknown, ctx?: CaptureContext): void { - if (isReported(err)) return; - Sentry.captureException(err, ctx); - markReported(err); - } - - captureMessage( - msg: string, - level: "info" | "warning" | "error" = "info", - ctx?: CaptureContext, - ): void { - Sentry.captureMessage(msg, { level, ...ctx }); - } - - addBreadcrumb(b: Breadcrumb): void { - Sentry.addBreadcrumb({ - category: b.category, - message: b.message, - level: b.level, - data: b.data, - }); - } - - setUser(user: { id: string } | null): void { - if (user === null) { - Sentry.setUser(null); - return; - } - const { id, ...extra } = user as { id: string } & Record; - if (Object.keys(extra).length > 0) { - // R36 — strip non-id keys; warn in dev for visibility - console.warn( - "[SentryLogger.setUser] stripped non-id keys for PII safety:", - Object.keys(extra), - ); - } - Sentry.setUser({ id }); - } -}