Skip to content

Commit bf4041c

Browse files
1stvampTrigger.dev RepoOps
authored andcommitted
fix(webapp): log why a template prebuild was skipped
Log why deploy finalisation skipped the template prebuild, with the eligibility inputs it used, and count skips by reason and eligibility. Mono-RevId: 8cc37b007e0ab7e74c3075ab21628597c5f729ab
1 parent bd2a14e commit bf4041c

2 files changed

Lines changed: 108 additions & 22 deletions

File tree

‎apps/webapp/app/v3/services/templatePrebuild.server.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ import {
1010
type TemplateBuilderConfig,
1111
TemplatePrebuildService,
1212
templateCreateRequest,
13+
templatePrebuildTelemetry,
1314
validTemplateKey,
1415
} from "./templatePrebuild.server";
16+
import { createInMemoryMetrics } from "../../../test/utils/tracing";
1517

1618
vi.setConfig({ testTimeout: 60_000 });
1719

@@ -318,6 +320,36 @@ describe("prebuild", () => {
318320
}
319321
});
320322

323+
postgresTest("counts a skip by its reason", async ({ prisma }) => {
324+
const metrics = createInMemoryMetrics();
325+
try {
326+
const project = await createProject(prisma, { workloadType: "MICROVM" });
327+
const service = new TemplatePrebuildService(
328+
prisma,
329+
config({ url: undefined }),
330+
undefined,
331+
templatePrebuildTelemetry(metrics.meter)
332+
);
333+
const result = await service.prebuild({
334+
...deployment,
335+
projectId: project.id,
336+
imageDigest: DIGEST,
337+
});
338+
339+
expect(result).toEqual({ mode: "skip", reason: "no-builder" });
340+
const exported = await metrics.getMetrics();
341+
const skips = exported
342+
.at(-1)
343+
?.scopeMetrics.flatMap((scope) => scope.metrics)
344+
.find((metric) => metric.descriptor.name === "deploy.template_prebuild.skips");
345+
expect(skips?.dataPoints.map(({ attributes, value }) => ({ attributes, value }))).toEqual([
346+
{ attributes: { reason: "no-builder", eligible: true }, value: 1 },
347+
]);
348+
} finally {
349+
await metrics.shutdown();
350+
}
351+
});
352+
321353
postgresTest("does not await the builder in shadow mode", async ({ prisma }) => {
322354
// Never answers, so prebuild can only return if it does not wait.
323355
const server = await builder(() => {});

‎apps/webapp/app/v3/services/templatePrebuild.server.ts‎

Lines changed: 76 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getMeter } from "@internal/tracing";
1+
import { getMeter, type Meter } from "@internal/tracing";
22
import type { MachinePresetName } from "@trigger.dev/core/v3";
33
import { prisma as defaultPrisma, type PrismaClientOrTransaction } from "~/db.server";
44
import { env } from "~/env.server";
@@ -35,6 +35,17 @@ export type TemplateBuildOutcome =
3535
| "timeout"
3636
| "rate_limited";
3737

38+
/** Why a deploy was or was not eligible, logged when the prebuild is skipped. */
39+
type EligibilityDiagnostics = {
40+
organizationId?: string;
41+
workloadType?: string;
42+
masterQueue?: string;
43+
hasBacking?: boolean;
44+
registryRows?: number | "cold";
45+
migrated?: boolean;
46+
eligible?: boolean;
47+
};
48+
3849
type TemplateBuildRequestSkip = "no_builder" | "has_key" | "no_pinned_image";
3950

4051
/** Why a 2xx from the builder still ended as `failed`: the key was not written. */
@@ -78,18 +89,26 @@ type TemplateCreateResponse = {
7889
error?: string;
7990
};
8091

81-
const telemetry = singleton("templatePrebuildTelemetry", () => {
82-
const meter = getMeter("template-prebuild");
92+
export function templatePrebuildTelemetry(meter: Meter) {
8393
return {
8494
calls: meter.createCounter("deploy.template_prebuild.calls", {
8595
description: "Template builder calls made at deploy finalisation",
8696
}),
97+
skips: meter.createCounter("deploy.template_prebuild.skips", {
98+
description: "Deploy finalisations that did not call the template builder",
99+
}),
87100
duration: meter.createHistogram("deploy.template_prebuild.duration_ms", {
88101
description: "Template builder call duration",
89102
unit: "ms",
90103
}),
91104
};
92-
});
105+
}
106+
107+
type TemplatePrebuildTelemetry = ReturnType<typeof templatePrebuildTelemetry>;
108+
109+
const defaultTelemetry = singleton("templatePrebuildTelemetry", () =>
110+
templatePrebuildTelemetry(getMeter("template-prebuild"))
111+
);
93112

94113
/** The image pinned by digest, or undefined when no valid digest is known. */
95114
export function pinnedImageReference(
@@ -143,7 +162,8 @@ export class TemplatePrebuildService {
143162
presets: env.TEMPLATE_BUILDER_MACHINE_PRESETS,
144163
shadowRolloutPct: env.TEMPLATE_BUILDER_SHADOW_ROLLOUT_PCT,
145164
},
146-
private readonly taskMetaCache: TaskMetadataCache = taskMetadataCacheInstance
165+
private readonly taskMetaCache: TaskMetadataCache = taskMetadataCacheInstance,
166+
private readonly telemetry: TemplatePrebuildTelemetry = defaultTelemetry
147167
) {}
148168

149169
/**
@@ -163,8 +183,16 @@ export class TemplatePrebuildService {
163183
outcome?: TemplateBuildOutcome;
164184
}> {
165185
try {
166-
const { mode, reason } = await this.resolveMode(options.projectId);
186+
const { mode, reason, diagnostics } = await this.resolve(options.projectId);
167187
if (mode === "skip") {
188+
logger.info("Template prebuild skipped", {
189+
deploymentId: options.deploymentFriendlyId,
190+
projectId: options.projectId,
191+
mode,
192+
reason,
193+
...diagnostics,
194+
});
195+
this.telemetry.skips.add(1, { reason, eligible: diagnostics.eligible ?? false });
168196
return { mode, reason };
169197
}
170198

@@ -174,6 +202,7 @@ export class TemplatePrebuildService {
174202
deploymentId: options.deploymentFriendlyId,
175203
imageReference: options.imageReference,
176204
});
205+
this.telemetry.skips.add(1, { reason: "no-digest", eligible: true });
177206
return { mode: "skip", reason };
178207
}
179208

@@ -196,17 +225,24 @@ export class TemplatePrebuildService {
196225
deploymentId: options.deploymentFriendlyId,
197226
error: error instanceof Error ? error.message : String(error),
198227
});
228+
this.telemetry.skips.add(1, { reason: "error" });
199229
return { mode: "skip" };
200230
}
201231
}
202232

203233
async resolveMode(
204234
projectId: string
205235
): Promise<{ mode: TemplatePrebuildMode; reason: TemplatePrebuildReason }> {
206-
if (!this.config.url) {
207-
return { mode: "skip", reason: "no-builder" };
208-
}
236+
const { mode, reason } = await this.resolve(projectId);
237+
return { mode, reason };
238+
}
209239

240+
// Resolves eligibility even without a builder, so a skip shows whether the deploy would build.
241+
private async resolve(projectId: string): Promise<{
242+
mode: TemplatePrebuildMode;
243+
reason: TemplatePrebuildReason;
244+
diagnostics: EligibilityDiagnostics;
245+
}> {
210246
const project = await this.prisma.project.findFirst({
211247
where: { id: projectId },
212248
select: {
@@ -216,16 +252,25 @@ export class TemplatePrebuildService {
216252
});
217253

218254
if (!project) {
219-
return { mode: "skip", reason: "no-project" };
255+
return { mode: "skip", reason: "no-project", diagnostics: {} };
220256
}
221257

222-
const reason = await this.eligibility(project);
258+
const diagnostics: EligibilityDiagnostics = {
259+
organizationId: project.organization.id,
260+
workloadType: project.defaultWorkerGroup?.workloadType,
261+
masterQueue: project.defaultWorkerGroup?.masterQueue,
262+
};
263+
const reason = await this.eligibility(project, diagnostics);
264+
diagnostics.eligible = reason !== "none";
265+
if (!this.config.url) {
266+
return { mode: "skip", reason: "no-builder", diagnostics };
267+
}
223268
if (reason === "none") {
224-
return { mode: "skip", reason };
269+
return { mode: "skip", reason, diagnostics };
225270
}
226271
// The rollout only samples orgs without compute, so it never holds their deploys.
227272
if (reason === "rollout") {
228-
return { mode: "shadow", reason };
273+
return { mode: "shadow", reason, diagnostics };
229274
}
230275

231276
const wait = await makeFlag(this.prisma)({
@@ -234,21 +279,29 @@ export class TemplatePrebuildService {
234279
overrides: (project.organization.featureFlags as Record<string, unknown>) ?? {},
235280
});
236281

237-
return { mode: wait ? "wait" : "shadow", reason };
282+
return { mode: wait ? "wait" : "shadow", reason, diagnostics };
238283
}
239284

240-
private async eligibility(project: {
241-
defaultWorkerGroup: { workloadType: string; masterQueue: string } | null;
242-
organization: { id: string; featureFlags: unknown };
243-
}): Promise<TemplatePrebuildReason> {
285+
private async eligibility(
286+
project: {
287+
defaultWorkerGroup: { workloadType: string; masterQueue: string } | null;
288+
organization: { id: string; featureFlags: unknown };
289+
},
290+
diagnostics: EligibilityDiagnostics
291+
): Promise<TemplatePrebuildReason> {
292+
const groups = workerRegionRegistry.current();
293+
const defaultQueue = project.defaultWorkerGroup?.masterQueue;
294+
const backing = defaultQueue ? backingForQueue(defaultQueue, groups ?? []) : undefined;
295+
diagnostics.registryRows = groups ? groups.length : "cold";
296+
diagnostics.hasBacking = backing !== undefined;
297+
244298
if (project.defaultWorkerGroup?.workloadType === "MICROVM") {
245299
return "microvm-native";
246300
}
247301

248302
// Migrated orgs run on the compute backing although their stored default is
249303
// still the container region. A cold registry read finds no backing, so skips.
250-
const defaultQueue = project.defaultWorkerGroup?.masterQueue;
251-
if (defaultQueue && backingForQueue(defaultQueue, workerRegionRegistry.current() ?? [])) {
304+
if (backing) {
252305
const decision = {
253306
orgId: project.organization.id,
254307
orgFeatureFlags: project.organization.featureFlags as Record<string, unknown> | null,
@@ -268,6 +321,7 @@ export class TemplatePrebuildService {
268321
}
269322
migrated = isOrgMigrated({ ...decision, planType });
270323
}
324+
diagnostics.migrated = migrated;
271325
if (migrated) {
272326
return "migrated";
273327
}
@@ -416,8 +470,8 @@ export class TemplatePrebuildService {
416470
}
417471

418472
const attributes = { outcome, mode };
419-
telemetry.calls.add(1, attributes);
420-
telemetry.duration.record(performance.now() - startedAt, attributes);
473+
this.telemetry.calls.add(1, attributes);
474+
this.telemetry.duration.record(performance.now() - startedAt, attributes);
421475
return outcome === "failed" ? { outcome, reason } : { outcome };
422476
}
423477

0 commit comments

Comments
 (0)