diff --git a/packages/cli/src/telemetry/events.test.ts b/packages/cli/src/telemetry/events.test.ts index 9ee0b8c04..5dd6c6253 100644 --- a/packages/cli/src/telemetry/events.test.ts +++ b/packages/cli/src/telemetry/events.test.ts @@ -186,6 +186,59 @@ describe("render telemetry events", () => { expect(flush).toHaveBeenCalledTimes(2); }); + // The enforcement decision for the advisory heap budget reads these fleet + // props (see computeWorkerSizing) — a silent drop in the summary→event hop + // would invalidate that decision without anyone noticing. + it("carries every worker-sizing provenance prop on render_complete", () => { + trackRenderComplete({ + durationMs: 1000, + fps: 30, + quality: "high", + docker: false, + gpu: false, + workers: 6, + workersBoundBy: "max_workers", + workersCpuBased: 16, + workersMemoryBased: 8, + workersHeapBased: 4, + workersFrameBased: 24, + workersHeapLimitMb: 4096, + workersExceedHeapAdvisory: true, + }); + + expect(trackEvent).toHaveBeenCalledWith( + "render_complete", + expect.objectContaining({ + workers: 6, + workers_bound_by: "max_workers", + workers_cpu_based: 16, + workers_memory_based: 8, + workers_heap_based: 4, + workers_frame_based: 24, + workers_heap_limit_mb: 4096, + workers_exceed_heap_advisory: true, + }), + undefined, + ); + }); + + it("ties feedback to its report and recent renders via feedback_id + recent_render_ids", () => { + trackRenderFeedback({ + rating: 3, + comment: "hook scene blank", + feedbackId: "feedback-uuid", + recentRenderIds: ["render-a", "render-b"], + }); + + expect(trackEvent).toHaveBeenCalledWith( + "survey sent", + expect.objectContaining({ + feedback_id: "feedback-uuid", + recent_render_ids: "render-a,render-b", + }), + ); + }); + it("redacts paths and URL query strings from render error messages", () => { trackRenderError({ fps: 30, diff --git a/packages/engine/src/services/parallelCoordinator.ts b/packages/engine/src/services/parallelCoordinator.ts index b3608349d..113c2cf9f 100644 --- a/packages/engine/src/services/parallelCoordinator.ts +++ b/packages/engine/src/services/parallelCoordinator.ts @@ -124,6 +124,7 @@ const HEAP_RESERVED_MB = 1024; // default heap ⇒ >~500MB/worker + base. ponytail: advisory-only until the // workers_heap_* telemetry added alongside this constant validates the figure // — enforcing a guessed budget could silently cut worker counts fleet-wide. +// TODO(PRINFRA-341): decide enforcement after ~2 weeks of fleet soak. const HEAP_PER_WORKER_MB = 640; const MIN_WORKERS = 1; const MAX_WORKER_DIAGNOSTIC_LINES = 8; diff --git a/packages/producer/src/services/render/captureCost.heapAdvisory.test.ts b/packages/producer/src/services/render/captureCost.heapAdvisory.test.ts new file mode 100644 index 000000000..39d2fdce1 --- /dev/null +++ b/packages/producer/src/services/render/captureCost.heapAdvisory.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vitest"; +import type { WorkerSizing } from "@hyperframes/engine"; +import { buildHeapAdvisoryWarning } from "./captureCost.js"; + +function sizing(overrides: Partial): WorkerSizing { + return { + workers: 6, + boundBy: "max_workers", + cpuBasedWorkers: 16, + memoryBasedWorkers: 8, + frameBasedWorkers: 24, + effectiveMaxWorkers: 6, + heapBasedWorkers: 4, + heapLimitMb: 4096, + totalMemoryMb: 24576, + cpuCount: 18, + captureCostMultiplier: 1, + exceedsHeapAdvisory: true, + ...overrides, + }; +} + +describe("buildHeapAdvisoryWarning", () => { + it("names the chosen count, heap limit, safe count, and both remediation knobs", () => { + const message = buildHeapAdvisoryWarning(sizing({}), undefined); + expect(message).toContain("6 capture workers"); + expect(message).toContain("limit 4096MB"); + expect(message).toContain("~4"); + expect(message).toContain("NODE_OPTIONS=--max-old-space-size=8192"); + expect(message).toContain("--workers 4"); + }); + + it("stays silent for explicit --workers requests (operator's own call)", () => { + expect(buildHeapAdvisoryWarning(sizing({}), 6)).toBeUndefined(); + }); + + it("stays silent when the chosen count fits the heap budget", () => { + expect( + buildHeapAdvisoryWarning(sizing({ exceedsHeapAdvisory: false }), undefined), + ).toBeUndefined(); + }); +}); diff --git a/packages/producer/src/services/render/captureCost.ts b/packages/producer/src/services/render/captureCost.ts index ccf0f5b23..db00538d9 100644 --- a/packages/producer/src/services/render/captureCost.ts +++ b/packages/producer/src/services/render/captureCost.ts @@ -108,6 +108,36 @@ function combineCaptureCostEstimates( }; } +/** + * Advisory heap warning (field OOM, 0.7.66 on 24GB: auto-picked 6 workers + * blew Node's default ~4GB heap). Returns the warning message, or undefined + * when it should not fire: + * + * - Auto-sized renders only (`requestedWorkers === undefined`) — the field + * failure was auto sizing, and an explicit `--workers N` is the operator's + * own call. + * - Not enforced as a cap yet — the per-worker budget constant is derived + * from one field report; the `workers_heap_*` telemetry emitted with the + * sizing decides whether to enforce (see the TODO on HEAP_PER_WORKER_MB in + * @hyperframes/engine's parallelCoordinator). The message gives the + * operator the actionable knobs today. + * + * Pure so the message shape + firing condition are unit-testable with a + * synthetic `WorkerSizing` (the real one depends on the host's heap). + */ +export function buildHeapAdvisoryWarning( + sizing: WorkerSizing, + requestedWorkers: number | undefined, +): string | undefined { + if (requestedWorkers !== undefined || !sizing.exceedsHeapAdvisory) return undefined; + return ( + `[Render] ${sizing.workers} capture workers may exceed this process's V8 heap ` + + `(limit ${sizing.heapLimitMb}MB supports ~${sizing.heapBasedWorkers}). If the render ` + + `dies with "JavaScript heap out of memory", raise the heap ` + + `(NODE_OPTIONS=--max-old-space-size=8192) or pass --workers ${sizing.heapBasedWorkers}.` + ); +} + export function resolveRenderWorkerCount( totalFrames: number, requestedWorkers: number | undefined, @@ -152,24 +182,13 @@ export function resolveRenderWorkerCount( ...cfg, captureCostMultiplier: captureCost.multiplier, }); + // The heap advisory is deliberately NOT logged here: this function's + // "no unexpected warns" unit-test contract would become dependent on the + // test machine's actual V8 heap limit. The orchestrator's onSizing consumer + // emits it via buildHeapAdvisoryWarning. onSizing?.(sizing); const workerCount = sizing.workers; - // Advisory heap check (field OOM, 0.7.66 on 24GB: auto-picked 6 workers - // blew Node's default ~4GB heap). Not enforced yet — the budget constant is - // derived from one field report; the workers_heap_* telemetry emitted with - // this sizing decides whether to enforce. The warning gives the operator - // the actionable knobs today. - if (sizing.exceedsHeapAdvisory) { - log.warn( - `[Render] ${workerCount} capture workers may exceed this process's V8 heap ` + - `(limit ${sizing.heapLimitMb}MB supports ~${sizing.heapBasedWorkers}). If the render ` + - `dies with "JavaScript heap out of memory", raise the heap ` + - `(NODE_OPTIONS=--max-old-space-size=8192) or pass --workers ${sizing.heapBasedWorkers}.`, - { heapLimitMb: sizing.heapLimitMb, heapBasedWorkers: sizing.heapBasedWorkers }, - ); - } - if (requestedWorkers !== undefined || captureCost.multiplier <= 1) { return workerCount; } diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index ad5836d76..f0673fda9 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -125,6 +125,7 @@ import { resolveVideoCaptureBeyondViewport } from "./render/captureBeyondViewpor import { type CaptureCalibrationSample, type CaptureCostEstimate, + buildHeapAdvisoryWarning, resolveRenderWorkerCount, runCaptureCalibration, } from "./render/captureCost.js"; @@ -2487,6 +2488,13 @@ async function executeRenderPipeline(input: { captureCalibration?.estimate, (sizing) => { workerSizing = sizing; + const heapAdvisory = buildHeapAdvisoryWarning(sizing, job.config.workers); + if (heapAdvisory) { + log.warn(heapAdvisory, { + heapLimitMb: sizing.heapLimitMb, + heapBasedWorkers: sizing.heapBasedWorkers, + }); + } }, ); // DE priority inversion — see shouldPreferSingleWorkerDrawElement for the