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 <noreply@anthropic.com>
This commit is contained in:
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 });
|
||||
}
|
||||
|
||||
@@ -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<typeof vi.fn>).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);
|
||||
});
|
||||
});
|
||||
@@ -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<string, unknown>;
|
||||
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 });
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user