feat(selfhost): sampled Sentry spans for review latency

.env.example

@@ -230,7 +230,13 @@ REDIS_URL=redis://redis:6379                # REQUIRED for the self-host review
 # --- Sentry error tracking (optional) ---
 # SENTRY_DSN=                                # enables self-host Sentry capture; unset = complete no-op
 # SENTRY_ENVIRONMENT=production
-# SENTRY_TRACES_SAMPLE_RATE=0                # traces are off by default; errors still report
+# SENTRY_TRACES_SAMPLE_RATE=0                # traces/spans are off by default; errors still report. Set a LOW rate
+#                                            #   (e.g. 0.05) with SENTRY_DSN to sample review tracing: each sampled
+#                                            #   review emits a connected trace — the queue-job span (whole-review
+#                                            #   latency) with the AI-provider span nested — so you can filter slow
+#                                            #   or failed STAGES in Sentry without reading scattered logs. Spans
+#                                            #   carry only safe dimensions (repo, job type, provider/model); never
+#                                            #   prompts, diffs, tokens, or bodies. Leave 0 to keep tracing a no-op.
 # SENTRY_RELEASE=                            # custom images only: set this ONLY when you uploaded source maps for
 #                                            #   the exact built bundle under this exact release id. Future official
 #                                            #   images bake GITTENSORY_VERSION=gittensory-selfhost@<version>, so do

src/selfhost/ai.ts

@@ -10,7 +10,7 @@ import type { CombineStrategy, OnMerge } from "../services/ai-review";
 import { isConfiguredSelfHostProvider, resolveConfiguredProviderNames } from "./ai-config";
 export { assertNoLegacySharedAiEnv } from "./ai-config";
 import { incr } from "./metrics";
-import { withOtelSpan } from "./otel";
+import { withReviewSpan } from "./tracing";
 import { delimiter } from "node:path";
 
 interface AiRunOptions {
@@ -627,7 +627,7 @@ function runProviderWithOtel(
   model: string,
   options: AiRunOptions,
 ): Promise<AiResult> {
-  return withOtelSpan(
+  return withReviewSpan(
     "selfhost.ai.provider",
     { "ai.provider": provider.name, "ai.model": model || "default", "ai.request_kind": requestKind(options) },
     () => provider.ai.run(model, options),

src/selfhost/pg-queue.ts

@@ -5,7 +5,7 @@
 import type { Pool } from "pg";
 import { logAudit, extractPayloadType } from "./audit";
 import { incr } from "./metrics";
-import { withOtelSpan } from "./otel";
+import { withReviewSpan } from "./tracing";
 import { captureError } from "./sentry";
 import {
   consumingRetryDelayMs,
@@ -370,7 +370,7 @@ export function createPgQueue(
         return true;
       }
       try {
-        await withOtelSpan(
+        await withReviewSpan(
           "selfhost.queue.job",
           { "job.type": message.type, "queue.backend": "postgres", "job.attempt": Number(job.attempts) + 1 },
           () => consume(message),

src/selfhost/sentry.ts

@@ -14,6 +14,9 @@ type SentryScope = {
 let Sentry: SentryNs | undefined;
 let active = false;
 let sentryEnvironment = "production";
+// The resolved tracing sample rate. Tracing stays a complete no-op (no spans started, no trace traffic) until this
+// is configured above 0 — distinct from error capture, which is on whenever the DSN is set. (#1734)
+let tracesSampleRate = 0;
 
 const SECRET_KEY =
   /(token|secret|key|password|passwd|authorization|auth|dsn|cookie|bearer|credential|private)/i;
@@ -104,6 +107,14 @@ export function resolveSentryRelease(
   return nonBlank(env.SENTRY_RELEASE) ?? nonBlank(env.GITTENSORY_VERSION);
 }
 
+/** Resolve the trace sample rate, clamped to [0, 1]. Defaults to 0 (tracing off) — a malformed value is treated as
+ *  off rather than full sampling, so a typo can never accidentally flood the tracer. (#1734) */
+export function resolveTracesSampleRate(env: NodeJS.ProcessEnv): number {
+  const parsed = Number(env.SENTRY_TRACES_SAMPLE_RATE ?? "0");
+  if (!Number.isFinite(parsed)) return 0;
+  return Math.min(1, Math.max(0, parsed));
+}
+
 /** beforeSend scrubber — redact anything token/secret-like before an event leaves the box (privacy boundary). */
 export function scrubEvent<T>(event: T): T {
   const redact = (obj: unknown, depth: number): void => {
@@ -135,11 +146,12 @@ export async function initSentry(env: NodeJS.ProcessEnv): Promise<boolean> {
   Sentry = await import("@sentry/node");
   const release = resolveSentryRelease(env);
   sentryEnvironment = nonBlank(env.SENTRY_ENVIRONMENT) ?? "production";
+  tracesSampleRate = resolveTracesSampleRate(env);
   Sentry.init({
     dsn: env.SENTRY_DSN,
     environment: sentryEnvironment,
     ...(release ? { release } : {}),
-    tracesSampleRate: Number(env.SENTRY_TRACES_SAMPLE_RATE ?? "0"),
+    tracesSampleRate,
     serverName: env.PUBLIC_API_ORIGIN,
     beforeSend: (e) => scrubEvent(e),
   });
@@ -186,6 +198,42 @@ export function captureReviewFailure(
   });
 }
 
+/** True only when error capture is active AND trace sampling is configured above 0. When false, every span helper
+ *  is a complete no-op — no span is started and no trace traffic is emitted (the #1734 "sampling off" guarantee). */
+export function sentryTracingEnabled(): boolean {
+  return active && Sentry !== undefined && tracesSampleRate > 0;
+}
+
+/** Project an attribute bag onto the safe, low-cardinality subset allowed on a span: drop secret-keyed keys and
+ *  null/undefined, keep finite numbers + booleans, and truncate strings — never a prompt/diff/token/body. */
+export function sentrySpanAttributes(
+  input: Record<string, unknown> | undefined,
+): Record<string, string | number | boolean> {
+  const out: Record<string, string | number | boolean> = {};
+  if (!input) return out;
+  for (const [key, value] of Object.entries(input)) {
+    if (SECRET_KEY.test(key) || value === null || value === undefined) continue;
+    if (typeof value === "string") out[key] = value.length > 160 ? `${value.slice(0, 157)}...` : value;
+    else if (typeof value === "number" && Number.isFinite(value)) out[key] = value;
+    else if (typeof value === "boolean") out[key] = value;
+  }
+  return out;
+}
+
+/** Run `fn` inside a Sentry span named `name`, tagged with the safe attributes. The span auto-closes and is marked
+ *  errored if `fn` throws (so slow/failed stages are filterable). A pure pass-through to `fn` when tracing is off. */
+export async function withSentrySpan<T>(
+  name: string,
+  attributes: Record<string, unknown> | undefined,
+  fn: () => T | Promise<T>,
+): Promise<T> {
+  if (!sentryTracingEnabled()) return fn();
+  return Sentry!.startSpan(
+    { name, op: name, attributes: sentrySpanAttributes(attributes) },
+    () => fn(),
+  );
+}
+
 // The structured-log fields worth indexing as Sentry tags — the dimensions operators filter + group by. Only
 // string|number values are tagged; everything else stays in the full "log" context.
 const SENTRY_LOG_TAG_KEYS = ["repo", "repository", "installationId", "installation_id", "pull", "pullNumber", "pr", "project", "kind", "deliveryId", "provider", "model", "effort", "timeoutMs", "trace_id", "span_id"] as const;
@@ -364,6 +412,7 @@ export function resetSentryForTest(): void {
   Sentry = undefined;
   active = false;
   sentryEnvironment = "production";
+  tracesSampleRate = 0;
 }
 
 interface StructuredLogConsole {

src/selfhost/sqlite-queue.ts

@@ -6,7 +6,7 @@
 import type { SqliteDriver } from "./d1-adapter";
 import { logAudit, extractPayloadType } from "./audit";
 import { incr } from "./metrics";
-import { withOtelSpan } from "./otel";
+import { withReviewSpan } from "./tracing";
 import { captureError } from "./sentry";
 import {
   consumingRetryDelayMs,
@@ -313,7 +313,7 @@ export function createSqliteQueue(
         return true;
       }
       try {
-        await withOtelSpan(
+        await withReviewSpan(
           "selfhost.queue.job",
           { "job.type": message.type, "queue.backend": "sqlite", "job.attempt": job.attempts + 1 },
           () => consume(message),

src/selfhost/tracing.ts

@@ -0,0 +1,16 @@
+// Shared review-pipeline span wrapper (#1734). Opens ONE boundary that feeds BOTH tracers — an OpenTelemetry span
+// and a Sentry span — so a stage is instrumented once and shows up in whichever backend is enabled. Kept in this
+// neutral module (rather than inside otel.ts or sentry.ts) so a caller wires the normal review boundary without
+// importing from, or coupling to, either tracer's module. Each side independently no-ops when its backend is off,
+// so this reduces to `fn()` when neither is configured.
+import { withOtelSpan } from "./otel";
+import { withSentrySpan } from "./sentry";
+
+export async function withReviewSpan<T>(
+  name: string,
+  attributes: Record<string, unknown> | undefined,
+  fn: () => T | Promise<T>,
+  options?: { parentTraceParent?: string | undefined },
+): Promise<T> {
+  return withOtelSpan(name, attributes, () => withSentrySpan(name, attributes, fn), options);
+}

test/unit/selfhost-sentry.test.ts

@@ -11,6 +11,8 @@ const mocks = vi.hoisted(() => {
     captureMessage: vi.fn(),
     captureCheckIn: vi.fn((checkIn: { checkInId?: string }) => checkIn.checkInId ?? "check-in-id"),
     flush: vi.fn().mockResolvedValue(true),
+    // Mirror @sentry/node's startSpan contract: invoke the callback inside the span and return its value.
+    startSpan: vi.fn(<T>(_opts: unknown, cb: () => T): T => cb()),
   };
 });
 const otelMocks = vi.hoisted(() => ({
@@ -23,6 +25,7 @@ vi.mock("@sentry/node", () => ({
   captureMessage: mocks.captureMessage,
   captureCheckIn: mocks.captureCheckIn,
   flush: mocks.flush,
+  startSpan: mocks.startSpan,
 }));
 vi.mock("../../src/selfhost/otel", () => ({
   currentOtelTraceIds: otelMocks.currentOtelTraceIds,
@@ -37,9 +40,13 @@ import {
   installStructuredLogForwarding,
   resolveSentryRelease,
   resolveSentryMonitorSlug,
+  resolveTracesSampleRate,
   scrubEvent,
   resetSentryForTest,
+  sentryTracingEnabled,
+  sentrySpanAttributes,
   withSentryMonitor,
+  withSentrySpan,
 } from "../../src/selfhost/sentry";
 
 beforeEach(() => {
@@ -698,3 +705,82 @@ describe("installStructuredLogForwarding — central console sink instrumentatio
     expect(base.error).toHaveBeenCalledTimes(2);
   });
 });
+
+const DSN = "https://k@o.ingest/1";
+const asEnv = (e: Record<string, string>) => e as unknown as NodeJS.ProcessEnv;
+
+describe("resolveTracesSampleRate — opt-in, clamped, safe default (#1734)", () => {
+  it("defaults to 0, parses a valid rate, clamps to [0,1], and treats a non-finite value as 0", () => {
+    expect(resolveTracesSampleRate(asEnv({}))).toBe(0);
+    expect(resolveTracesSampleRate(asEnv({ SENTRY_TRACES_SAMPLE_RATE: "0.25" }))).toBe(0.25);
+    expect(resolveTracesSampleRate(asEnv({ SENTRY_TRACES_SAMPLE_RATE: "5" }))).toBe(1);
+    expect(resolveTracesSampleRate(asEnv({ SENTRY_TRACES_SAMPLE_RATE: "-2" }))).toBe(0);
+    expect(resolveTracesSampleRate(asEnv({ SENTRY_TRACES_SAMPLE_RATE: "abc" }))).toBe(0);
+  });
+});
+
+describe("sentrySpanAttributes — safe, low-cardinality only", () => {
+  it("drops secret-keyed and null/undefined keys, keeps scalars, truncates long strings", () => {
+    const out = sentrySpanAttributes({
+      "ai.model": "gpt",
+      "job.attempt": 2,
+      ok: true,
+      apiKey: "shh",
+      token: "x",
+      missing: null,
+      undef: undefined,
+      nan: Number.NaN, // a non-finite number is dropped, never tagged
+      nested: { a: 1 }, // a non-scalar is dropped (no unbounded blobs on a span)
+      long: "z".repeat(200),
+    });
+    expect(out).toEqual({
+      "ai.model": "gpt",
+      "job.attempt": 2,
+      ok: true,
+      long: `${"z".repeat(157)}...`,
+    });
+  });
+
+  it("returns an empty object for undefined input", () => {
+    expect(sentrySpanAttributes(undefined)).toEqual({});
+  });
+});
+
+describe("tracing is a complete no-op unless sampling is configured > 0 (#1734)", () => {
+  it("with Sentry off, withSentrySpan runs fn but starts NO span and reports tracing disabled", async () => {
+    expect(sentryTracingEnabled()).toBe(false);
+    expect(await withSentrySpan("s", { a: 1 }, async () => "r")).toBe("r");
+    expect(mocks.startSpan).not.toHaveBeenCalled();
+  });
+
+  it("with the DSN set but sample rate 0 (default), tracing stays off and starts no span", async () => {
+    await initSentry(asEnv({ SENTRY_DSN: DSN })); // no SENTRY_TRACES_SAMPLE_RATE → 0
+    expect(sentryTracingEnabled()).toBe(false);
+    await withSentrySpan("s", undefined, async () => "r");
+    expect(mocks.startSpan).not.toHaveBeenCalled();
+  });
+});
+
+describe("tracing emits spans when sampling is enabled (#1734)", () => {
+  beforeEach(async () => {
+    await initSentry(asEnv({ SENTRY_DSN: DSN, SENTRY_TRACES_SAMPLE_RATE: "1" }));
+  });
+
+  it("starts a named span tagged with safe attributes and returns fn's value", async () => {
+    expect(sentryTracingEnabled()).toBe(true);
+    const result = await withSentrySpan("selfhost.ai.provider", { "ai.model": "gpt", apiKey: "shh" }, async () => 42);
+    expect(result).toBe(42);
+    expect(mocks.startSpan).toHaveBeenCalledTimes(1);
+    const [opts] = mocks.startSpan.mock.calls[0]!;
+    expect(opts).toMatchObject({ name: "selfhost.ai.provider", op: "selfhost.ai.provider" });
+    expect((opts as { attributes: Record<string, unknown> }).attributes).toEqual({ "ai.model": "gpt" }); // secret dropped
+  });
+
+  it("propagates an error thrown by fn (the caller's error is never swallowed by the span wrapper)", async () => {
+    await expect(
+      withSentrySpan("selfhost.queue.job", undefined, async () => {
+        throw new Error("boom");
+      }),
+    ).rejects.toThrow("boom");
+  });
+});

test/unit/selfhost-tracing.test.ts

@@ -0,0 +1,41 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+// withReviewSpan composes the two tracer wrappers; both are mocked here as passthroughs so we assert the
+// composition (one boundary → both tracers, option forwarding, value pass-through) without any real SDK.
+const otel = vi.hoisted(() => ({
+  withOtelSpan: vi.fn(
+    <T>(_name: string, _attrs: unknown, fn: () => T | Promise<T>, _options?: unknown): T | Promise<T> => fn(),
+  ),
+}));
+const sentry = vi.hoisted(() => ({
+  withSentrySpan: vi.fn(
+    <T>(_name: string, _attrs: unknown, fn: () => T | Promise<T>): T | Promise<T> => fn(),
+  ),
+}));
+vi.mock("../../src/selfhost/otel", () => ({ withOtelSpan: otel.withOtelSpan }));
+vi.mock("../../src/selfhost/sentry", () => ({ withSentrySpan: sentry.withSentrySpan }));
+
+import { withReviewSpan } from "../../src/selfhost/tracing";
+
+beforeEach(() => vi.clearAllMocks());
+
+describe("withReviewSpan — one boundary feeds both tracers (#1734)", () => {
+  it("runs fn through the OTEL span wrapping the Sentry span, and returns fn's value", async () => {
+    const result = await withReviewSpan("selfhost.queue.job", { "job.type": "github-webhook" }, async () => "ok");
+    expect(result).toBe("ok");
+    expect(otel.withOtelSpan).toHaveBeenCalledTimes(1);
+    expect(sentry.withSentrySpan).toHaveBeenCalledTimes(1);
+    // Same span name + attributes are handed to both tracers.
+    expect(otel.withOtelSpan.mock.calls[0]![0]).toBe("selfhost.queue.job");
+    expect(sentry.withSentrySpan.mock.calls[0]![0]).toBe("selfhost.queue.job");
+  });
+
+  it("forwards the parentTraceParent option to the OTEL span (cross-job trace continuity)", async () => {
+    await withReviewSpan("n", undefined, async () => 1, { parentTraceParent: "00-trace-span-01" });
+    expect(otel.withOtelSpan.mock.calls[0]![3]).toEqual({ parentTraceParent: "00-trace-span-01" });
+  });
+
+  it("reduces to fn() when both tracer wrappers no-op (neither backend configured)", async () => {
+    expect(await withReviewSpan("n", undefined, async () => 42)).toBe(42);
+  });
+});