From ec921e143b35fbaf82068a8442a9e45faa693a58 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Thu, 9 Jul 2026 15:31:58 -0700 Subject: [PATCH] feat(producer,cli): full telemetry visibility for DE parallel-router/inversion failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit render_error previously carried zero DE-cohort context — a hard failure while routed (worker crash, OOM, capture timeout from the fixed 3-worker pin overriding calibration) was indistinguishable from any other failure. The data existed (RenderCaptureObservability is mutated live and survives into job.errorDetails on the failure path) but was never projected into the render_error payload, which only ever drew de_* fields from perfSummary (success-only). - RenderCaptureObservability now also records dePreInversionWorkers / dePreRouterWorkers — the worker count calibration would have picked absent the experiment — so a resource-pressure failure can be correlated with the router overriding a lower calibrated count. - New capture-sourced de_* fields on RenderObservabilityTelemetryPayload, shared by trackRenderComplete and trackRenderError. Explicit perfSummary-sourced fields still win on render_complete (spread moved first in the event object) — this is purely a failure-path fallback. Co-Authored-By: Claude Sonnet 5 --- packages/cli/src/telemetry/events.test.ts | 44 +++++++++++++++++++ packages/cli/src/telemetry/events.ts | 23 +++++++++- .../src/telemetry/renderObservability.test.ts | 28 ++++++++++++ .../cli/src/telemetry/renderObservability.ts | 5 +++ .../src/services/render/observability.ts | 4 ++ .../src/services/renderOrchestrator.ts | 13 +++++- 6 files changed, 115 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/telemetry/events.test.ts b/packages/cli/src/telemetry/events.test.ts index 5801d8f9b..91bcc8b5b 100644 --- a/packages/cli/src/telemetry/events.test.ts +++ b/packages/cli/src/telemetry/events.test.ts @@ -51,6 +51,50 @@ describe("render telemetry events", () => { ); }); + it("carries the DE parallel-router/inversion cohort on render_error (hard failure, not just self-verify revert)", () => { + trackRenderError({ + fps: 30, + quality: "standard", + docker: false, + errorMessage: "worker crashed", + captureDeParallelRouter: "routed", + captureDePreRouterWorkers: 2, + captureWorkerCount: 3, + captureMemoryExhaustionDetected: true, + }); + + expect(trackEvent).toHaveBeenCalledWith( + "render_error", + expect.objectContaining({ + de_parallel_router: "routed", + de_pre_router_workers: 2, + capture_worker_count: 3, + capture_memory_exhaustion_detected: true, + }), + undefined, + ); + }); + + it("prefers the explicit perfSummary-sourced de_worker_inversion over the capture-observability fallback on render_complete", () => { + trackRenderComplete({ + durationMs: 1000, + fps: 30, + quality: "standard", + docker: false, + gpu: false, + deWorkerInversion: "inverted", + // Simulates a stale/divergent capture-observability value — the explicit + // perfSummary field above must win, not this one. + captureDeWorkerInversion: "reverted", + }); + + expect(trackEvent).toHaveBeenCalledWith( + "render_complete", + expect.objectContaining({ de_worker_inversion: "inverted" }), + undefined, + ); + }); + it("emits render_preflight_rejected with the low-cardinality issue kind", () => { trackRenderPreflightRejected({ kind: "aspect-mismatch" }); expect(trackEvent).toHaveBeenCalledWith("render_preflight_rejected", { diff --git a/packages/cli/src/telemetry/events.ts b/packages/cli/src/telemetry/events.ts index 89462246a..93b3d79f4 100644 --- a/packages/cli/src/telemetry/events.ts +++ b/packages/cli/src/telemetry/events.ts @@ -34,6 +34,18 @@ export interface RenderObservabilityTelemetryPayload { capturePlayerReadyTimeoutMs?: number; captureTransientRetries?: number; captureMemoryExhaustionDetected?: boolean; + // Mirror of the DE inversion/router state on `RenderCaptureObservability` — + // sourced from the live-mutated capture object rather than `perfSummary`, + // so a hard failure (crash, OOM, timeout) that never reaches perfSummary + // construction still reports which DE experiment cohort it was in. Mapped + // to the SAME `de_*` event keys `trackRenderComplete` sets explicitly from + // `perfSummary.drawElement`; the caller must spread this payload FIRST so + // the more authoritative perfSummary value wins when both are present. + captureDeWorkerInversion?: string; + captureDePreInversionWorkers?: number; + captureDeParallelRouter?: string; + captureDePreRouterWorkers?: number; + captureDeSelfVerifyFallback?: boolean; observabilityExtractVideoCount?: number; observabilityExtractedVideoCount?: number; observabilityExtractTotalFrames?: number; @@ -79,6 +91,11 @@ function renderObservabilityEventProperties(props: RenderObservabilityTelemetryP capture_player_ready_timeout_ms: props.capturePlayerReadyTimeoutMs, capture_transient_retries: props.captureTransientRetries, capture_memory_exhaustion_detected: props.captureMemoryExhaustionDetected, + de_worker_inversion: props.captureDeWorkerInversion, + de_pre_inversion_workers: props.captureDePreInversionWorkers, + de_parallel_router: props.captureDeParallelRouter, + de_pre_router_workers: props.captureDePreRouterWorkers, + de_self_verify_fallback: props.captureDeSelfVerifyFallback, observability_extract_video_count: props.observabilityExtractVideoCount, observability_extracted_video_count: props.observabilityExtractedVideoCount, observability_extract_total_frames: props.observabilityExtractTotalFrames, @@ -189,6 +206,11 @@ export function trackRenderComplete( trackEvent( "render_complete", { + // Spread first: explicit de_* keys below (sourced from the more + // authoritative perfSummary.drawElement, always present on this + // success path) must win over the observability-capture fallback + // this shares with trackRenderError's failure path. + ...renderObservabilityEventProperties(props), duration_ms: props.durationMs, fps: props.fps, quality: props.quality, @@ -252,7 +274,6 @@ export function trackRenderComplete( extract_phase3_ms: props.extractPhase3Ms, extract_cache_hits: props.extractCacheHits, extract_cache_misses: props.extractCacheMisses, - ...renderObservabilityEventProperties(props), }, props.distinctId, ); diff --git a/packages/cli/src/telemetry/renderObservability.test.ts b/packages/cli/src/telemetry/renderObservability.test.ts index 5e3c75b3b..d952ba7cb 100644 --- a/packages/cli/src/telemetry/renderObservability.test.ts +++ b/packages/cli/src/telemetry/renderObservability.test.ts @@ -38,3 +38,31 @@ describe("renderObservabilityTelemetryPayload — render-reliability counters", expect(payload.captureMemoryExhaustionDetected).toBeUndefined(); }); }); + +describe("renderObservabilityTelemetryPayload — DE inversion/router cohort (failure-path visibility)", () => { + it("maps the router cohort and its pre-router worker count", () => { + const payload = renderObservabilityTelemetryPayload( + makeSummary({ deParallelRouter: "routed", dePreRouterWorkers: 2 }), + ); + expect(payload.captureDeParallelRouter).toBe("routed"); + expect(payload.captureDePreRouterWorkers).toBe(2); + expect(payload.captureDeWorkerInversion).toBeUndefined(); + expect(payload.captureDePreInversionWorkers).toBeUndefined(); + }); + + it("maps the inversion cohort and its pre-inversion worker count", () => { + const payload = renderObservabilityTelemetryPayload( + makeSummary({ deWorkerInversion: "inverted", dePreInversionWorkers: 4 }), + ); + expect(payload.captureDeWorkerInversion).toBe("inverted"); + expect(payload.captureDePreInversionWorkers).toBe(4); + expect(payload.captureDeParallelRouter).toBeUndefined(); + }); + + it("carries deSelfVerifyFallback so a hard failure mid-verify is still visible", () => { + const payload = renderObservabilityTelemetryPayload( + makeSummary({ deParallelRouter: "routed", deSelfVerifyFallback: true }), + ); + expect(payload.captureDeSelfVerifyFallback).toBe(true); + }); +}); diff --git a/packages/cli/src/telemetry/renderObservability.ts b/packages/cli/src/telemetry/renderObservability.ts index 748d112cd..9f505d63a 100644 --- a/packages/cli/src/telemetry/renderObservability.ts +++ b/packages/cli/src/telemetry/renderObservability.ts @@ -40,6 +40,11 @@ export function renderObservabilityTelemetryPayload( capturePlayerReadyTimeoutMs: capture.playerReadyTimeoutMs, captureTransientRetries: capture.transientRetries, captureMemoryExhaustionDetected: capture.memoryExhaustionDetected, + captureDeWorkerInversion: capture.deWorkerInversion, + captureDePreInversionWorkers: capture.dePreInversionWorkers, + captureDeParallelRouter: capture.deParallelRouter, + captureDePreRouterWorkers: capture.dePreRouterWorkers, + captureDeSelfVerifyFallback: capture.deSelfVerifyFallback, observabilityExtractVideoCount: extraction?.videoCount, observabilityExtractedVideoCount: extraction?.extractedVideoCount, observabilityExtractTotalFrames: extraction?.totalFramesExtracted, diff --git a/packages/producer/src/services/render/observability.ts b/packages/producer/src/services/render/observability.ts index 8e486dc1a..a1fe82c83 100644 --- a/packages/producer/src/services/render/observability.ts +++ b/packages/producer/src/services/render/observability.ts @@ -44,8 +44,12 @@ export interface RenderCaptureObservability { deSelfVerifyFallback?: boolean; /** Auto-parallel inversion outcome: "inverted" (fired, held) | "reverted" (fired, self-verify retry rolled back). */ deWorkerInversion?: "inverted" | "reverted"; + /** Worker count the resolver would have used absent the inversion; undefined if it never fired. */ + dePreInversionWorkers?: number; /** DE parallel-router outcome: "routed" (fired, held) | "reverted" (fired, self-verify retry rolled back). */ deParallelRouter?: "routed" | "reverted"; + /** Worker count the resolver would have used absent the router; undefined if it never fired. */ + dePreRouterWorkers?: number; protocolTimeoutMs?: number; pageNavigationTimeoutMs?: number; playerReadyTimeoutMs?: number; diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index 614ebb75c..c03a5e848 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -1970,7 +1970,18 @@ export async function executeRenderJob( ); workerCount = 1; } - updateCaptureObservability({ workerCount, deWorkerInversion, deParallelRouter }); + updateCaptureObservability({ + workerCount, + deWorkerInversion, + deParallelRouter, + // Recorded here (not just in the success-path perfSummary) so a hard + // failure while routed/inverted still tells us what worker count the + // resolver would have used absent the experiment — the DE-router pin + // to 3 workers regardless of calibration is the leading suspect for + // any resource-pressure failure unique to this cohort. + dePreInversionWorkers: deWorkerInversion ? preRoutingWorkerCount : undefined, + dePreRouterWorkers: deParallelRouter ? preRoutingWorkerCount : undefined, + }); observability.checkpoint("worker_resolution", "resolved", { workerCount, deWorkerInversion: deWorkerInversion ?? "none",