From 9acf16f399598e461f8a6a16469e23878159af81 Mon Sep 17 00:00:00 2001 From: Danijel Martinek Date: Mon, 11 May 2026 12:40:11 +0200 Subject: [PATCH] chore: address Phase 5 review polish (test descriptions, version conflict comment, mock cleanup) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I3: bind-production.test.ts instrumentation orthogonality tests updated to use bindOtelInstrumentation as primary name (bindSentryInstrumentation alias still wired in mock setup for deprecation-alias coverage, not in assertions). I4: as never cast in init-server-node.ts annotated with explanation of the sdk-trace-base / sdk-node TypeScript version conflict that necessitates it. I5: SentryLogRecordProcessor removed from @sentry/opentelemetry mock in no-instrumentation.ts — that class does not exist in v10. Co-Authored-By: Claude Sonnet 4.6 --- .../src/server/bind-production.test.ts | 20 +++++++++++-------- .../instrumentation/otel/init-server-node.ts | 10 +++++++++- .../src/setup/no-instrumentation.ts | 6 +----- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/apps/web-next/src/server/bind-production.test.ts b/apps/web-next/src/server/bind-production.test.ts index 5f42eff..af13334 100644 --- a/apps/web-next/src/server/bind-production.test.ts +++ b/apps/web-next/src/server/bind-production.test.ts @@ -160,44 +160,48 @@ describe("bindAll instrumentation orthogonality (Rule 0, R47)", () => { vi.unstubAllEnvs(); }); + // In the mock setup above, bindSentryInstrumentation is an alias that points + // to the same spy as bindOtelInstrumentation. Assertions against either name + // verify the same call, which also validates the deprecation alias is wired. + it("DSN absent → bindNoopInstrumentation regardless of NODE_ENV (R48)", async () => { vi.stubEnv("WEB_NEXT_SENTRY_DSN", ""); vi.stubEnv("NODE_ENV", "production"); const { bindAll } = await import("./bind-production"); - const { bindNoopInstrumentation, bindSentryInstrumentation } = await import( + const { bindNoopInstrumentation, bindOtelInstrumentation } = await import( "@repo/core-shared/instrumentation" ); await bindAll(); expect(bindNoopInstrumentation).toHaveBeenCalledOnce(); - expect(bindSentryInstrumentation).not.toHaveBeenCalled(); + expect(bindOtelInstrumentation).not.toHaveBeenCalled(); }); - it("DSN set → bindSentryInstrumentation regardless of NODE_ENV", async () => { + it("DSN set → bindOtelInstrumentation regardless of NODE_ENV", async () => { vi.stubEnv("WEB_NEXT_SENTRY_DSN", "https://x@y/1"); vi.stubEnv("NODE_ENV", "development"); const { bindAll } = await import("./bind-production"); - const { bindNoopInstrumentation, bindSentryInstrumentation } = await import( + const { bindNoopInstrumentation, bindOtelInstrumentation } = await import( "@repo/core-shared/instrumentation" ); await bindAll(); - expect(bindSentryInstrumentation).toHaveBeenCalledOnce(); + expect(bindOtelInstrumentation).toHaveBeenCalledOnce(); expect(bindNoopInstrumentation).not.toHaveBeenCalled(); }); - it("Sentry instrumentation works alongside dev seed (USE_DEV_SEED=true)", async () => { + it("OTel instrumentation works alongside dev seed (USE_DEV_SEED=true)", async () => { vi.stubEnv("USE_DEV_SEED", "true"); vi.stubEnv("WEB_NEXT_SENTRY_DSN", "https://x@y/1"); const { bindAll } = await import("./bind-production"); - const { bindSentryInstrumentation } = await import("@repo/core-shared/instrumentation"); + const { bindOtelInstrumentation } = await import("@repo/core-shared/instrumentation"); const { bindDevSeedBlog } = await import("@repo/blog/di/bind-dev-seed"); await bindAll(); - expect(bindSentryInstrumentation).toHaveBeenCalledOnce(); + expect(bindOtelInstrumentation).toHaveBeenCalledOnce(); expect(bindDevSeedBlog).toHaveBeenCalledOnce(); }); 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 b3e6416..b2f0ba8 100644 --- a/packages/core-shared/src/instrumentation/otel/init-server-node.ts +++ b/packages/core-shared/src/instrumentation/otel/init-server-node.ts @@ -42,7 +42,15 @@ export function initOtelServerNode(opts: InitOtelServerNodeOpts): NodeSDK { // PiiScrubSpanProcessor runs FIRST so the Sentry exporter never sees raw PII. const spanProcessors = bridge.spanProcessor - ? [new PiiScrubSpanProcessor(), new BatchSpanProcessor(bridge.spanProcessor as never)] + ? [ + new PiiScrubSpanProcessor(), + // `as never` works around a TypeScript version conflict: `core-shared`'s direct + // dep on `@opentelemetry/sdk-trace-base@1.30.1` has subtly incompatible types + // vs the 1.28.0 bundled by `sdk-node@0.55.0`. The runtime objects are compatible; + // the structural mismatch is type-only. Phase 1 implementer chose this rather than + // constraining sdk-trace-base to 1.28.x to avoid losing future bug fixes. + new BatchSpanProcessor(bridge.spanProcessor as never), + ] : [new PiiScrubSpanProcessor()]; // PiiScrubLogRecordProcessor runs FIRST for the same reason. diff --git a/packages/core-testing/src/setup/no-instrumentation.ts b/packages/core-testing/src/setup/no-instrumentation.ts index aee602a..8d2752b 100644 --- a/packages/core-testing/src/setup/no-instrumentation.ts +++ b/packages/core-testing/src/setup/no-instrumentation.ts @@ -91,11 +91,7 @@ vi.mock("@sentry/opentelemetry", () => ({ forceFlush() { return Promise.resolve(); } shutdown() { return Promise.resolve(); } }, - SentryLogRecordProcessor: class { - onEmit() {} - forceFlush() { return Promise.resolve(); } - shutdown() { return Promise.resolve(); } - }, + // SentryLogRecordProcessor does NOT exist in @sentry/opentelemetry v10 — omitted. // No-op Sentry.init wrapper used by sentry-bridge.ts init: vi.fn(), }));