diff --git a/apps/loopover-ui/content/docs/self-hosting-operations.mdx b/apps/loopover-ui/content/docs/self-hosting-operations.mdx index 1ef889dfb..aa17047d2 100644 --- a/apps/loopover-ui/content/docs/self-hosting-operations.mdx +++ b/apps/loopover-ui/content/docs/self-hosting-operations.mdx @@ -683,6 +683,11 @@ three standalone bindings (`AI_EMBED`, `AI_VISION`, `AI_ADVISORY`). description: "$ai_model is always the model the provider actually resolved — on the failure path too. The core passes a Workers-AI model id that every self-host provider discards, so it is resolved to the real one (or -default) before capture and never reported verbatim.", }, + { + title: "Trace naming", + description: + "$ai_trace names the whole review in PostHog's Traces view. Emitted once per trace, by the outermost pipeline span, and only when at least one AI call actually ran under it — a pipeline span with no generations is never given a trace row.", + }, { title: "Degraded requests", description: diff --git a/src/selfhost/posthog.ts b/src/selfhost/posthog.ts index 6bc81dee3..a21330f71 100644 --- a/src/selfhost/posthog.ts +++ b/src/selfhost/posthog.ts @@ -507,9 +507,108 @@ function repoGroup(operational: Record): { groups?: { repo: str return typeof operational.repo === "string" ? { groups: { repo: operational.repo } } : {}; } +/** #10221: PostHog's Traces view takes a trace's NAME from a trace-level `$ai_trace` event, not from any + * property on the generations underneath it (confirmed upstream in PostHog/posthog#33179 -- an + * `$ai_span_name` on a generation does not populate that column). This project emitted only generations, so + * every trace read as an anonymous id with no way to tell a gate review from an embedding batch. + * + * Per in-flight trace: how deeply {@link withPostHogAiTrace} is nested, and whether any generation actually + * landed underneath. Both matter -- see that function for why. */ +const aiTraceState = new Map(); + +/** Note that a generation landed under `traceId`, so its enclosing pipeline span knows the trace is worth + * naming. A generation captured outside any pipeline span has no entry and is simply ignored. */ +function markAiTraceGeneration(traceId: unknown): void { + if (typeof traceId !== "string") return; + const state = aiTraceState.get(traceId); + if (state) state.generations += 1; +} + +export const POSTHOG_AI_TRACE_EVENT = "$ai_trace"; + +/** + * Name the AI trace a pipeline span represents (#10221), emitting one `$ai_trace` as the OUTERMOST span for + * that trace completes. + * + * Two conditions guard the emit, and both are load-bearing: + * + * - **Outermost only.** `withReviewPipelineSpan` is called from several sites that can nest, and PostHog + * expects one `$ai_trace` per trace id. A depth counter means the inner calls contribute nothing and the + * name that survives is the outermost one -- the whole operation, not whichever leaf happened to finish. + * - **Only when generations exist.** Not every pipeline span wraps an AI call (the gate, for one). Emitting + * unconditionally would manufacture trace rows with zero generations under them, which is a worse reading + * than an unnamed trace. + * + * A no-op when PostHog is off or there is no ambient OTel trace to name -- in both cases the callback runs + * untouched, so this never changes what the wrapped work does or what it throws. + */ +export async function withPostHogAiTrace( + name: string, + context: Record | undefined, + fn: () => T | Promise, +): Promise { + const traceId = currentOtelTraceIds()?.trace_id; + // Pin the client that was active when the span opened, rather than re-reading the module binding in the + // `finally` below -- it also lets the emit helper stay free of a second, unreachable off-switch check. + const target = client; + if (!active || !target || !traceId) return await fn(); + const state = aiTraceState.get(traceId) ?? { depth: 0, generations: 0 }; + if (state.depth === 0) aiTraceState.set(traceId, state); + state.depth += 1; + const startedAtMs = Date.now(); + let failure: unknown; + try { + return await fn(); + } catch (error) { + failure = error; + throw error; + } finally { + state.depth -= 1; + if (state.depth === 0) { + aiTraceState.delete(traceId); + if (state.generations > 0) captureAiTraceEnvelope(target, name, context, traceId, Date.now() - startedAtMs, failure); + } + } +} + +/** Emit the trace-level envelope. Separate from {@link withPostHogAiTrace} only so the bookkeeping above + * reads as bookkeeping. */ +function captureAiTraceEnvelope( + target: PostHogClient, + name: string, + context: Record | undefined, + traceId: string, + latencyMs: number, + failure: unknown, +): void { + const operational = operationalProperties(context); + const properties: Record = { + ...operational, + // The trace id is the one the generations already carry -- taken from the SAME ambient OTel trace, so the + // envelope and its children can never disagree about which trace they belong to. + $ai_trace_id: traceId, + $ai_span_name: name, + $ai_latency: latencyMs / 1000, + $ai_is_error: failure !== undefined, + environment: posthogEnvironment, + }; + if (failure !== undefined) { + const error = failure instanceof Error ? failure : new Error(String(failure)); + properties.$ai_error = error.message.slice(0, 500); + } + target.capture({ + distinctId: POSTHOG_DISTINCT_ID, + event: POSTHOG_AI_TRACE_EVENT, + properties, + ...repoGroup(operational), + }); +} + export function capturePostHogAiGeneration(event: PostHogAiGenerationEvent): void { if (!active || !client) return; const operational = operationalProperties(event.context); + // #10221: tells the enclosing pipeline span this trace has real AI work under it and is worth naming. + markAiTraceGeneration(operational.trace_id); const properties: Record = { ...operational, ...aiTraceProperties(operational), @@ -677,5 +776,7 @@ export function resetPostHogForTest(): void { centralKeyAnonSecret = undefined; aiContentCapture = false; aiContentMaxChars = DEFAULT_AI_CONTENT_MAX_CHARS; + // #10221: in-flight trace bookkeeping, so one test's pipeline span cannot leak into the next. + aiTraceState.clear(); resetRedactionScrubForTest(); } diff --git a/src/selfhost/review-tracing.ts b/src/selfhost/review-tracing.ts index aff6d15b0..3226895e7 100644 --- a/src/selfhost/review-tracing.ts +++ b/src/selfhost/review-tracing.ts @@ -1,5 +1,6 @@ import { sha256Hex } from "../utils/crypto"; import { setCurrentOtelSpanAttributes, withOtelSpan } from "./otel"; +import { withPostHogAiTrace } from "./posthog"; const INSTALLATION_HASH_SEED = "github-installation:"; @@ -54,7 +55,13 @@ export async function withReviewPipelineSpan( input: ReviewTraceInput, fn: () => T | Promise, ): Promise { - return withOtelSpan(name, await reviewTraceAttributes(input), fn); + // #10221: withPostHogAiTrace runs INSIDE the OTel span, because the trace id it names the trace by is the + // ambient one this span establishes -- the same id capturePostHogAiGeneration stamps on every generation + // underneath it. Outside the span there is nothing yet to name. It is a pass-through whenever PostHog is + // off or the trace turns out to hold no AI calls, so a non-AI pipeline span is unaffected. + return withOtelSpan(name, await reviewTraceAttributes(input), () => + withPostHogAiTrace(name, { repo: input.repoFullName, pullNumber: input.pullNumber }, fn), + ); } export async function setReviewPipelineSpanOutcome( diff --git a/test/unit/docs-selfhost-posthog-observability.test.ts b/test/unit/docs-selfhost-posthog-observability.test.ts index 7bd4d31aa..d6a03653e 100644 --- a/test/unit/docs-selfhost-posthog-observability.test.ts +++ b/test/unit/docs-selfhost-posthog-observability.test.ts @@ -1,7 +1,7 @@ import { readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; -import { POSTHOG_AI_DEGRADED_EVENT, POSTHOG_MONITOR_HEARTBEAT_EVENT } from "../../src/selfhost/posthog"; +import { POSTHOG_AI_DEGRADED_EVENT, POSTHOG_AI_TRACE_EVENT, POSTHOG_MONITOR_HEARTBEAT_EVENT } from "../../src/selfhost/posthog"; // Drift guard (#8287, #1468 -- 2026-07-25 Sentry removal): self-host PostHog docs must stay aligned with the // exported monitor-heartbeat event name. Sentry's own docs test (docs-selfhost-sentry-observability.test.ts) @@ -49,6 +49,11 @@ describe("self-host PostHog observability docs (#8287)", () => { expect(operations).toContain("private source code leaving your infrastructure"); }); + it("documents the trace envelope with its real exported event name (#10221)", () => { + expect(operations).toContain(POSTHOG_AI_TRACE_EVENT); + expect(operations).toContain("Trace naming"); + }); + it("documents the cron-monitor heartbeat replacement with the real exported event name", () => { expect(operations).toContain("Cron Monitors"); expect(operations).toContain(POSTHOG_MONITOR_HEARTBEAT_EVENT); diff --git a/test/unit/selfhost-posthog.test.ts b/test/unit/selfhost-posthog.test.ts index 459039cca..186aff0ff 100644 --- a/test/unit/selfhost-posthog.test.ts +++ b/test/unit/selfhost-posthog.test.ts @@ -35,7 +35,9 @@ import { capturePostHogAiMetric, POSTHOG_AI_DEGRADED_EVENT, POSTHOG_AI_METRIC_EVENT, + POSTHOG_AI_TRACE_EVENT, POSTHOG_MONITOR_HEARTBEAT_EVENT, + withPostHogAiTrace, resetPostHogForTest, resolvePostHogRelease, scrubPostHogEvent, @@ -1019,6 +1021,122 @@ describe("capturePostHogAiMetric (#10226 — review quality, joined to the AI tr }); }); +describe("withPostHogAiTrace (#10221 — naming the trace PostHog's Traces view shows)", () => { + const GENERATION = { provider: "claude-code", model: "claude-sonnet-5", requestKind: "review" as const, latencyMs: 1500, isError: false }; + const traceEvents = (): Array<{ properties: Record; groups?: unknown }> => + mocks.capture.mock.calls.map((call) => call[0]).filter((call) => call.event === POSTHOG_AI_TRACE_EVENT); + + it("is a transparent pass-through when PostHog is unconfigured", async () => { + await expect(withPostHogAiTrace("review.gate", undefined, async () => "result")).resolves.toBe("result"); + expect(mocks.capture).not.toHaveBeenCalled(); + }); + + it("is a pass-through when there is no ambient OTel trace to name", async () => { + otelMocks.currentOtelTraceIds.mockReturnValue(undefined); + await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); + await expect(withPostHogAiTrace("review.gate", undefined, async () => "result")).resolves.toBe("result"); + expect(traceEvents()).toHaveLength(0); + }); + + it("names the trace once a generation has landed under it", async () => { + otelMocks.currentOtelTraceIds.mockReturnValue({ trace_id: "review-trace-1", span_id: "span-1" }); + await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); + await withPostHogAiTrace("review.pipeline", { repo: "owner/repo", pullNumber: 7 }, async () => { + capturePostHogAiGeneration({ ...GENERATION, context: { repo: "owner/repo", pullNumber: 7 } }); + }); + const [envelope] = traceEvents(); + // The envelope must claim the SAME trace id the generation carries, or the two never join up. + expect(envelope?.properties.$ai_trace_id).toBe("review-trace-1"); + expect(envelope?.properties.$ai_span_name).toBe("review.pipeline"); + expect(envelope?.properties.$ai_is_error).toBe(false); + expect(envelope?.properties.repo).toBe("owner/repo"); + expect(envelope?.groups).toEqual({ repo: "owner/repo" }); + }); + + it("does NOT name a pipeline span that ran no AI calls at all", async () => { + otelMocks.currentOtelTraceIds.mockReturnValue({ trace_id: "gate-trace", span_id: "span-1" }); + await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); + // The gate is a real pipeline span with no generation under it. Naming it would manufacture a trace row + // with nothing in it, which reads worse than an unnamed trace. + await withPostHogAiTrace("review.gate", { repo: "owner/repo" }, async () => "no ai here"); + expect(traceEvents()).toHaveLength(0); + }); + + it("emits exactly ONE envelope for nested spans, named by the OUTERMOST one", async () => { + otelMocks.currentOtelTraceIds.mockReturnValue({ trace_id: "nested-trace", span_id: "span-1" }); + await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); + await withPostHogAiTrace("review.outer", { repo: "owner/repo" }, async () => { + await withPostHogAiTrace("review.inner", { repo: "owner/repo" }, async () => { + capturePostHogAiGeneration({ ...GENERATION, context: { repo: "owner/repo" } }); + }); + }); + const envelopes = traceEvents(); + expect(envelopes).toHaveLength(1); + // The whole operation, not whichever leaf happened to finish. + expect(envelopes[0]?.properties.$ai_span_name).toBe("review.outer"); + }); + + it("marks the trace errored and rethrows when the wrapped work throws", async () => { + otelMocks.currentOtelTraceIds.mockReturnValue({ trace_id: "failing-trace", span_id: "span-1" }); + await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); + await expect( + withPostHogAiTrace("review.pipeline", { repo: "owner/repo" }, async () => { + capturePostHogAiGeneration({ ...GENERATION, context: { repo: "owner/repo" } }); + throw new Error("reviewer exploded"); + }), + ).rejects.toThrow("reviewer exploded"); + const [envelope] = traceEvents(); + expect(envelope?.properties.$ai_is_error).toBe(true); + expect(envelope?.properties.$ai_error).toBe("reviewer exploded"); + }); + + it("handles a non-Error thrown value, and bounds the recorded message", async () => { + otelMocks.currentOtelTraceIds.mockReturnValue({ trace_id: "string-throw-trace", span_id: "span-1" }); + await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); + await expect( + withPostHogAiTrace("review.pipeline", undefined, async () => { + capturePostHogAiGeneration(GENERATION); + throw "a string failure"; + }), + ).rejects.toBe("a string failure"); + expect(traceEvents()[0]?.properties.$ai_error).toBe("a string failure"); + + mocks.capture.mockClear(); + await expect( + withPostHogAiTrace("review.pipeline", undefined, async () => { + capturePostHogAiGeneration(GENERATION); + throw new Error("y".repeat(600)); + }), + ).rejects.toThrow(); + expect(traceEvents()[0]?.properties.$ai_error).toHaveLength(500); + }); + + it("omits the repo group when the span has no repo, and still names the trace", async () => { + otelMocks.currentOtelTraceIds.mockReturnValue({ trace_id: "no-repo-trace", span_id: "span-1" }); + await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); + await withPostHogAiTrace("ai.advisory", undefined, async () => { + capturePostHogAiGeneration(GENERATION); + }); + const [envelope] = traceEvents(); + expect(envelope?.properties.$ai_span_name).toBe("ai.advisory"); + expect(envelope && "groups" in envelope).toBe(false); + }); + + it("a second, later span over the SAME trace id starts from a clean slate", async () => { + otelMocks.currentOtelTraceIds.mockReturnValue({ trace_id: "reused-trace", span_id: "span-1" }); + await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); + await withPostHogAiTrace("first", undefined, async () => { + capturePostHogAiGeneration(GENERATION); + }); + // The bookkeeping entry is deleted on completion, so the second span must not inherit the first's + // generation count and name an empty trace. + await withPostHogAiTrace("second", undefined, async () => "no ai here"); + const envelopes = traceEvents(); + expect(envelopes).toHaveLength(1); + expect(envelopes[0]?.properties.$ai_span_name).toBe("first"); + }); +}); + describe("flushPostHog / shutdownPostHog", () => { it("flushPostHog is a no-op when unconfigured", async () => { await flushPostHog();