metrics: set to 0 when routes are fixed

- Modify UpdateInvalidRoute to set previously seen metrics to 0 when not present in current counts
- Refactor processRouteDefs to return reasonCounts instead of calling metrics internally
- Update receiveRouteMatcher to aggregate all validation errors and call UpdateInvalidRoute once

Signed-off-by: Veronika Volokitina <v.volokitinaa@gmail.com>

Author: vlktna

metrics/codahale.go

@@ -44,20 +44,23 @@ const (
 
 // CodaHale is the CodaHale format backend, implements Metrics interface in DropWizard's CodaHale metrics format.
 type CodaHale struct {
-	reg           metrics.Registry
-	createTimer   func() metrics.Timer
-	createCounter func() metrics.Counter
-	createGauge   func() metrics.GaugeFloat64
-	options       Options
-	handler       http.Handler
-	quit          chan struct{}
+	reg                 metrics.Registry
+	createTimer         func() metrics.Timer
+	createCounter       func() metrics.Counter
+	createGauge         func() metrics.GaugeFloat64
+	options             Options
+	handler             http.Handler
+	quit                chan struct{}
+	invalidRouteReasons map[string]bool
 }
 
 // NewCodaHale returns a new CodaHale backend of metrics.
 func NewCodaHale(o Options) *CodaHale {
 	o = applyCompatibilityDefaults(o)
 
-	c := &CodaHale{}
+	c := &CodaHale{
+		invalidRouteReasons: make(map[string]bool),
+	}
 
 	c.quit = make(chan struct{})
 	c.reg = metrics.NewRegistry()
@@ -266,8 +269,17 @@ func (c *CodaHale) IncErrorsStreaming(routeId string) {
 }
 
 func (c *CodaHale) UpdateInvalidRoute(reasonCounts map[string]int) {
+	// Set current counts
 	for reason, count := range reasonCounts {
 		c.UpdateGauge(fmt.Sprintf(KeyInvalidRoutes, reason), float64(count))
+		c.invalidRouteReasons[reason] = true
+	}
+
+	// Set previously seen reasons to 0 if they're not in current counts
+	for reason := range c.invalidRouteReasons {
+		if _, exists := reasonCounts[reason]; !exists {
+			c.UpdateGauge(fmt.Sprintf(KeyInvalidRoutes, reason), 0)
+		}
 	}
 }
 

metrics/codahale_test.go

@@ -622,3 +622,103 @@ func TestCodaHaleProxyLatencyMetrics(t *testing.T) {
 		})
 	}
 }
+
+func TestCodaHaleRouteValidationMetricsRealRoutingBehavior(t *testing.T) {
+	c := NewCodaHale(Options{})
+	defer c.Close()
+
+	getGaugeValue := func(reason string) float64 {
+		key := fmt.Sprintf(KeyInvalidRoutes, reason)
+		gauge := c.getGauge(key)
+		return gauge.Value()
+	}
+
+	// Phase 1: Initial state - no metrics present
+	t.Log("Phase 1: Initial state - no metrics should have values")
+
+	// Phase 2: Routes with validation errors (simulating receiveRouteMatcher)
+	t.Log("Phase 2: Routes with validation errors")
+	reasonCounts2 := map[string]int{
+		"unknown_filter":  2,
+		"invalid_matcher": 1,
+	}
+	c.UpdateInvalidRoute(reasonCounts2)
+
+	if got := getGaugeValue("unknown_filter"); got != 2 {
+		t.Errorf("Expected unknown_filter gauge to be 2, got %f", got)
+	}
+	if got := getGaugeValue("invalid_matcher"); got != 1 {
+		t.Errorf("Expected invalid_matcher gauge to be 1, got %f", got)
+	}
+
+	// Phase 3: Routes are fixed - receiveRouteMatcher calls UpdateInvalidRoute with fresh empty/nil map
+	t.Log("Phase 3: Routes are fixed - empty reasonCounts map")
+	reasonCounts3 := map[string]int{}
+	c.UpdateInvalidRoute(reasonCounts3)
+
+	if got := getGaugeValue("unknown_filter"); got != 0 {
+		t.Errorf("Expected unknown_filter gauge to be 0 after routes fixed, got %f", got)
+	}
+	if got := getGaugeValue("invalid_matcher"); got != 0 {
+		t.Errorf("Expected invalid_matcher gauge to be 0 after routes fixed, got %f", got)
+	}
+
+	t.Log("Key insight: CodaHale metrics are now properly set to 0 when routes are fixed")
+	t.Log("Previously seen invalid routes are tracked and reset to 0 when not present in new counts")
+}
+
+func TestCodaHaleRouteValidationMetricsLifecycleBehavior(t *testing.T) {
+	c := NewCodaHale(Options{})
+	defer c.Close()
+
+	getGaugeValue := func(reason string) float64 {
+		key := fmt.Sprintf(KeyInvalidRoutes, reason)
+		gauge := c.getGauge(key)
+		return gauge.Value()
+	}
+
+	t.Log("Scenario 1: Initial route errors")
+	c.UpdateInvalidRoute(map[string]int{
+		"unknown_filter":  3,
+		"invalid_matcher": 2,
+	})
+
+	if got := getGaugeValue("unknown_filter"); got != 3 {
+		t.Errorf("Expected unknown_filter gauge to be 3, got %f", got)
+	}
+	if got := getGaugeValue("invalid_matcher"); got != 2 {
+		t.Errorf("Expected invalid_matcher gauge to be 2, got %f", got)
+	}
+
+	t.Log("Scenario 2: Some routes fixed, some errors increase, new errors appear")
+	c.UpdateInvalidRoute(map[string]int{
+		"unknown_filter":    5,
+		"invalid_predicate": 1,
+	})
+
+	if got := getGaugeValue("unknown_filter"); got != 5 {
+		t.Errorf("Expected unknown_filter gauge to be 5, got %f", got)
+	}
+	if got := getGaugeValue("invalid_predicate"); got != 1 {
+		t.Errorf("Expected invalid_predicate gauge to be 1, got %f", got)
+	}
+	if got := getGaugeValue("invalid_matcher"); got != 0 {
+		t.Errorf("Expected invalid_matcher gauge to be 0 after being fixed, got %f", got)
+	}
+
+	t.Log("Scenario 3: All routes become valid")
+	c.UpdateInvalidRoute(map[string]int{})
+
+	if got := getGaugeValue("unknown_filter"); got != 0 {
+		t.Errorf("Expected unknown_filter gauge to be 0, got %f", got)
+	}
+	if got := getGaugeValue("invalid_predicate"); got != 0 {
+		t.Errorf("Expected invalid_predicate gauge to be 0, got %f", got)
+	}
+	if got := getGaugeValue("invalid_matcher"); got != 0 {
+		t.Errorf("Expected invalid_matcher gauge to be 0, got %f", got)
+	}
+
+	t.Log("Answer: When routes are fixed, CodaHale metrics are properly set to 0")
+	t.Log("This happens because UpdateInvalidRoute now tracks previously seen reasons and resets them")
+}

metrics/prometheus.go

@@ -72,6 +72,7 @@ type Prometheus struct {
 	customCounterM             *prometheus.CounterVec
 	customGaugeM               *prometheus.GaugeVec
 	invalidRouteM              *prometheus.GaugeVec
+	invalidRouteReasons        map[string]bool
 
 	opts     Options
 	registry *prometheus.Registry
@@ -83,8 +84,9 @@ func NewPrometheus(opts Options) *Prometheus {
 	opts = applyCompatibilityDefaults(opts)
 
 	p := &Prometheus{
-		registry: opts.PrometheusRegistry,
-		opts:     opts,
+		registry:            opts.PrometheusRegistry,
+		opts:                opts,
+		invalidRouteReasons: make(map[string]bool),
 	}
 
 	if p.registry == nil {
@@ -527,6 +529,14 @@ func (p *Prometheus) IncErrorsStreaming(routeID string) {
 func (p *Prometheus) UpdateInvalidRoute(reasonCounts map[string]int) {
 	for reason, count := range reasonCounts {
 		p.invalidRouteM.WithLabelValues(reason).Set(float64(count))
+		p.invalidRouteReasons[reason] = true
+	}
+
+	// Set previously seen reasons to 0 if they're not in current counts
+	for reason := range p.invalidRouteReasons {
+		if _, exists := reasonCounts[reason]; !exists {
+			p.invalidRouteM.WithLabelValues(reason).Set(0)
+		}
 	}
 }
 

metrics/prometheus_test.go

@@ -1302,3 +1302,104 @@ func TestPrometheusMetricsStartTimestamp(t *testing.T) {
 	checkMetric(`skipper_serve_host_count{code="201",host="bar_test",method="POST",start="(\d+)"} 2`)
 	checkMetric(`skipper_route_error_total{start="(\d+)"} 3`)
 }
+
+func TestRouteValidationMetricsRealRoutingBehavior(t *testing.T) {
+	pm := metrics.NewPrometheus(metrics.Options{})
+	path := "/metrics"
+
+	mux := http.NewServeMux()
+	pm.RegisterHandler(path, mux)
+
+	getMetricsOutput := func() string {
+		req := httptest.NewRequest("GET", path, nil)
+		w := httptest.NewRecorder()
+		mux.ServeHTTP(w, req)
+
+		resp := w.Result()
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+
+		body, err := io.ReadAll(resp.Body)
+		require.NoError(t, err)
+		return string(body)
+	}
+
+	t.Log("Phase 1: Initial state - no metrics present")
+	output1 := getMetricsOutput()
+	assert.NotContains(t, output1, `skipper_route_invalid{reason="unknown_filter"}`)
+	assert.NotContains(t, output1, `skipper_route_invalid{reason="invalid_matcher"}`)
+
+	t.Log("Phase 2: Routes with validation errors (simulating receiveRouteMatcher)")
+	reasonCounts2 := map[string]int{
+		"unknown_filter":  2,
+		"invalid_matcher": 1,
+	}
+	pm.UpdateInvalidRoute(reasonCounts2)
+
+	output2 := getMetricsOutput()
+	assert.Contains(t, output2, `skipper_route_invalid{reason="unknown_filter"} 2`)
+	assert.Contains(t, output2, `skipper_route_invalid{reason="invalid_matcher"} 1`)
+
+	t.Log("Phase 3: Routes are fixed - receiveRouteMatcher calls UpdateInvalidRoute with fresh empty/nil map")
+	reasonCounts3 := map[string]int{}
+	pm.UpdateInvalidRoute(reasonCounts3)
+
+	output3 := getMetricsOutput()
+	assert.Contains(t, output3, `skipper_route_invalid{reason="unknown_filter"} 0`)
+	assert.Contains(t, output3, `skipper_route_invalid{reason="invalid_matcher"} 0`)
+
+	t.Log("Key insight: Metrics are now properly set to 0 when routes are fixed")
+	t.Log("Previously seen invalid routes are tracked and reset to 0 when not present in new counts")
+}
+
+func TestRouteValidationMetricsLifecycleBehavior(t *testing.T) {
+	pm := metrics.NewPrometheus(metrics.Options{})
+	path := "/metrics"
+
+	mux := http.NewServeMux()
+	pm.RegisterHandler(path, mux)
+
+	getMetricsOutput := func() string {
+		req := httptest.NewRequest("GET", path, nil)
+		w := httptest.NewRecorder()
+		mux.ServeHTTP(w, req)
+
+		resp := w.Result()
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+
+		body, err := io.ReadAll(resp.Body)
+		require.NoError(t, err)
+		return string(body)
+	}
+
+	t.Log("Scenario 1: Initial route errors")
+	pm.UpdateInvalidRoute(map[string]int{
+		"unknown_filter":  3,
+		"invalid_matcher": 2,
+	})
+
+	output1 := getMetricsOutput()
+	assert.Contains(t, output1, `skipper_route_invalid{reason="unknown_filter"} 3`)
+	assert.Contains(t, output1, `skipper_route_invalid{reason="invalid_matcher"} 2`)
+
+	t.Log("Scenario 2: Some routes fixed, some errors increase, new errors appear")
+	pm.UpdateInvalidRoute(map[string]int{
+		"unknown_filter":    5,
+		"invalid_predicate": 1,
+	})
+
+	output2 := getMetricsOutput()
+	assert.Contains(t, output2, `skipper_route_invalid{reason="unknown_filter"} 5`)
+	assert.Contains(t, output2, `skipper_route_invalid{reason="invalid_predicate"} 1`)
+	assert.Contains(t, output2, `skipper_route_invalid{reason="invalid_matcher"} 0`)
+
+	t.Log("Scenario 3: All routes become valid")
+	pm.UpdateInvalidRoute(map[string]int{})
+
+	output3 := getMetricsOutput()
+	assert.Contains(t, output3, `skipper_route_invalid{reason="unknown_filter"} 0`)
+	assert.Contains(t, output3, `skipper_route_invalid{reason="invalid_predicate"} 0`)
+	assert.Contains(t, output3, `skipper_route_invalid{reason="invalid_matcher"} 0`)
+
+	t.Log("Answer: When routes are fixed, metrics are properly set to 0")
+	t.Log("This happens because UpdateInvalidRoute now tracks previously seen reasons and resets them")
+}

routing/datasource.go

@@ -524,10 +524,8 @@ func mapPredicates(cps []PredicateSpec) map[string]PredicateSpec {
 }
 
 // processes a set of route definitions for the routing table
-func processRouteDefs(o *Options, defs []*eskip.Route) (routes []*Route, invalidDefs []*eskip.Route) {
+func processRouteDefs(o *Options, defs []*eskip.Route) (routes []*Route, invalidDefs []*eskip.Route, reasonCounts map[string]int) {
 	cpm := mapPredicates(o.Predicates)
-	reasonCounts := make(map[string]int)
-
 	for _, def := range defs {
 		route, err := processRouteDef(o, cpm, def)
 		if err == nil {
@@ -545,11 +543,6 @@ func processRouteDefs(o *Options, defs []*eskip.Route) (routes []*Route, invalid
 			reasonCounts[reason]++
 		}
 	}
-
-	if o.Metrics != nil {
-		o.Metrics.UpdateInvalidRoute(reasonCounts)
-	}
-
 	return
 }
 
@@ -602,7 +595,7 @@ func receiveRouteMatcher(o Options, out chan<- *routeTable, quit <-chan struct{}
 				defs = o.PreProcessors[i].Do(defs)
 			}
 
-			routes, invalidRoutes := processRouteDefs(&o, defs)
+			routes, invalidRoutes, reasonCounts := processRouteDefs(&o, defs)
 
 			for i := range o.PostProcessors {
 				routes = o.PostProcessors[i].Do(routes)
@@ -618,8 +611,12 @@ func receiveRouteMatcher(o Options, out chan<- *routeTable, quit <-chan struct{}
 				invalidRouteIds[err.ID] = struct{}{}
 			}
 
+			if len(errs) > 0 {
+				reasonCounts[errInvalidMatcher.Code()] = len(errs)
+			}
+
 			if o.Metrics != nil {
-				o.Metrics.UpdateInvalidRoute(map[string]int{errInvalidMatcher.Code(): len(errs)})
+				o.Metrics.UpdateInvalidRoute(reasonCounts)
 			}
 
 			for i := range routes {

routing/matcher_test.go

@@ -101,7 +101,7 @@ func docToRoutes(doc string) ([]*Route, error) {
 	if err != nil {
 		return nil, err
 	}
-	routes, _ := processRouteDefs(&Options{Predicates: []PredicateSpec{&truePredicate{}}}, defs)
+	routes, _, _ := processRouteDefs(&Options{Predicates: []PredicateSpec{&truePredicate{}}}, defs)
 	return routes, nil
 }
 
@@ -193,7 +193,7 @@ func generateRoutes(paths []string) []*Route {
 		defs[i] = &eskip.Route{Id: fmt.Sprintf("route%d", i), Path: p, Backend: p}
 	}
 
-	routes, _ := processRouteDefs(&Options{}, defs)
+	routes, _, _ := processRouteDefs(&Options{}, defs)
 	return routes
 }