metrics: remove invalid routes deletion

metrics/all_kind.go

@@ -136,11 +136,6 @@ func (a *All) SetInvalidRoute(routeId, reason string) {
 	a.codaHale.SetInvalidRoute(routeId, reason)
 }
 
-func (a *All) DeleteInvalidRoute(routeId string) {
-	a.prometheus.DeleteInvalidRoute(routeId)
-	a.codaHale.DeleteInvalidRoute(routeId)
-}
-
 func (a *All) Close() {
 	a.codaHale.Close()
 	a.prometheus.Close()

metrics/codahale.go

@@ -6,7 +6,6 @@ import (
 	"net/http"
 	"path"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/rcrowley/go-metrics"
@@ -45,15 +44,13 @@ 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{}
-	invalidRoutesMu sync.RWMutex
-	invalidRoutes   map[string]string // routeId -> metric key
+	reg           metrics.Registry
+	createTimer   func() metrics.Timer
+	createCounter func() metrics.Counter
+	createGauge   func() metrics.GaugeFloat64
+	options       Options
+	handler       http.Handler
+	quit          chan struct{}
 }
 
 // NewCodaHale returns a new CodaHale backend of metrics.
@@ -64,7 +61,6 @@ func NewCodaHale(o Options) *CodaHale {
 
 	c.quit = make(chan struct{})
 	c.reg = metrics.NewRegistry()
-	c.invalidRoutes = make(map[string]string)
 
 	var createSample func() metrics.Sample
 	if o.UseExpDecaySample {
@@ -270,24 +266,10 @@ func (c *CodaHale) IncErrorsStreaming(routeId string) {
 }
 
 func (c *CodaHale) SetInvalidRoute(routeId, reason string) {
-	c.invalidRoutesMu.Lock()
-	defer c.invalidRoutesMu.Unlock()
-
 	key := fmt.Sprintf(KeyInvalidRoutes, routeId, reason)
-	c.invalidRoutes[routeId] = key
 	c.UpdateGauge(key, 1)
 }
 
-func (c *CodaHale) DeleteInvalidRoute(routeId string) {
-	c.invalidRoutesMu.Lock()
-	defer c.invalidRoutesMu.Unlock()
-
-	if key, exists := c.invalidRoutes[routeId]; exists {
-		c.UpdateGauge(key, 0)
-		delete(c.invalidRoutes, routeId)
-	}
-}
-
 func (c *CodaHale) Close() {
 	close(c.quit)
 }

metrics/metrics.go

@@ -81,7 +81,6 @@ type Metrics interface {
 	RegisterHandler(path string, handler *http.ServeMux)
 	UpdateGauge(key string, value float64)
 	SetInvalidRoute(routeId, reason string)
-	DeleteInvalidRoute(routeId string)
 	Close()
 }
 

metrics/metricstest/metricsmock.go

@@ -5,7 +5,6 @@ package metricstest
 import (
 	"fmt"
 	"net/http"
-	"strings"
 	"sync"
 	"time"
 )
@@ -230,15 +229,4 @@ func (m *MockMetrics) SetInvalidRoute(routeID, reason string) {
 	m.UpdateGauge(key, 1)
 }
 
-func (m *MockMetrics) DeleteInvalidRoute(routeID string) {
-	m.WithGauges(func(gauges map[string]float64) {
-		prefix := fmt.Sprintf("route.invalid.%s.", routeID)
-		for key := range gauges {
-			if strings.HasPrefix(key, prefix) {
-				gauges[key] = 0
-			}
-		}
-	})
-}
-
 func (*MockMetrics) Close() {}

metrics/metricstest/metricsmock_test.go

@@ -165,7 +165,7 @@ func TestMockMetrics(t *testing.T) {
 		}
 	})
 
-	t.Run("test-measure-backend-request-header", func(t *testing.T) {
+	t.Run("test-set-invalid-route", func(t *testing.T) {
 		routeID := "my-route"
 		reason := "foo"
 		key := fmt.Sprintf("route.invalid.%s..%s", routeID, reason)
@@ -177,13 +177,6 @@ func TestMockMetrics(t *testing.T) {
 		} else if f != 1 {
 			t.Fatalf("Failed to get the right value after inc: %0.2f", f)
 		}
-
-		m.DeleteInvalidRoute(routeID)
-		if f, ok := m.gauges[key]; !ok {
-			t.Fatalf("Failed to find value %q for routeID %q", key, routeID)
-		} else if f != 0 {
-			t.Fatalf("Failed to get the right value after inc: %0.2f", f)
-		}
 	})
 
 	t.Run("test-gauge", func(t *testing.T) {

metrics/prometheus.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"net/http"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -74,10 +73,6 @@ type Prometheus struct {
 	customGaugeM               *prometheus.GaugeVec
 	invalidRouteM              *prometheus.GaugeVec
 
-	// Track invalid route metrics for proper cleanup
-	invalidRoutesMu    sync.RWMutex
-	invalidRouteLabels map[string][]prometheus.Labels // routeId -> []labels
-
 	opts     Options
 	registry *prometheus.Registry
 	handler  http.Handler
@@ -88,9 +83,8 @@ func NewPrometheus(opts Options) *Prometheus {
 	opts = applyCompatibilityDefaults(opts)
 
 	p := &Prometheus{
-		registry:           opts.PrometheusRegistry,
-		opts:               opts,
-		invalidRouteLabels: make(map[string][]prometheus.Labels),
+		registry: opts.PrometheusRegistry,
+		opts:     opts,
 	}
 
 	if p.registry == nil {
@@ -537,25 +531,6 @@ func (p *Prometheus) IncErrorsStreaming(routeID string) {
 // SetInvalidRoute satisfies Metrics interface.
 func (p *Prometheus) SetInvalidRoute(routeId, reason string) {
 	p.invalidRouteM.WithLabelValues(routeId, reason).Set(1)
-
-	p.invalidRoutesMu.Lock()
-	defer p.invalidRoutesMu.Unlock()
-
-	labels := prometheus.Labels{"route_id": routeId, "reason": reason}
-	p.invalidRouteLabels[routeId] = append(p.invalidRouteLabels[routeId], labels)
-}
-
-// DeleteInvalidRoute satisfies Metrics interface.
-func (p *Prometheus) DeleteInvalidRoute(routeId string) {
-	p.invalidRoutesMu.Lock()
-	defer p.invalidRoutesMu.Unlock()
-
-	if labels, exists := p.invalidRouteLabels[routeId]; exists {
-		for _, labelSet := range labels {
-			p.invalidRouteM.With(labelSet).Set(0)
-		}
-		delete(p.invalidRouteLabels, routeId)
-	}
 }
 
 func (p *Prometheus) Close() {}

routing/datasource.go

@@ -531,9 +531,6 @@ func processRouteDefs(o *Options, defs []*eskip.Route) (routes []*Route, invalid
 		route, err := processRouteDef(o, cpm, def)
 		if err == nil {
 			routes = append(routes, route)
-			if o.Metrics != nil {
-				o.Metrics.DeleteInvalidRoute(def.Id)
-			}
 		} else {
 			invalidDefs = append(invalidDefs, def)
 			o.Log.Errorf("failed to process route %s: %v", def.Id, err)

routing/datasource_test.go

@@ -594,21 +594,21 @@ func waitForIndividualRouteMetrics(t *testing.T, metrics *metricstest.MockMetric
 	}
 }
 
-func TestRouteMetricsSetToZeroWhenFixed(t *testing.T) {
+func TestRouteMetricsStaySetWhenRouteIsFixed(t *testing.T) {
 	t.Run("MockMetrics", func(t *testing.T) {
 		synctest.Test(t, func(t *testing.T) {
-			testRouteMetricsSetToZeroWhenFixedWithMock(t)
+			testRouteMetricsStaySetWhenRouteIsFixedWithMock(t)
 		})
 	})
 
 	t.Run("Prometheus", func(t *testing.T) {
 		synctest.Test(t, func(t *testing.T) {
-			testRouteMetricsSetToZeroWhenFixedWithPrometheus(t)
+			testRouteMetricsStaySetWhenRouteIsFixedWithPrometheus(t)
 		})
 	})
 }
 
-func testRouteMetricsSetToZeroWhenFixedWithMock(t *testing.T) {
+func testRouteMetricsStaySetWhenRouteIsFixedWithMock(t *testing.T) {
 	metrics := &metricstest.MockMetrics{}
 
 	// Start with an invalid route
@@ -649,16 +649,16 @@ func testRouteMetricsSetToZeroWhenFixedWithMock(t *testing.T) {
 	// Wait for the update to be processed
 	time.Sleep(50 * time.Millisecond)
 
-	// Verify the metric is now set to 0 (not deleted)
+	// Verify the metric remains set (not deleted or zeroed out)
 	metrics.WithGauges(func(gauges map[string]float64) {
 		expectedKey := "route.invalid.invalidBackend1..failed_backend_split"
-		if gauges[expectedKey] != 0 {
-			t.Errorf("Expected invalid route metric to be set to 0 when route becomes valid, got %f", gauges[expectedKey])
+		if gauges[expectedKey] != 1 {
+			t.Errorf("Expected invalid route metric to stay set when route becomes valid, got %f", gauges[expectedKey])
 		}
 	})
 }
 
-func testRouteMetricsSetToZeroWhenFixedWithPrometheus(t *testing.T) {
+func testRouteMetricsStaySetWhenRouteIsFixedWithPrometheus(t *testing.T) {
 	pm := metrics.NewPrometheus(metrics.Options{})
 	path := "/metrics"
 
@@ -708,16 +708,16 @@ func testRouteMetricsSetToZeroWhenFixedWithPrometheus(t *testing.T) {
 	// Wait for the update to be processed
 	time.Sleep(50 * time.Millisecond)
 
-	// Check that the metric is now set to 0
+	// Check that the metric stays set (not deleted or zeroed out)
 	req = httptest.NewRequest("GET", path, nil)
 	w = httptest.NewRecorder()
 	mux.ServeHTTP(w, req)
 	body, _ = io.ReadAll(w.Result().Body)
 	output = string(body)
 
-	expectedZeroLine := `skipper_route_invalid{reason="failed_backend_split",route_id="invalidBackend1"} 0`
-	if !strings.Contains(output, expectedZeroLine) {
-		t.Errorf("Expected to find invalid route metric set to 0 when route becomes valid")
+	expectedLine = `skipper_route_invalid{reason="failed_backend_split",route_id="invalidBackend1"} 1`
+	if !strings.Contains(output, expectedLine) {
+		t.Errorf("Expected invalid route metric to remain set when route becomes valid")
 		t.Logf("Output: %s", output)
 	}
 }

routing/errors.go

@@ -30,7 +30,6 @@ func WrapInvalidDefinitionReason(reason string, err error) error {
 
 func HandleValidationError(mtr metrics.Metrics, err error, routeId string) error {
 	if err == nil {
-		mtr.DeleteInvalidRoute(routeId)
 		return nil
 	}
 

routing/errors_test.go

@@ -2,6 +2,7 @@ package routing
 
 import (
 	"errors"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -87,10 +88,9 @@ func TestValidationErrorsNoError(t *testing.T) {
 	}
 
 	mtr.WithGauges(func(g map[string]float64) {
-		for k, v := range g {
-			if strings.HasPrefix(k, "route.invalid") && v > 0 {
-				t.Fatalf("Failed to have no invalid routes %q -> %0.2f", k, v)
-			}
+		key := fmt.Sprintf("route.invalid.%s..%s", routeID, errInvalidPredicateParams)
+		if v := g[key]; v != 1 {
+			t.Fatalf("Invalid route metric should remain set, got %0.2f", v)
 		}
 	})
 }