From 9492864b29adec8d05a9a4634e883b1be3876e04 Mon Sep 17 00:00:00 2001 From: Rawad Hossain Date: Mon, 24 Aug 2026 11:05:38 +0600 Subject: [PATCH 1/2] reshape metric text Signed-off-by: Rawad Hossain --- docs/book/src/operations/monitoring.md | 2 +- internal/metrics/metrics.go | 2 +- internal/metrics/metrics_test.go | 95 ++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 2 deletions(-) diff --git a/docs/book/src/operations/monitoring.md b/docs/book/src/operations/monitoring.md index 2d568402..d705fcad 100644 --- a/docs/book/src/operations/monitoring.md +++ b/docs/book/src/operations/monitoring.md @@ -37,7 +37,7 @@ Total number of taint operations performed by the controller. ### `node_readiness_evaluation_duration_seconds` -Duration of rule evaluations per rule. +Duration of rule evaluations per rule, including taint operations. | Property | Value | | --- | --- | diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index db418e48..23d114e6 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -87,7 +87,7 @@ var ( EvaluationDuration = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Name: "node_readiness_evaluation_duration_seconds", - Help: "Duration of rule evaluations per rule", + Help: "Duration of rule evaluations per rule, including taint operations", Buckets: prometheus.DefBuckets, }, []string{"rule"}, diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 0a58a880..774ff753 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -20,6 +20,7 @@ import ( "strings" "testing" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" "sigs.k8s.io/controller-runtime/pkg/metrics" ) @@ -53,3 +54,97 @@ node_readiness_build_info{version="unknown"} 1 t.Fatal("expected node_readiness_build_info to be registered with the controller-runtime metrics registry") } } + +func TestEvaluationDuration(t *testing.T) { + t.Run("registered as histogram with expected help text", func(t *testing.T) { + EvaluationDuration.Reset() + EvaluationDuration.WithLabelValues("registration-check").Observe(0) + defer EvaluationDuration.Reset() + + gathered, err := metrics.Registry.Gather() + if err != nil { + t.Fatalf("failed to gather metrics: %v", err) + } + + var found bool + for _, mf := range gathered { + if mf.GetName() == "node_readiness_evaluation_duration_seconds" { + found = true + if got := mf.GetType().String(); got != "HISTOGRAM" { + t.Fatalf("expected node_readiness_evaluation_duration_seconds to be a histogram, got %s", got) + } + const wantHelp = "Duration of rule evaluations per rule, including taint operations" + if got := mf.GetHelp(); got != wantHelp { + t.Fatalf("unexpected help text: got %q, want %q", got, wantHelp) + } + break + } + } + if !found { + t.Fatal("expected node_readiness_evaluation_duration_seconds to be registered with the controller-runtime metrics registry") + } + }) + + t.Run("label set is exactly rule", func(t *testing.T) { + descs := make(chan *prometheus.Desc, 1) + EvaluationDuration.Describe(descs) + close(descs) + + desc := <-descs + if desc == nil { + t.Fatal("expected EvaluationDuration to yield a descriptor") + } + if got, want := desc.String(), "variableLabels: {rule}"; !strings.Contains(got, want) { + t.Fatalf("expected descriptor to declare exactly one variable label %q, got: %s", want, got) + } + }) + + t.Run("observation is reflected", func(t *testing.T) { + EvaluationDuration.Reset() + + EvaluationDuration.WithLabelValues("test-rule").Observe(0.2) + + expected := ` +# HELP node_readiness_evaluation_duration_seconds Duration of rule evaluations per rule, including taint operations +# TYPE node_readiness_evaluation_duration_seconds histogram +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="0.005"} 0 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="0.01"} 0 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="0.025"} 0 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="0.05"} 0 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="0.1"} 0 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="0.25"} 1 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="0.5"} 1 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="1"} 1 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="2.5"} 1 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="5"} 1 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="10"} 1 +node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="+Inf"} 1 +node_readiness_evaluation_duration_seconds_sum{rule="test-rule"} 0.2 +node_readiness_evaluation_duration_seconds_count{rule="test-rule"} 1 +` + if err := testutil.CollectAndCompare(EvaluationDuration, strings.NewReader(expected), "node_readiness_evaluation_duration_seconds"); err != nil { + t.Fatalf("unexpected collecting result:\n%s", err) + } + + if got, want := testutil.CollectAndCount(EvaluationDuration, "node_readiness_evaluation_duration_seconds"), 1; got != want { + t.Fatalf("expected %d observed series, got %d", want, got) + } + }) + + t.Run("DeleteLabelValues removes the rule's series", func(t *testing.T) { + EvaluationDuration.Reset() + + EvaluationDuration.WithLabelValues("delete-me").Observe(0.1) + if got, want := testutil.CollectAndCount(EvaluationDuration, "node_readiness_evaluation_duration_seconds"), 1; got != want { + t.Fatalf("expected %d observed series before delete, got %d", want, got) + } + + if deleted := EvaluationDuration.DeleteLabelValues("delete-me"); !deleted { + t.Fatal("expected DeleteLabelValues to report that a series was deleted") + } + + if got, want := testutil.CollectAndCount(EvaluationDuration, "node_readiness_evaluation_duration_seconds"), 0; got != want { + t.Fatalf("expected %d observed series after delete, got %d", want, got) + } + }) +} From 5ed9a3fae830316e992f79a8fbf3147fcefe7bd5 Mon Sep 17 00:00:00 2001 From: Rawad Hossain Date: Tue, 1 Sep 2026 12:55:06 +0600 Subject: [PATCH 2/2] adjust tests and wording fix --- docs/book/src/operations/monitoring.md | 2 +- internal/metrics/metrics.go | 2 +- internal/metrics/metrics_test.go | 64 +++++++++++++++----------- 3 files changed, 40 insertions(+), 28 deletions(-) diff --git a/docs/book/src/operations/monitoring.md b/docs/book/src/operations/monitoring.md index d705fcad..acf50847 100644 --- a/docs/book/src/operations/monitoring.md +++ b/docs/book/src/operations/monitoring.md @@ -37,7 +37,7 @@ Total number of taint operations performed by the controller. ### `node_readiness_evaluation_duration_seconds` -Duration of rule evaluations per rule, including taint operations. +Duration of evaluating a rule against a node, including any taint add/remove operations. | Property | Value | | --- | --- | diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 23d114e6..716fe95c 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -87,7 +87,7 @@ var ( EvaluationDuration = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Name: "node_readiness_evaluation_duration_seconds", - Help: "Duration of rule evaluations per rule, including taint operations", + Help: "Duration of evaluating a rule against a node, including any taint add/remove operations", Buckets: prometheus.DefBuckets, }, []string{"rule"}, diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 774ff753..531adbd0 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -61,28 +61,9 @@ func TestEvaluationDuration(t *testing.T) { EvaluationDuration.WithLabelValues("registration-check").Observe(0) defer EvaluationDuration.Reset() - gathered, err := metrics.Registry.Gather() - if err != nil { - t.Fatalf("failed to gather metrics: %v", err) - } - - var found bool - for _, mf := range gathered { - if mf.GetName() == "node_readiness_evaluation_duration_seconds" { - found = true - if got := mf.GetType().String(); got != "HISTOGRAM" { - t.Fatalf("expected node_readiness_evaluation_duration_seconds to be a histogram, got %s", got) - } - const wantHelp = "Duration of rule evaluations per rule, including taint operations" - if got := mf.GetHelp(); got != wantHelp { - t.Fatalf("unexpected help text: got %q, want %q", got, wantHelp) - } - break - } - } - if !found { - t.Fatal("expected node_readiness_evaluation_duration_seconds to be registered with the controller-runtime metrics registry") - } + assertMetricRegistered(t, metrics.Registry, + "node_readiness_evaluation_duration_seconds", "HISTOGRAM", + "Duration of evaluating a rule against a node, including any taint add/remove operations") }) t.Run("label set is exactly rule", func(t *testing.T) { @@ -105,7 +86,7 @@ func TestEvaluationDuration(t *testing.T) { EvaluationDuration.WithLabelValues("test-rule").Observe(0.2) expected := ` -# HELP node_readiness_evaluation_duration_seconds Duration of rule evaluations per rule, including taint operations +# HELP node_readiness_evaluation_duration_seconds Duration of evaluating a rule against a node, including any taint add/remove operations # TYPE node_readiness_evaluation_duration_seconds histogram node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="0.005"} 0 node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="0.01"} 0 @@ -122,9 +103,7 @@ node_readiness_evaluation_duration_seconds_bucket{rule="test-rule",le="+Inf"} 1 node_readiness_evaluation_duration_seconds_sum{rule="test-rule"} 0.2 node_readiness_evaluation_duration_seconds_count{rule="test-rule"} 1 ` - if err := testutil.CollectAndCompare(EvaluationDuration, strings.NewReader(expected), "node_readiness_evaluation_duration_seconds"); err != nil { - t.Fatalf("unexpected collecting result:\n%s", err) - } + assertObservationReflected(t, EvaluationDuration, "node_readiness_evaluation_duration_seconds", expected) if got, want := testutil.CollectAndCount(EvaluationDuration, "node_readiness_evaluation_duration_seconds"), 1; got != want { t.Fatalf("expected %d observed series, got %d", want, got) @@ -148,3 +127,36 @@ node_readiness_evaluation_duration_seconds_count{rule="test-rule"} 1 } }) } + +// assertMetricRegistered checks that the metric is registered correctly. +func assertMetricRegistered(t *testing.T, registry prometheus.Gatherer, name, wantType, wantHelp string) { + t.Helper() + + gathered, err := registry.Gather() + if err != nil { + t.Fatalf("failed to gather metrics: %v", err) + } + + for _, mf := range gathered { + if mf.GetName() != name { + continue + } + if got := mf.GetType().String(); got != wantType { + t.Fatalf("expected %s to be a %s, got %s", name, wantType, got) + } + if got := mf.GetHelp(); got != wantHelp { + t.Fatalf("unexpected help text for %s: got %q, want %q", name, got, wantHelp) + } + return + } + t.Fatalf("expected %s to be registered with the controller-runtime metrics registry", name) +} + +// assertObservationReflected checks the collected metric. +func assertObservationReflected(t *testing.T, collector prometheus.Collector, name, expectedExposition string) { + t.Helper() + + if err := testutil.CollectAndCompare(collector, strings.NewReader(expectedExposition), name); err != nil { + t.Fatalf("unexpected collecting result:\n%s", err) + } +}