Unverified Commit 2a93a9e0 authored by Minseong Choi's avatar Minseong Choi 💬
Browse files

feat(metrics): record felis_image_build_failures_total on failed builds 

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.
parent 75642d90

internal/build/build.go

+18 −0
Changes for internal/build/build.go: 18 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -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

internal/build/build_test.go

+53 −0
Changes for internal/build/build_test.go: 53 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -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) {