From 2a93a9e0cb661d425203e42b3881733028557256 Mon Sep 17 00:00:00 2001
From: Minseong Choi <me@flyemoji.moe>
Date: Tue, 30 Jun 2026 12:47:49 +0900
Subject: [PATCH] feat(metrics): record felis_image_build_failures_total on
 failed builds
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Wire the build subsystem to the felis_image_build_failures_total counter
(spec §23). It advances at the two terminal-failure producers: finishAt
(the Sync JobFailed/JobUnknown verdict — a kaniko failure or a CRITICAL
CVE from trivy's --exit-code 1) and Submit's job-creation bypass path,
which records its failure directly without going through finishAt.
Cancellations and successful builds are deliberately not counted.

A delta-asserting test exercises both Inc sites plus a successful-build
negative control that proves the StatusFailed guard discriminates rather
than firing on every terminal write, all over the existing in-memory
Store/Jobs fakes.
---
 internal/build/build.go      | 18 ++++++++++++
 internal/build/build_test.go | 53 ++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/internal/build/build.go b/internal/build/build.go
index 89ff26f..02ce21d 100644
--- a/internal/build/build.go
+++ b/internal/build/build.go
@@ -36,6 +36,8 @@ import (
 	"fmt"
 	"strings"
 	"time"
+
+	"felis.lolicon.best/internal/metrics"
 )
 
 // Status mirrors the build_status enum (spec §6).
@@ -307,6 +309,12 @@ func (b *Builder) Submit(ctx context.Context, req Request) (*Build, error) {
 	if err != nil {
 		// The pending row exists; mark it failed so it is not reconciled forever.
 		_ = b.Store.FinishBuild(ctx, bld.ID, StatusFailed, "job creation failed: "+err.Error(), b.now())
+		// felis_image_build_failures_total (spec §23): this terminal-failure path
+		// records the build directly, not via finishAt, so it increments the counter
+		// itself. The FinishBuild error is deliberately ignored (the build is failed
+		// for the caller regardless), so the count tracks the failure event, not the
+		// store write.
+		metrics.ImageBuildFailuresTotal.Inc()
 		bld.Status = StatusFailed
 		bld.Error = "job creation failed: " + err.Error()
 		return bld, fmt.Errorf("build: create job: %w", err)
@@ -444,6 +452,16 @@ func (b *Builder) finishAt(ctx context.Context, bld *Build, status Status, msg s
 	if err := b.Store.FinishBuild(ctx, bld.ID, status, msg, at); err != nil {
 		return nil, err
 	}
+	if status == StatusFailed {
+		// felis_image_build_failures_total (spec §23) counts builds that reached a
+		// failed terminal state — a kaniko failure or a CRITICAL CVE surfaced by
+		// trivy's --exit-code 1, observed here as the Sync JobFailed/JobUnknown
+		// verdict. Cancellations (StatusCancelled) are deliberately not failures.
+		// Incremented only after the failed status is persisted, so the counter
+		// never runs ahead of the store. (Submit's job-creation path records its
+		// failure outside finishAt and increments there.)
+		metrics.ImageBuildFailuresTotal.Inc()
+	}
 	bld.Status = status
 	bld.Error = msg
 	finished := at
diff --git a/internal/build/build_test.go b/internal/build/build_test.go
index 2d0b859..6fe2a14 100644
--- a/internal/build/build_test.go
+++ b/internal/build/build_test.go
@@ -5,6 +5,9 @@ import (
 	"errors"
 	"testing"
 	"time"
+
+	"felis.lolicon.best/internal/metrics"
+	"github.com/prometheus/client_golang/prometheus/testutil"
 )
 
 var testNow = time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC)
@@ -356,6 +359,56 @@ func TestSyncAllAdvancesUnfinishedBuilds(t *testing.T) {
 	}
 }
 
+// TestImageBuildFailuresMetricCountsFailedBuilds asserts felis_image_build_failures_total
+// (spec §23) advances exactly once per failed build from BOTH terminal-failure
+// producers, and never on a successful build. There are two distinct Inc sites —
+// finishAt (the Sync verdict) and Submit's job-creation bypass — so each is
+// exercised separately; the success case is the negative control proving the
+// StatusFailed guard discriminates rather than firing on every terminal write.
+// Deltas are read around each action because the counter is a process-global
+// singleton these package tests share (they run sequentially).
+func TestImageBuildFailuresMetricCountsFailedBuilds(t *testing.T) {
+	read := func() float64 { return testutil.ToFloat64(metrics.ImageBuildFailuresTotal) }
+
+	t.Run("sync job-failed verdict increments via finishAt", func(t *testing.T) {
+		b, _, jb := newBuilder()
+		bld, _ := b.Submit(context.Background(), goodRequest())
+		before := read()
+		jb.phase = JobFailed
+		if _, err := b.Sync(context.Background(), bld.ID); err != nil {
+			t.Fatalf("Sync: %v", err)
+		}
+		if got := read() - before; got != 1 {
+			t.Errorf("failures delta = %v, want 1", got)
+		}
+	})
+
+	t.Run("submit job-create failure increments via bypass", func(t *testing.T) {
+		b, _, jb := newBuilder()
+		jb.createErr = errors.New("apiserver down")
+		before := read()
+		if _, err := b.Submit(context.Background(), goodRequest()); err == nil {
+			t.Fatal("expected Submit error when job creation fails")
+		}
+		if got := read() - before; got != 1 {
+			t.Errorf("failures delta = %v, want 1", got)
+		}
+	})
+
+	t.Run("successful build does not increment", func(t *testing.T) {
+		b, _, jb := newBuilder()
+		bld, _ := b.Submit(context.Background(), goodRequest())
+		before := read()
+		jb.phase = JobSucceeded
+		if _, err := b.Sync(context.Background(), bld.ID); err != nil {
+			t.Fatalf("Sync: %v", err)
+		}
+		if got := read() - before; got != 0 {
+			t.Errorf("failures delta on success = %v, want 0", got)
+		}
+	})
+}
+
 // ---- cancellation ----
 
 func TestCancelDeletesJobAndMarksCancelled(t *testing.T) {