Unverified Commit 75642d90 authored by Minseong Choi's avatar Minseong Choi 💬
Browse files

feat(metrics): add named felis_* Prometheus collectors

Introduce internal/metrics exposing the four metric families spec §23
mandates at minimum: felis_servers_total (gauge by desired state),
felis_start_duration_seconds (histogram with Minecraft cold-start
buckets), felis_image_build_failures_total and
felis_reaper_worlds_deleted_total (counters). Collectors are
package-level vars so any subsystem records without an import cycle;
Register wires them into a prometheus.Registerer and is idempotent.

Wire registration into the operator against controller-runtime's global
Registry, so /metrics on the manager's existing metrics endpoint carries
the felis_* families. Instrument the reaper to increment
felis_reaper_worlds_deleted_total in lockstep with Summary.WorldsReaped,
at the one point a world's PVC has actually been deleted.
parent 50b8487f
Loading
Loading
Loading
Loading
+12 −0
Changes for cmd/felis/operator.go: 12 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -6,12 +6,14 @@ import (
	"io"

	"felis.lolicon.best/internal/apis/felis/v1alpha1"
	felismetrics "felis.lolicon.best/internal/metrics"
	"felis.lolicon.best/internal/operator"
	"k8s.io/apimachinery/pkg/runtime"
	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
	ctrl "sigs.k8s.io/controller-runtime"
	"sigs.k8s.io/controller-runtime/pkg/cache"
	ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
)

@@ -57,6 +59,16 @@ func cmdOperator(args []string, _, stderr io.Writer) int {
	}
	fmt.Fprintf(stderr, "felis operator: watching namespace %q\n", *namespace)

	// Publish the named felis_* metrics (spec §23) on the endpoint the manager
	// already serves (metricsAddr). controller-runtime's metrics server exposes
	// its global Registry, so registering into it is all that is needed for
	// /metrics to carry felis_servers_total and friends. Register is idempotent,
	// so an in-process restart never double-registers fatally.
	if err := felismetrics.Register(ctrlmetrics.Registry); err != nil {
		fmt.Fprintf(stderr, "felis operator: register metrics: %v\n", err)
		return 1
	}

	r := &operator.Reconciler{
		Client: mgr.GetClient(),
		Scheme: mgr.GetScheme(),
+89 −0
Changes for internal/metrics/metrics.go: 89 added lines, 0 removed lines.
Original line number Diff line number Diff line
// Package metrics defines and registers the named felis_* Prometheus
// collectors mandated by spec §23.
//
// The collectors are package-level vars so any subsystem (operator, reaper,
// image builder, prober) can record into them without importing back into a
// metrics owner and risking an import cycle. Wiring them into a registry is a
// single Register call that takes a prometheus.Registerer — the same
// inject-the-interface, fake-at-the-edge pattern used elsewhere in Felis
// (ownerStore, PVCResolver): production passes controller-runtime's global
// Registry (served by the operator's :metrics endpoint), tests pass a fresh
// prometheus.NewRegistry() so assertions never collide with global state.
package metrics

import (
	"errors"

	"github.com/prometheus/client_golang/prometheus"
)

// namespace prefixes every collector, so the exposed names are exactly
// felis_<name> — matching the metric names spec §23 mandates.
const namespace = "felis"

var (
	// ServersTotal is the current number of MinecraftServers the operator knows
	// about, partitioned by desiredState. It is a gauge, not a monotonic counter:
	// the reconcile loop Set()s it to the live fleet size, so it can fall as
	// servers are deleted. (The _total suffix follows the name spec §23 fixed.)
	ServersTotal = prometheus.NewGaugeVec(prometheus.GaugeOpts{
		Namespace: namespace,
		Name:      "servers_total",
		Help:      "Current number of Minecraft servers known to the operator, by desired state.",
	}, []string{"state"})

	// StartDurationSeconds observes the wall-clock time from desiredState=Running
	// to a server reporting ready. Buckets are tuned for Minecraft cold starts
	// (seconds to a few minutes), not the default sub-second web-latency buckets.
	StartDurationSeconds = prometheus.NewHistogram(prometheus.HistogramOpts{
		Namespace: namespace,
		Name:      "start_duration_seconds",
		Help:      "Time from desiredState=Running to a server reporting ready, in seconds.",
		Buckets:   []float64{1, 2, 5, 10, 20, 30, 45, 60, 90, 120, 180, 300, 600},
	})

	// ImageBuildFailuresTotal counts modpack/image build failures (spec §8 lane).
	ImageBuildFailuresTotal = prometheus.NewCounter(prometheus.CounterOpts{
		Namespace: namespace,
		Name:      "image_build_failures_total",
		Help:      "Total number of image build failures.",
	})

	// ReaperWorldsDeletedTotal counts world volumes deleted by the reaper.
	ReaperWorldsDeletedTotal = prometheus.NewCounter(prometheus.CounterOpts{
		Namespace: namespace,
		Name:      "reaper_worlds_deleted_total",
		Help:      "Total number of world volumes deleted by the reaper.",
	})
)

// Collectors returns every felis_* collector in a stable order. Production and
// tests register the same slice, so the test asserting the full set is exposed
// also pins the production surface.
func Collectors() []prometheus.Collector {
	return []prometheus.Collector{
		ServersTotal,
		StartDurationSeconds,
		ImageBuildFailuresTotal,
		ReaperWorldsDeletedTotal,
	}
}

// Register wires every felis_* collector into r.
//
// It is idempotent: re-registering an already-registered collector (a manager
// restart in-process, a test that calls Register twice) is tolerated rather
// than fatal, so a benign double-call never takes the operator down. Any other
// registration error is returned to the caller to surface at startup.
func Register(r prometheus.Registerer) error {
	for _, c := range Collectors() {
		if err := r.Register(c); err != nil {
			var already prometheus.AlreadyRegisteredError
			if errors.As(err, &already) {
				continue
			}
			return err
		}
	}
	return nil
}
+94 −0
Changes for internal/metrics/metrics_test.go: 94 added lines, 0 removed lines.
Original line number Diff line number Diff line
package metrics

import (
	"strings"
	"testing"

	"github.com/prometheus/client_golang/prometheus"
	"github.com/prometheus/client_golang/prometheus/testutil"
)

// wantNames is the exact set of metric names spec §23 mandates "at minimum".
// Pinning them here makes a rename a deliberate, test-visible act rather than a
// silent dashboard/alert break.
var wantNames = []string{
	"felis_servers_total",
	"felis_start_duration_seconds",
	"felis_image_build_failures_total",
	"felis_reaper_worlds_deleted_total",
}

func TestRegisterExposesNamedFelisMetrics(t *testing.T) {
	reg := prometheus.NewRegistry()
	if err := Register(reg); err != nil {
		t.Fatalf("Register: %v", err)
	}

	// Record into each collector through its typed API so a Gather emits the
	// family — this proves the exported vars are the ones actually registered,
	// not shadow copies.
	ServersTotal.WithLabelValues("Running").Set(3)
	StartDurationSeconds.Observe(12.5)
	ImageBuildFailuresTotal.Inc()
	ReaperWorldsDeletedTotal.Add(2)

	mfs, err := reg.Gather()
	if err != nil {
		t.Fatalf("Gather: %v", err)
	}
	got := map[string]bool{}
	for _, mf := range mfs {
		got[mf.GetName()] = true
	}
	for _, name := range wantNames {
		if !got[name] {
			t.Errorf("mandated metric %q not exposed by the registry", name)
		}
	}
	// Nothing may leak out from under the felis_ namespace — a stray name would
	// mean a collector was declared without the namespace prefix.
	for name := range got {
		if !strings.HasPrefix(name, "felis_") {
			t.Errorf("metric %q is not under the felis_ namespace", name)
		}
	}
}

func TestRegisterIsIdempotent(t *testing.T) {
	reg := prometheus.NewRegistry()
	if err := Register(reg); err != nil {
		t.Fatalf("first Register: %v", err)
	}
	// A second Register into the same registry must not error: setup that runs
	// twice (manager restart, test re-entry) should be harmless, not fatal.
	if err := Register(reg); err != nil {
		t.Fatalf("second Register should be idempotent, got: %v", err)
	}
}

func TestCountersRecordExpectedValues(t *testing.T) {
	// These collectors are package-level singletons. This test asserts on a
	// gauge child labelled "Stopped" (no other test touches it) and a >=1 bound
	// on the build-failure counter, so values stay stable regardless of test
	// ordering or accumulation across the binary.
	reg := prometheus.NewRegistry()
	if err := Register(reg); err != nil {
		t.Fatalf("Register: %v", err)
	}

	ImageBuildFailuresTotal.Inc()
	if got := testutil.ToFloat64(ImageBuildFailuresTotal); got < 1 {
		t.Errorf("image build failures = %v, want >= 1", got)
	}

	g := ServersTotal.WithLabelValues("Stopped")
	g.Set(7)
	if got := testutil.ToFloat64(g); got != 7 {
		t.Errorf("servers_total{state=Stopped} = %v, want 7", got)
	}

	StartDurationSeconds.Observe(30)
	if n := testutil.CollectAndCount(StartDurationSeconds); n != 1 {
		t.Errorf("start_duration_seconds collected %d metrics, want 1 histogram", n)
	}
}
+5 −0
Changes for internal/reaper/reaper.go: 5 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ import (
	"time"

	"felis.lolicon.best/internal/backup"
	"felis.lolicon.best/internal/metrics"
)

// Day is a calendar day; the retention windows in §18 are expressed in days.
@@ -382,6 +383,10 @@ func (r *Reaper) reap(ctx context.Context, now time.Time, c Candidate, crd Serve
	}

	sum.WorldsReaped++
	// felis_reaper_worlds_deleted_total (spec §23) advances in lockstep with the
	// per-run Summary tally — incremented here, at the one point a world's PVC has
	// actually been deleted, not at evaluation time.
	metrics.ReaperWorldsDeletedTotal.Inc()
	r.log().Info("reaper: world reaped", "server", c.Name, "former_owner", c.OwnerID, "backup_ref", ref)
	return nil
}
+31 −0
Changes for internal/reaper/reaper_test.go: 31 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -12,6 +12,9 @@ import (
	"time"

	"felis.lolicon.best/internal/backup"
	"felis.lolicon.best/internal/metrics"

	"github.com/prometheus/client_golang/prometheus/testutil"
)

// testNow is the frozen clock for every hermetic case. Idle is expressed as an
@@ -307,6 +310,34 @@ func TestReapIdleWorldFullSequence(t *testing.T) {
	}
}

// §23 instrumentation: felis_reaper_worlds_deleted_total advances by exactly one
// per world whose PVC is actually deleted — in lockstep with Summary.WorldsReaped,
// and never for a skipped/preserved world. Asserted as a delta because the counter
// is a process-global singleton other tests in this package also advance.
func TestReapIncrementsDeletedWorldsMetric(t *testing.T) {
	before := testutil.ToFloat64(metrics.ReaperWorldsDeletedTotal)

	r, _, cl, _ := newReaper(DefaultConfig(),
		Candidate{Name: "metric-a", OwnerID: "user-9", LastActiveAt: idleBy(20 * Day)},
		Candidate{Name: "metric-b", OwnerID: "user-9", LastActiveAt: idleBy(20 * Day)},
		// Fresh server: under the deadline, must NOT be reaped or counted.
		Candidate{Name: "metric-fresh", OwnerID: "user-9", LastActiveAt: idleBy(2 * Day)},
	)

	sum := mustRun(t, r)
	if sum.WorldsReaped != 2 {
		t.Fatalf("WorldsReaped = %d, want 2", sum.WorldsReaped)
	}
	if cl.deletePVCCalls != 2 {
		t.Fatalf("deletePVC calls = %d, want 2", cl.deletePVCCalls)
	}

	delta := testutil.ToFloat64(metrics.ReaperWorldsDeletedTotal) - before
	if delta != float64(sum.WorldsReaped) {
		t.Fatalf("felis_reaper_worlds_deleted_total advanced by %v, want %d (one per reaped world)", delta, sum.WorldsReaped)
	}
}

// CENTERPIECE — red line ④: when the archive fails, the PVC is never deleted,
// ownership is untouched, no backup row is written, and the same server is
// retried (state unchanged) on the next run.