Release 2.6.2-rc1

2026-10-05 19:35:58 +00:00
parent 269675c562
commit 198b7866ef
14 changed files with 699 additions and 113 deletions
@@ -23,6 +23,7 @@ import {
  materializeWorkspace,
  parseWorkspaceManifest,
  type MaterializeCas,
+  type MaterializeTiming,
 } from "./materialize";
 import { parseImageReference, resolveRegistryDigest } from "./registry";

@@ -39,12 +40,7 @@ export interface BuildCas extends MaterializeCas {
 }

 export type JobPhase =
-  | "queued"
-  | "creating"
-  | "starting"
-  | "running"
-  | "succeeded"
-  | "failed";
+  "queued" | "creating" | "starting" | "running" | "succeeded" | "failed";

 export interface BuildJobObservation {
  phase: JobPhase;
@@ -451,9 +447,68 @@ export class BuildController {
      }));
    validateRequest(request);
    const imageKey = `${request.project}\0${request.service}\0${request.spec.image}`;
+    const started = performance.now();
    const previous =
      this.imageSubmissionLocks.get(imageKey) ?? Promise.resolve();
-    const current = previous.then(() => this.submitBuildInternal(request));
+    const current = previous.then(async () => {
+      const queueWaitMs = performance.now() - started;
+      const phases: Record<string, number> = {};
+      const materialize: MaterializeTiming = { casReadMs: 0, fsWriteMs: 0 };
+      let outcome = "failure";
+      let jobCreated = false;
+      const timed = async <T>(
+        phase: string,
+        run: () => Promise<T>,
+      ): Promise<T> => {
+        const start = performance.now();
+        try {
+          return await run();
+        } finally {
+          phases[phase] = performance.now() - start;
+        }
+      };
+      try {
+        const status = await this.submitBuildInternal(
+          request,
+          timed,
+          materialize,
+          () => {
+            jobCreated = true;
+          },
+        );
+        outcome = "success";
+        return status;
+      } finally {
+        // Fixed-schema numeric telemetry only: never serialize a request or error.
+        try {
+          console.info(
+            JSON.stringify({
+              event: "build_submission_timing",
+              buildRequestId:
+                /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(
+                  request.id,
+                )
+                  ? request.id
+                  : null,
+              outcome,
+              jobCreated,
+              elapsedMs: performance.now() - started,
+              queueWaitMs,
+              phases,
+              materialize: {
+                fileCount: materialize.fileCount ?? null,
+                manifestBytes: materialize.manifestBytes ?? null,
+                fileBytes: materialize.fileBytes ?? null,
+                casReadMs: materialize.casReadMs,
+                fsWriteMs: materialize.fsWriteMs,
+              },
+            }),
+          );
+        } catch {
+          // Telemetry must not change submission behavior.
+        }
+      }
+    });
    const entry = current.catch(() => undefined);
    this.imageSubmissionLocks.set(imageKey, entry);
    try {
@@ -466,9 +521,14 @@ export class BuildController {

  private async submitBuildInternal(
    request: BuildRequest,
+    timed: <T>(phase: string, run: () => Promise<T>) => Promise<T>,
+    materialize: MaterializeTiming,
+    markJobCreated: () => void,
  ): Promise<BuildStatus> {
    const imageKey = `${request.project}\0${request.service}\0${request.spec.image}`;
-    const snapshot = await this.negotiateSnapshot(request.spec.workspace);
+    const snapshot = await timed("snapshotMs", () =>
+      this.negotiateSnapshot(request.spec.workspace),
+    );
    if (!snapshot.ready)
      throw new BuildConflictError(
        `Workspace snapshot is incomplete: ${snapshot.missing.join(", ")}`,
@@ -511,26 +571,33 @@ export class BuildController {
    };
    let stored: BuildRecord;
    try {
-      const result = await this.options.store.createBuild(record);
+      const result = await timed("recordCreateMs", () =>
+        this.options.store.createBuild(record),
+      );
      stored = result.record;
-      for (const superseded of result.superseded ?? []) {
-        // Deletion is best effort and does not wait for the Job or its pods.
-        await this.options.kubernetes
-          .deleteJob(this.options.namespace, superseded.spec.jobName)
-          .catch(() => undefined);
-        await rm(
-          join(this.options.workspaceRoot, superseded.spec.workspaceSubPath),
-          { recursive: true, force: true },
-        ).catch(() => undefined);
-      }
+      await timed("supersededCleanupMs", async () => {
+        for (const superseded of result.superseded ?? []) {
+          // Deletion is best effort and does not wait for the Job or its pods.
+          await this.options.kubernetes
+            .deleteJob(this.options.namespace, superseded.spec.jobName)
+            .catch(() => undefined);
+          await rm(
+            join(this.options.workspaceRoot, superseded.spec.workspaceSubPath),
+            { recursive: true, force: true },
+          ).catch(() => undefined);
+        }
+      });
      if (!result.created) return recordStatus(stored);
-      const current = await this.options.store.getBuild(request.id);
-      if (
-        !current ||
-        current.status.state !== "queued" ||
-        (this.options.store.ownsBuild &&
-          !(await this.options.store.ownsBuild(imageKey, request.id)))
-      )
+      const owns = await timed("initialOwnershipMs", async () => {
+        const current = await this.options.store.getBuild(request.id);
+        return (
+          !!current &&
+          current.status.state === "queued" &&
+          (!this.options.store.ownsBuild ||
+            (await this.options.store.ownsBuild(imageKey, request.id)))
+        );
+      });
+      if (!owns)
        throw new BuildConflictError(
          `Build '${request.id}' was superseded before Job creation`,
        );
@@ -540,72 +607,84 @@ export class BuildController {
      throw error;
    }
    try {
-      await this.materializer(
-        this.options.cas,
-        request.spec.workspace,
-        join(this.options.workspaceRoot, stored.spec.workspaceSubPath),
+      await timed("materializeMs", () =>
+        this.materializer(
+          this.options.cas,
+          request.spec.workspace,
+          join(this.options.workspaceRoot, stored.spec.workspaceSubPath),
+          materialize,
+        ),
      );
      if (
        this.options.store.ownsBuild &&
-        !(await this.options.store.ownsBuild(imageKey, request.id))
+        !(await timed("finalOwnershipMs", () =>
+          this.options.store.ownsBuild!(imageKey, request.id),
+        ))
      )
        throw new BuildConflictError(
          `Build '${request.id}' lost image lock before Job creation`,
        );
-      const cacheImage =
-        typeof this.options.cacheImage === "function"
-          ? this.options.cacheImage(request)
-          : this.options.cacheImage;
-      const pushImage = this.options.pushImage
-        ? this.options.pushImage(request)
-        : request.spec.image;
-      const pushImages = request.destinations?.map((destination) =>
-        this.options.pushImage
-          ? this.options.pushImage({
-              ...request,
-              service: destination.service,
-              spec: { ...request.spec, image: destination.image },
-              destinations: undefined,
-            })
-          : destination.image,
+      const job = await timed("jobSpecMs", async () => {
+        const cacheImage =
+          typeof this.options.cacheImage === "function"
+            ? this.options.cacheImage(request)
+            : this.options.cacheImage;
+        const pushImage = this.options.pushImage
+          ? this.options.pushImage(request)
+          : request.spec.image;
+        const pushImages = request.destinations?.map((destination) =>
+          this.options.pushImage
+            ? this.options.pushImage({
+                ...request,
+                service: destination.service,
+                spec: { ...request.spec, image: destination.image },
+                destinations: undefined,
+              })
+            : destination.image,
+        );
+        return createBuildJob({
+          name: jobName,
+          namespace: this.options.namespace,
+          spec: request.spec,
+          workspaceClaimName: this.options.workspaceClaimName,
+          workspaceSubPath: jobWorkspaceSubPath(
+            this.options.workspaceRoot,
+            stored.spec.workspaceSubPath,
+          ),
+          cacheImage,
+          pushImage,
+          pushImages,
+          pushRegistryInsecure: this.options.pushRegistryInsecure,
+          cacheRegistryInsecure:
+            this.options.cacheRegistryInsecure ??
+            this.options.pushRegistryInsecure,
+          buildkitImage: this.options.buildkitImage,
+          serviceAccountName: this.options.serviceAccountName,
+          registrySecretName: this.options.registrySecretName,
+          nodeSelector: this.options.nodeSelector,
+          tolerations: this.options.tolerations,
+        });
+      });
+      await timed("jobCreateMs", () => this.options.kubernetes.createJob(job));
+      markJobCreated();
+      await timed("recordUpdateMs", () =>
+        this.updateBuild(stored, (next) => {
+          next.status.jobCreated = true;
+        }),
      );
-      const job = createBuildJob({
-        name: jobName,
-        namespace: this.options.namespace,
-        spec: request.spec,
-        workspaceClaimName: this.options.workspaceClaimName,
-        workspaceSubPath: jobWorkspaceSubPath(
-          this.options.workspaceRoot,
-          stored.spec.workspaceSubPath,
-        ),
-        cacheImage,
-        pushImage,
-        pushImages,
-        pushRegistryInsecure: this.options.pushRegistryInsecure,
-        cacheRegistryInsecure:
-          this.options.cacheRegistryInsecure ??
-          this.options.pushRegistryInsecure,
-        buildkitImage: this.options.buildkitImage,
-        serviceAccountName: this.options.serviceAccountName,
-        registrySecretName: this.options.registrySecretName,
-        nodeSelector: this.options.nodeSelector,
-        tolerations: this.options.tolerations,
-      });
-      await this.options.kubernetes.createJob(job);
-      await this.updateBuild(stored, (next) => {
-        next.status.jobCreated = true;
-      });
      return initial;
    } catch (error) {
-      try {
-        await this.terminateJob(stored.spec.jobName);
-      } catch {
-        // Job cleanup is best effort; the record still releases its lock.
-      }
-      await this.failBuild(
-        stored.metadata.name,
-        error instanceof Error ? error.message : String(error),
-      );
+      await timed("failureCleanupMs", async () => {
+        try {
+          await this.terminateJob(stored.spec.jobName);
+        } catch {
+          // Job cleanup is best effort; the record still releases its lock.
+        }
+        await this.failBuild(
+          stored.metadata.name,
+          error instanceof Error ? error.message : String(error),
+        );
+      });
      throw error;
    }
  }
@@ -27,7 +27,7 @@ export interface MaintenanceLeaseProvider {
  ): Promise<{ release(): Promise<void> } | undefined>;
 }

-/** Accept DNS hostnames only: no URL syntax, port, address literals, or wildcards. */
+/** Accept DNS hostnames and an optional leading *. only; no URL syntax, port, or address literals. */
 export function normalizeMaintenanceHost(value: unknown): string {
  if (typeof value !== "string") throw new Error("host is required");
  const host = value.trim().toLowerCase().replace(/\.$/, "");
@@ -35,7 +35,7 @@ export function normalizeMaintenanceHost(value: unknown): string {
    host.length === 0 ||
    host.length > 253 ||
    !host.includes(".") ||
-    !/^(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)+[a-z]{2,63}$/.test(host)
+    !/^(?:\*\.)?(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)+[a-z]{2,63}$/.test(host)
  )
    throw new Error("host must be a DNS hostname");
  return host;
@@ -29,6 +29,16 @@ export interface MaterializeCas {
  get(digest: Sha256Digest): Promise<Uint8Array>;
 }

+export interface MaterializeTiming {
+  fileCount?: number;
+  manifestBytes?: number;
+  fileBytes?: number;
+  // Sum of operation durations; concurrent operations can exceed wall time.
+  // CAS get includes its content hash verification.
+  casReadMs: number;
+  fsWriteMs: number;
+}
+
 const MATERIALIZE_CONCURRENCY = 20;

 async function mapConcurrent<T, R>(
@@ -101,8 +111,18 @@ export function parseWorkspaceManifest(data: Uint8Array): WorkspaceManifest {
    }
    paths.add(file.path);
  }
+  const ancestors = new Set<string>();
  for (const path of paths) {
-    if ([...paths].some((other) => other.startsWith(`${path}/`))) {
+    for (
+      let separator = path.indexOf("/");
+      separator !== -1;
+      separator = path.indexOf("/", separator + 1)
+    ) {
+      ancestors.add(path.slice(0, separator));
+    }
+  }
+  for (const path of paths) {
+    if (ancestors.has(path)) {
      throw new Error(`Workspace path conflicts with a directory: ${path}`);
    }
  }
@@ -125,13 +145,43 @@ export async function materializeWorkspace(
  cas: MaterializeCas,
  manifestDigest: Sha256Digest,
  destination: string,
+  timing?: MaterializeTiming,
 ): Promise<WorkspaceManifest> {
+  const read = timing
+    ? async (digest: Sha256Digest) => {
+        const start = performance.now();
+        try {
+          return await cas.get(digest);
+        } finally {
+          timing.casReadMs += performance.now() - start;
+        }
+      }
+    : (digest: Sha256Digest) => cas.get(digest);
+  const write = timing
+    ? async <T>(operation: () => Promise<T>): Promise<T> => {
+        const start = performance.now();
+        try {
+          return await operation();
+        } finally {
+          timing.fsWriteMs += performance.now() - start;
+        }
+      }
+    : <T>(operation: () => Promise<T>) => operation();
  assertSha256Digest(manifestDigest);
-  const manifest = parseWorkspaceManifest(await cas.get(manifestDigest));
+  const manifestData = await read(manifestDigest);
+  if (timing) timing.manifestBytes = manifestData.byteLength;
+  const manifest = parseWorkspaceManifest(manifestData);
+  if (timing) {
+    timing.fileCount = manifest.files.length;
+    timing.fileBytes = manifest.files.reduce(
+      (total, file) => total + file.size,
+      0,
+    );
+  }

-  await mkdir(dirname(destination), { recursive: true });
+  await write(() => mkdir(dirname(destination), { recursive: true }));
  try {
-    await lstat(destination);
+    await write(() => lstat(destination));
    throw new Error(`Workspace destination already exists: ${destination}`);
  } catch (error) {
    if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error;
@@ -140,14 +190,16 @@ export async function materializeWorkspace(
    dirname(destination),
    `.${basename(destination)}.${process.pid}.${randomUUID()}.tmp`,
  );
-  await mkdir(temporary, { mode: 0o755 });
+  await write(() => mkdir(temporary, { mode: 0o755 }));
  try {
    await mapConcurrent(manifest.files, async (file) => {
      const target = join(temporary, file.path);
      if (relative(temporary, target).startsWith(".."))
        throw new Error("Unsafe workspace path");
-      await mkdir(dirname(target), { recursive: true, mode: 0o755 });
-      const data = await cas.get(file.digest);
+      await write(() =>
+        mkdir(dirname(target), { recursive: true, mode: 0o755 }),
+      );
+      const data = await read(file.digest);
      if (data.byteLength !== file.size) {
        throw new Error(`Workspace blob size mismatch for ${file.path}`);
      }
@@ -155,24 +207,26 @@ export async function materializeWorkspace(
        const link = new TextDecoder("utf-8", { fatal: true }).decode(data);
        if (!safeSymlinkTarget(file.path, link))
          throw new Error(`Unsafe symlink target for ${file.path}`);
-        await symlink(link, target);
+        await write(() => symlink(link, target));
      } else {
-        const handle = await open(
-          target,
-          constants.O_CREAT | constants.O_EXCL | constants.O_WRONLY,
-          file.mode,
+        const handle = await write(() =>
+          open(
+            target,
+            constants.O_CREAT | constants.O_EXCL | constants.O_WRONLY,
+            file.mode,
+          ),
        );
        try {
-          await handle.writeFile(data);
+          await write(() => handle.writeFile(data));
        } finally {
-          await handle.close();
+          await write(() => handle.close());
        }
-        await chmod(target, file.mode);
+        await write(() => chmod(target, file.mode));
      }
    });
-    await rename(temporary, destination);
+    await write(() => rename(temporary, destination));
  } catch (error) {
-    await rm(temporary, { recursive: true, force: true });
+    await write(() => rm(temporary, { recursive: true, force: true }));
    throw error;
  }
  return manifest;