Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions packages/cli/src/commands/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,38 @@ function getMemorySnapshot() {
};
}

/**
* Sizing + memory props for the failure path.
*
* `job.perfSummary` — where the success path reads these from — is assembled
* only after a render completes, so these come off the job fields the
* orchestrator's unconditional memory-sampler disposer writes
* (`recordJobFailureMetrics`). Extracted from `handleRenderError` to keep its
* branch count down.
*
* Prefers the sampled running peak to the teardown RSS snapshot: the snapshot
* reads whatever RSS happens to be at teardown and misses the mid-render spike
* the field exists to catch.
*/
function failureSizingTelemetry(job: RenderJob | undefined, requestedWorkers: number | undefined) {
const snapshot = getMemorySnapshot();
const base = {
...snapshot,
peakMemoryMb: job?.peakRssMb ?? snapshot.peakMemoryMb,
peakHeapUsedMb: job?.peakHeapUsedMb,
};
const sizing = job?.workerSizing;
if (!sizing) return { ...base, workers: requestedWorkers };
return {
...base,
workers: requestedWorkers ?? sizing.workers,
workersBoundBy: sizing.boundBy,
workersHeapBased: sizing.heapBasedWorkers,
workersHeapLimitMb: sizing.heapLimitMb,
workersExceedHeapAdvisory: sizing.exceedsHeapAdvisory,
};
}

function metaString(meta: Record<string, unknown> | undefined, key: string): string | undefined {
const value = meta?.[key];
return typeof value === "string" ? value : undefined;
Expand Down Expand Up @@ -1439,14 +1471,13 @@ function handleRenderError(
fps: fpsToNumber(options.fps),
quality: options.quality,
docker,
workers: options.workers,
gpu: options.gpu,
authoringSkill: options.authoringSkill,
elapsedMs: Date.now() - startTime,
errorMessage: message,
failedStage,
...renderJobObservabilityTelemetryPayload(job),
...getMemorySnapshot(),
...failureSizingTelemetry(job, options.workers),
});
// Failed renders join the recent-renders ring too — a bug report filed via
// `hyperframes feedback` is MOST likely to be about a failed render.
Expand Down Expand Up @@ -1599,6 +1630,10 @@ function trackRenderMetrics(
extractCacheMisses: extract?.cacheMisses,
...renderJobObservabilityTelemetryPayload(job),
...getMemorySnapshot(),
// Same reason as the failure path: prefer the sampled running peak to the
// teardown snapshot. Falls back when perfSummary is absent (e.g. Docker).
peakMemoryMb: perf?.peakRssMb ?? getMemorySnapshot().peakMemoryMb,
peakHeapUsedMb: perf?.peakHeapUsedMb,
});
}

Expand Down
69 changes: 69 additions & 0 deletions packages/cli/src/telemetry/events.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,38 @@ describe("render telemetry events", () => {
);
});

// PRINFRA-341 could not be decided because these props were render_complete
// only: 0 of 317k render_error events carried them, so advisory-true renders
// could never be correlated with failures. Dropping them again re-opens that.
it("carries worker-sizing provenance and sampled peaks on render_error too", () => {
trackRenderError({
fps: 30,
quality: "high",
docker: false,
workers: 6,
workersBoundBy: "heap",
workersHeapBased: 4,
workersHeapLimitMb: 4144,
workersExceedHeapAdvisory: true,
peakMemoryMb: 1633,
peakHeapUsedMb: 900,
});

expect(trackEvent).toHaveBeenCalledWith(
"render_error",
expect.objectContaining({
workers: 6,
workers_bound_by: "heap",
workers_heap_based: 4,
workers_heap_limit_mb: 4144,
workers_exceed_heap_advisory: true,
peak_memory_mb: 1633,
peak_heap_used_mb: 900,
}),
undefined,
);
});

it("ties feedback to its report and recent renders via feedback_id + recent_render_ids", () => {
trackRenderFeedback({
rating: 3,
Expand Down Expand Up @@ -864,3 +896,40 @@ describe("power-state sampling respects the telemetry opt-out", () => {
expect(getPowerState).not.toHaveBeenCalled();
});
});

// Appended at end of file deliberately: inserting mid-file shifts the line
// numbers of pre-existing clone groups, which makes fallow re-report them as
// new findings on an unrelated change.
describe("render_error worker-sizing provenance", () => {
// PRINFRA-341 could not be decided because these props were render_complete
// only: 0 of 317k render_error events carried them, so advisory-true renders
// could never be correlated with failures. Dropping them reopens that.
it("carries sizing and sampled peaks on the failure path", () => {
trackRenderError({
fps: 30,
quality: "high",
docker: false,
workers: 6,
workersBoundBy: "heap",
workersHeapBased: 4,
workersHeapLimitMb: 4144,
workersExceedHeapAdvisory: true,
peakMemoryMb: 1633,
peakHeapUsedMb: 900,
});
const props = trackEvent.mock.calls.at(-1)?.[1] as Record<string, unknown>;
expect(props.workers_bound_by).toBe("heap");
expect(props.workers_heap_based).toBe(4);
expect(props.workers_heap_limit_mb).toBe(4144);
expect(props.workers_exceed_heap_advisory).toBe(true);
expect(props.peak_memory_mb).toBe(1633);
expect(props.peak_heap_used_mb).toBe(900);
});

it("omits them when the render failed before sizing was computed", () => {
trackRenderError({ fps: 30, quality: "draft", docker: false });
const props = trackEvent.mock.calls.at(-1)?.[1] as Record<string, unknown>;
expect(props.workers_bound_by).toBeUndefined();
expect(props.workers_heap_limit_mb).toBeUndefined();
});
});
19 changes: 19 additions & 0 deletions packages/cli/src/telemetry/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,8 @@ export function trackRenderComplete(
capturePeakMs?: number;
// Resource usage
peakMemoryMb?: number;
/** Sampled running peak of V8 heapUsed (complement to RSS). */
peakHeapUsedMb?: number;
memoryFreeMb?: number;
tmpPeakBytes?: number;
// Per-stage timings (subset of RenderPerfSummary.stages)
Expand Down Expand Up @@ -413,6 +415,7 @@ export function trackRenderComplete(
video_count: props.videoCount,
capture_peak_ms: props.capturePeakMs,
peak_memory_mb: props.peakMemoryMb,
peak_heap_used_mb: props.peakHeapUsedMb,
memory_free_mb: props.memoryFreeMb,
tmp_peak_bytes: props.tmpPeakBytes,
stage_compile_ms: props.stageCompileMs,
Expand Down Expand Up @@ -460,7 +463,18 @@ export function trackRenderError(
errorMessage?: string;
elapsedMs?: number;
peakMemoryMb?: number;
peakHeapUsedMb?: number;
memoryFreeMb?: number;
/**
* Worker sizing on the failure path. Previously `render_complete`-only,
* which left PRINFRA-341's central question unanswerable: 0 of 317k
* `render_error` events carried these, so advisory-true renders could
* never be correlated with failures.
*/
workersBoundBy?: string;
workersHeapBased?: number;
workersHeapLimitMb?: number;
workersExceedHeapAdvisory?: boolean;
// Attribute this event to a specific user (e.g. the browser user who
// triggered a studio render); defaults to the install anonymousId.
distinctId?: string;
Expand All @@ -480,7 +494,12 @@ export function trackRenderError(
error_message: props.errorMessage ? redactTelemetryMessage(props.errorMessage) : undefined,
elapsed_ms: props.elapsedMs,
peak_memory_mb: props.peakMemoryMb,
peak_heap_used_mb: props.peakHeapUsedMb,
memory_free_mb: props.memoryFreeMb,
workers_bound_by: props.workersBoundBy,
workers_heap_based: props.workersHeapBased,
workers_heap_limit_mb: props.workersHeapLimitMb,
workers_exceed_heap_advisory: props.workersExceedHeapAdvisory,
...powerStateFields(),
// gpu_renderer arrives via renderObservabilityEventProperties below:
// on the failure path perfSummary is never built, so live capture
Expand Down
38 changes: 37 additions & 1 deletion packages/producer/src/services/render/shared.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it } from "bun:test";
import { resolveBrowserMediaEnd } from "./shared.js";
import { recordJobFailureMetrics, resolveBrowserMediaEnd } from "./shared.js";

describe("resolveBrowserMediaEnd", () => {
it("prefers a runtime duration over a stale compiler-clamped end", () => {
Expand All @@ -15,3 +15,39 @@ describe("resolveBrowserMediaEnd", () => {
expect(resolveBrowserMediaEnd(0, 5.04, 0)).toBe(5.04);
});
});

describe("recordJobFailureMetrics", () => {
const sampler = {
peakRssBytes: () => 300 * 1024 * 1024,
peakHeapUsedBytes: () => 120 * 1024 * 1024,
};

// The whole point: perfSummary is success-only, so a thrown render must
// still leave sizing + peaks on the job for render_error to read.
it("copies sampled peaks and sizing onto the job", () => {
const job: { peakRssMb?: number; peakHeapUsedMb?: number; workerSizing?: { workers: number } } =
{};
recordJobFailureMetrics(job, sampler, { workers: 6 });
expect(job.peakRssMb).toBe(300);
expect(job.peakHeapUsedMb).toBe(120);
expect(job.workerSizing).toEqual({ workers: 6 });
});

it("records peaks even when sizing was never computed (failure before capture)", () => {
const job: { peakRssMb?: number; peakHeapUsedMb?: number; workerSizing?: { workers: number } } =
{};
recordJobFailureMetrics(job, sampler, undefined);
expect(job.peakRssMb).toBe(300);
expect(job.workerSizing).toBeUndefined();
});

it("keeps an already-recorded sizing rather than blanking it", () => {
const job: { peakRssMb?: number; peakHeapUsedMb?: number; workerSizing?: { workers: number } } =
{
workerSizing: { workers: 4 },
};
recordJobFailureMetrics(job, null, undefined);
expect(job.workerSizing).toEqual({ workers: 4 });
expect(job.peakRssMb).toBeUndefined();
});
});
32 changes: 32 additions & 0 deletions packages/producer/src/services/render/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,38 @@ export interface MemorySampler {
stop: () => void;
}

/**
* Copy sizing + sampled peak memory onto the job so they survive a failed
* render.
*
* `job.perfSummary` is assembled only after a render succeeds, so a throw
* mid-capture previously reached `render_error` with no sizing or memory
* context — 0 of 317k fleet failures carried `workers_heap_*`, which is why
* PRINFRA-341's "do advisory-true renders OOM?" question was unanswerable.
* Called from the unconditional memory-sampler disposer, so it runs on both
* the success and failure paths.
*
* ponytail: structurally typed rather than importing RenderJob/WorkerSizing —
* renderOrchestrator already imports this module, so a nominal import here
* would be a cycle.
*
* Does NOT make fatal V8 OOMs observable: `FATAL ERROR: Reached heap limit`
* aborts the process before any event is sent. This covers failures that
* reach an error handler at all.
*/
export function recordJobFailureMetrics<TSizing>(
job: { peakRssMb?: number; peakHeapUsedMb?: number; workerSizing?: TSizing },
sampler: Pick<MemorySampler, "peakRssBytes" | "peakHeapUsedBytes"> | null,
sizing: TSizing | undefined,
): void {
if (sampler) {
job.peakRssMb = Math.round(sampler.peakRssBytes() / (1024 * 1024));
job.peakHeapUsedMb = Math.round(sampler.peakHeapUsedBytes() / (1024 * 1024));
}
// Leave a previously-recorded sizing in place rather than blanking it.
if (sizing !== undefined) job.workerSizing = sizing;
}

export function createMemorySampler(intervalMs: number = 250): MemorySampler {
let peakRss = 0;
let peakHeap = 0;
Expand Down
26 changes: 25 additions & 1 deletion packages/producer/src/services/renderOrchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,12 @@ import {
outputSupportsPageSideShaderCompositing,
type RenderOutputFormat,
} from "./render/renderFormat.js";
import { createMemorySampler, type MemorySampler, updateJobStatus } from "./render/shared.js";
import {
createMemorySampler,
type MemorySampler,
recordJobFailureMetrics,
updateJobStatus,
} from "./render/shared.js";
import { buildRenderErrorDetails } from "./render/cleanup.js";
import { publishRenderFailure } from "./render/renderEventPublisher.js";
import { EncoderInterruptedError } from "./render/encoderInterruption.js";
Expand Down Expand Up @@ -625,6 +630,22 @@ export interface RenderJob {
totalFrames?: number;
framesRendered?: number;
perfSummary?: RenderPerfSummary;
/**
* Worker sizing + peak memory, recorded as soon as they are known rather
* than assembled at the end like {@link perfSummary}.
*
* `perfSummary` is only assigned on the success path, so a render that dies
* mid-capture carried no sizing or memory context into `render_error` — the
* gap that made PRINFRA-341's "do advisory-true renders OOM?" question
* unanswerable across 317k fleet failures. These are written by the
* unconditional memory-sampler disposer, so they survive a throw.
*
* NOT a fix for fatal V8 OOMs: `FATAL ERROR: Reached heap limit` aborts the
* process, so no telemetry is sent at all. This covers recoverable failures.
*/
workerSizing?: WorkerSizing;
peakRssMb?: number;
peakHeapUsedMb?: number;
failedStage?: string;
errorDetails?: {
message: string;
Expand Down Expand Up @@ -2250,6 +2271,9 @@ async function executeRenderPipeline(input: {
await closeCaptureSession(session);
});
execution.defer("stop memory sampler", () => {
// Runs on every exit including a throw, unlike the perfSummary assembly
// at the end of the happy path — see recordJobFailureMetrics.
recordJobFailureMetrics(job, memSampler, workerSizing);
memSampler?.stop();
memSampler = null;
});
Expand Down
Loading