fix(otel): consolidate to single OTel SDK init at instrumentation.register hook
All three apps' instrumentation.ts files now call initOtelServerNode directly instead of initSentryServer/initSentryServerNode, closing the startup window where @sentry/nextjs auto-instrumentation could send unscrubbed errors before bindAll() fires. bindOtelInstrumentation no longer calls initOtelServerNode (SDK init belongs at app boot, binding at request scope). Orphaned sentry/ init-server*.ts files deleted; their package.json subpath exports removed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -16,9 +16,7 @@
|
||||
"./instrumentation": "./src/instrumentation/index.ts",
|
||||
"./instrumentation/otel": "./src/instrumentation/otel/index.ts",
|
||||
"./instrumentation/otel/init-server-node": "./src/instrumentation/otel/init-server-node.ts",
|
||||
"./instrumentation/sentry/init-server": "./src/instrumentation/sentry/init-server.ts",
|
||||
"./instrumentation/sentry/init-client": "./src/instrumentation/sentry/init-client.ts",
|
||||
"./instrumentation/sentry/init-server-node": "./src/instrumentation/sentry/init-server-node.ts",
|
||||
"./instrumentation/sentry/init-client-react": "./src/instrumentation/sentry/init-client-react.ts"
|
||||
},
|
||||
"scripts": {
|
||||
|
||||
@@ -10,32 +10,22 @@ vi.mock("@sentry/nextjs", () => ({
|
||||
setUser: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/instrumentation/otel/init-server-node", () => ({
|
||||
initOtelServerNode: vi.fn(),
|
||||
}));
|
||||
|
||||
import { Container } from "inversify";
|
||||
import { bindOtelInstrumentation } from "@/instrumentation/di/bind-otel-instrumentation";
|
||||
import { initOtelServerNode } from "@/instrumentation/otel/init-server-node";
|
||||
import { INSTRUMENTATION_SYMBOLS } from "@/instrumentation/symbols";
|
||||
import { OtelTracer } from "@/instrumentation/otel/otel-tracer";
|
||||
import { OtelLogger } from "@/instrumentation/otel/otel-logger";
|
||||
|
||||
// NOTE: initOtelServerNode is intentionally NOT called by bindOtelInstrumentation.
|
||||
// The SDK is initialized by each app's instrumentation.ts register() hook so PII
|
||||
// scrub processors are active before the first request (C1 fix). There is therefore
|
||||
// no initOtelServerNode mock or call-count assertion here.
|
||||
|
||||
describe("bindOtelInstrumentation", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("calls initOtelServerNode with the provided DSN and app as serviceName", () => {
|
||||
const c = new Container();
|
||||
bindOtelInstrumentation(c, { dsn: "https://x@y/1", app: "web-next" });
|
||||
expect(initOtelServerNode).toHaveBeenCalledTimes(1);
|
||||
expect((initOtelServerNode as ReturnType<typeof vi.fn>).mock.calls[0]![0]).toMatchObject({
|
||||
dsn: "https://x@y/1",
|
||||
serviceName: "web-next",
|
||||
});
|
||||
});
|
||||
|
||||
it("binds OtelTracer + OtelLogger to the container", () => {
|
||||
const c = new Container();
|
||||
bindOtelInstrumentation(c, { dsn: "https://x@y/1", app: "web-next" });
|
||||
@@ -52,4 +42,14 @@ describe("bindOtelInstrumentation", () => {
|
||||
expect(tracer).toBeInstanceOf(OtelTracer);
|
||||
expect(logger).toBeInstanceOf(OtelLogger);
|
||||
});
|
||||
|
||||
it("rebinds when called a second time (idempotent container state)", () => {
|
||||
const c = new Container();
|
||||
bindOtelInstrumentation(c, { dsn: "https://x@y/1", app: "web-next" });
|
||||
// Second call should not throw even if TRACER is already bound.
|
||||
expect(() =>
|
||||
bindOtelInstrumentation(c, { dsn: "https://x@y/2", app: "cms" }),
|
||||
).not.toThrow();
|
||||
expect(c.get(INSTRUMENTATION_SYMBOLS.TRACER)).toBeInstanceOf(OtelTracer);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,7 +3,6 @@ import type { Container } from "inversify";
|
||||
import { OtelTracer } from "../otel/otel-tracer";
|
||||
import { OtelLogger } from "../otel/otel-logger";
|
||||
import { OtelMetrics } from "../otel/otel-metrics";
|
||||
import { initOtelServerNode } from "../otel/init-server-node";
|
||||
import { INSTRUMENTATION_SYMBOLS } from "../symbols";
|
||||
import type { ITracer, ILogger, IMetrics } from "../index";
|
||||
|
||||
@@ -13,24 +12,20 @@ export type BindOtelOpts = {
|
||||
release?: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Binds OtelTracer, OtelLogger, and OtelMetrics to the DI container.
|
||||
*
|
||||
* NOTE: The OTel NodeSDK is NOT initialized here. It is initialized by each
|
||||
* app's instrumentation.ts `register()` hook (Next.js convention / server-entry
|
||||
* hook for TanStack) so that PII scrub processors are active before the very
|
||||
* first request handler runs — before bindAll() fires. Calling initOtelServerNode
|
||||
* here as well would create a second SDK init path and reintroduce the startup
|
||||
* window vulnerability (C1 fix).
|
||||
*/
|
||||
export function bindOtelInstrumentation(
|
||||
container: Container,
|
||||
opts: BindOtelOpts,
|
||||
): { tracer: ITracer; logger: ILogger; metrics: IMetrics } {
|
||||
const environment =
|
||||
process.env["SENTRY_ENVIRONMENT"] ??
|
||||
process.env["VERCEL_ENV"] ??
|
||||
process.env["NODE_ENV"] ??
|
||||
"development";
|
||||
const release = opts.release ?? process.env["VERCEL_GIT_COMMIT_SHA"] ?? "unknown";
|
||||
|
||||
initOtelServerNode({
|
||||
dsn: opts.dsn,
|
||||
serviceName: opts.app,
|
||||
environment,
|
||||
release,
|
||||
});
|
||||
|
||||
const tracer = new OtelTracer();
|
||||
const logger = new OtelLogger();
|
||||
const metrics = new OtelMetrics();
|
||||
|
||||
@@ -27,6 +27,4 @@ export {
|
||||
export { bindOtelInstrumentation as bindSentryInstrumentation } from "./di/bind-otel-instrumentation";
|
||||
export type { BindOtelOpts as BindSentryOpts } from "./di/bind-otel-instrumentation";
|
||||
|
||||
// initSentryServerNode removed from barrel — callers use the subpath export directly:
|
||||
// @repo/core-shared/instrumentation/sentry/init-server-node
|
||||
export { initSentryClientReact } from "./sentry/init-client-react";
|
||||
|
||||
@@ -1,53 +0,0 @@
|
||||
// packages/core-shared/src/instrumentation/sentry/init-server-node.test.ts
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
vi.mock("@sentry/node", () => ({
|
||||
init: vi.fn(),
|
||||
}));
|
||||
|
||||
import * as SentryNode from "@sentry/node";
|
||||
import { initSentryServerNode } from "@/instrumentation/sentry/init-server-node";
|
||||
|
||||
describe("initSentryServerNode", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("calls SentryNode.init with sendDefaultPii: false (R31)", () => {
|
||||
initSentryServerNode({ dsn: "https://x@y/1", app: "web-tanstack" });
|
||||
const call = (SentryNode.init as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
expect(call["sendDefaultPii"]).toBe(false);
|
||||
});
|
||||
|
||||
it("does NOT attach beforeSend/beforeSendTransaction (scrubbing is at OTel layer)", () => {
|
||||
initSentryServerNode({ dsn: "https://x@y/1", app: "web-tanstack" });
|
||||
const call = (SentryNode.init as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
// PII scrubbing happens in PiiScrubSpanProcessor / PiiScrubLogRecordProcessor
|
||||
// before data reaches the Sentry exporter — no need for beforeSend hooks here.
|
||||
expect(call["beforeSend"]).toBeUndefined();
|
||||
expect(call["beforeSendTransaction"]).toBeUndefined();
|
||||
});
|
||||
|
||||
it("tags events with the app name", () => {
|
||||
initSentryServerNode({ dsn: "https://x@y/1", app: "web-tanstack" });
|
||||
const call = (SentryNode.init as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
const initialScope = call["initialScope"] as { tags?: Record<string, string> };
|
||||
expect(initialScope?.tags?.["app"]).toBe("web-tanstack");
|
||||
});
|
||||
|
||||
it("is a no-op when dsn is missing", () => {
|
||||
initSentryServerNode({ dsn: "", app: "web-tanstack" });
|
||||
expect(SentryNode.init).not.toHaveBeenCalled();
|
||||
initSentryServerNode({ dsn: undefined as unknown as string, app: "web-tanstack" });
|
||||
expect(SentryNode.init).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -1,40 +0,0 @@
|
||||
// packages/core-shared/src/instrumentation/sentry/init-server-node.ts
|
||||
// NOTE: PII scrubbing is now handled at the OTel pipeline layer via PiiScrubSpanProcessor
|
||||
// and PiiScrubLogRecordProcessor (see otel/pii-scrub-processor.ts). The beforeSend /
|
||||
// beforeSendTransaction hooks are no longer needed here (R32, R33 still enforced at OTel layer).
|
||||
import * as SentryNode from "@sentry/node";
|
||||
import type { InitServerOpts } from "./init-server";
|
||||
|
||||
/**
|
||||
* Server-side init for non-Next.js runtimes (TanStack Start). Mirrors
|
||||
* init-server.ts but uses @sentry/node directly. R31, R32, R33 still apply
|
||||
* (scrubbing enforced at the OTel processor layer before data reaches Sentry).
|
||||
*/
|
||||
export function initSentryServerNode(opts: InitServerOpts): void {
|
||||
if (!opts.dsn) return;
|
||||
|
||||
const isProd = process.env["NODE_ENV"] === "production";
|
||||
const tracesSampleRate =
|
||||
process.env["SENTRY_TRACES_SAMPLE_RATE"] !== undefined
|
||||
? Number(process.env["SENTRY_TRACES_SAMPLE_RATE"])
|
||||
: isProd
|
||||
? 0.1
|
||||
: 1.0;
|
||||
|
||||
const environment =
|
||||
process.env["SENTRY_ENVIRONMENT"] ??
|
||||
process.env["VERCEL_ENV"] ??
|
||||
process.env["NODE_ENV"] ??
|
||||
"development";
|
||||
const release = opts.release ?? process.env["VITE_GIT_COMMIT_SHA"] ?? "unknown";
|
||||
|
||||
SentryNode.init({
|
||||
dsn: opts.dsn,
|
||||
environment,
|
||||
release,
|
||||
tracesSampleRate,
|
||||
sendDefaultPii: false, // R31
|
||||
// R32, R33: PII scrubbing happens at the OTel processor layer before data reaches Sentry.
|
||||
initialScope: { tags: { app: opts.app } },
|
||||
});
|
||||
}
|
||||
@@ -1,101 +0,0 @@
|
||||
// packages/core-shared/src/instrumentation/sentry/init-server.test.ts
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
vi.mock("@sentry/nextjs", () => ({
|
||||
init: vi.fn(),
|
||||
replayIntegration: vi.fn(() => ({ name: "Replay" })),
|
||||
}));
|
||||
|
||||
import * as Sentry from "@sentry/nextjs";
|
||||
import { initSentryServer } from "@/instrumentation/sentry/init-server";
|
||||
|
||||
describe("initSentryServer", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("calls Sentry.init with sendDefaultPii: false (R31)", () => {
|
||||
initSentryServer({ dsn: "https://x@y/1", app: "web-next" });
|
||||
expect(Sentry.init).toHaveBeenCalledTimes(1);
|
||||
const call = (Sentry.init as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
expect(call["sendDefaultPii"]).toBe(false);
|
||||
});
|
||||
|
||||
it("passes the configured DSN", () => {
|
||||
initSentryServer({ dsn: "https://x@y/1", app: "web-next" });
|
||||
const call = (Sentry.init as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
expect(call["dsn"]).toBe("https://x@y/1");
|
||||
});
|
||||
|
||||
it("does NOT attach beforeSend/beforeSendTransaction (scrubbing is at OTel layer)", () => {
|
||||
initSentryServer({ dsn: "https://x@y/1", app: "web-next" });
|
||||
const call = (Sentry.init as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
// PII scrubbing happens in PiiScrubSpanProcessor / PiiScrubLogRecordProcessor
|
||||
// before data reaches the Sentry exporter — no need for beforeSend hooks here.
|
||||
expect(call["beforeSend"]).toBeUndefined();
|
||||
expect(call["beforeSendTransaction"]).toBeUndefined();
|
||||
});
|
||||
|
||||
it("uses SENTRY_TRACES_SAMPLE_RATE env when set", () => {
|
||||
vi.stubEnv("SENTRY_TRACES_SAMPLE_RATE", "0.25");
|
||||
initSentryServer({ dsn: "https://x@y/1", app: "web-next" });
|
||||
const call = (Sentry.init as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
expect(call["tracesSampleRate"]).toBe(0.25);
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
it("defaults tracesSampleRate to 1.0 in dev, 0.1 in production", () => {
|
||||
// Save and clear rate env to test default logic
|
||||
const prevRate = process.env["SENTRY_TRACES_SAMPLE_RATE"];
|
||||
delete process.env["SENTRY_TRACES_SAMPLE_RATE"];
|
||||
|
||||
vi.stubEnv("NODE_ENV", "development");
|
||||
initSentryServer({ dsn: "https://x@y/1", app: "web-next" });
|
||||
expect(
|
||||
((Sentry.init as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Record<string, unknown>)[
|
||||
"tracesSampleRate"
|
||||
],
|
||||
).toBe(1.0);
|
||||
|
||||
(Sentry.init as ReturnType<typeof vi.fn>).mockClear();
|
||||
vi.stubEnv("NODE_ENV", "production");
|
||||
initSentryServer({ dsn: "https://x@y/1", app: "web-next" });
|
||||
expect(
|
||||
((Sentry.init as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Record<string, unknown>)[
|
||||
"tracesSampleRate"
|
||||
],
|
||||
).toBe(0.1);
|
||||
|
||||
vi.unstubAllEnvs();
|
||||
if (prevRate !== undefined) process.env["SENTRY_TRACES_SAMPLE_RATE"] = prevRate;
|
||||
});
|
||||
|
||||
it("tags events with the app name", () => {
|
||||
initSentryServer({ dsn: "https://x@y/1", app: "web-next" });
|
||||
const call = (Sentry.init as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
const initialScope = call["initialScope"] as { tags?: Record<string, string> };
|
||||
expect(initialScope?.tags?.["app"]).toBe("web-next");
|
||||
});
|
||||
|
||||
it("is a no-op when dsn is empty/undefined", () => {
|
||||
initSentryServer({ dsn: "", app: "web-next" });
|
||||
expect(Sentry.init).not.toHaveBeenCalled();
|
||||
initSentryServer({ dsn: undefined as unknown as string, app: "web-next" });
|
||||
expect(Sentry.init).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -1,40 +0,0 @@
|
||||
// packages/core-shared/src/instrumentation/sentry/init-server.ts
|
||||
// NOTE: PII scrubbing is now handled at the OTel pipeline layer via PiiScrubSpanProcessor
|
||||
// and PiiScrubLogRecordProcessor (see otel/pii-scrub-processor.ts). The beforeSend /
|
||||
// beforeSendTransaction hooks are no longer needed here (R32, R33 still enforced at OTel layer).
|
||||
import * as Sentry from "@sentry/nextjs";
|
||||
|
||||
export type InitServerOpts = {
|
||||
dsn: string | undefined;
|
||||
app: "web-next" | "cms" | "web-tanstack";
|
||||
release?: string;
|
||||
};
|
||||
|
||||
export function initSentryServer(opts: InitServerOpts): void {
|
||||
if (!opts.dsn) return;
|
||||
|
||||
const isProd = process.env["NODE_ENV"] === "production";
|
||||
const tracesSampleRate =
|
||||
process.env["SENTRY_TRACES_SAMPLE_RATE"] !== undefined
|
||||
? Number(process.env["SENTRY_TRACES_SAMPLE_RATE"])
|
||||
: isProd
|
||||
? 0.1
|
||||
: 1.0;
|
||||
|
||||
const environment =
|
||||
process.env["SENTRY_ENVIRONMENT"] ??
|
||||
process.env["VERCEL_ENV"] ??
|
||||
process.env["NODE_ENV"] ??
|
||||
"development";
|
||||
const release = opts.release ?? process.env["VERCEL_GIT_COMMIT_SHA"] ?? "unknown";
|
||||
|
||||
Sentry.init({
|
||||
dsn: opts.dsn,
|
||||
environment,
|
||||
release,
|
||||
tracesSampleRate,
|
||||
sendDefaultPii: false, // R31 — non-negotiable
|
||||
// R32, R33: PII scrubbing happens at the OTel processor layer before data reaches Sentry.
|
||||
initialScope: { tags: { app: opts.app } },
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user