From 5785282f5bb5672d5be686c200390559c8dc3e08 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 31 Jul 2026 06:00:47 -0700 Subject: [PATCH] feat(observability): name AI traces in PostHog with a $ai_trace envelope 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, where an $ai_span_name set 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. Emit the envelope from withReviewPipelineSpan, which already wraps a whole review and carries the repo, PR and the ambient OTel trace id the generations group by. It runs INSIDE the OTel span, because that id only exists once the span is open. Two guards, both load-bearing: - Outermost only. withReviewPipelineSpan is called from several sites that can nest, and PostHog expects one $ai_trace per trace. A depth counter means inner calls contribute nothing and the surviving name is the whole operation rather than whichever leaf finished last. - Only when generations exist. Not every pipeline span wraps an AI call -- the gate does not. Emitting unconditionally would manufacture trace rows with nothing under them, which reads worse than an unnamed trace. A pass-through when PostHog is off or no ambient trace exists, so it never changes what the wrapped work does or what it throws. Closes #10221 --- .../content/docs/self-hosting-operations.mdx | 5 + src/selfhost/posthog.ts | 101 +++++++++++++++ src/selfhost/review-tracing.ts | 9 +- ...ocs-selfhost-posthog-observability.test.ts | 7 +- test/unit/selfhost-posthog.test.ts | 118 ++++++++++++++++++ 5 files changed, 238 insertions(+), 2 deletions(-) diff --git a/apps/loopover-ui/content/docs/self-hosting-operations.mdx b/apps/loopover-ui/content/docs/self-hosting-operations.mdx index 1ef889dfbb..aa17047d2c 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 6bc81dee34..a21330f71f 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 aff6d15b0c..3226895e7b 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 7bd4d31aa9..d6a03653e9 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 459039ccad..186aff0ff3 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();