refactor: Improve endpoints_per_pool metric reporting and cleanup

Rename DeleteEndpointsPerPool to UncaptureEndpointsPerPool, add metric
cleanup on route pruning and LB algorithm changes, and add tests.

Author: hoffmaen

src/code.cloudfoundry.org/gorouter/metrics/compositereporter.go

@@ -48,7 +48,7 @@ type MetricReporter interface {
 	CaptureNATSDroppedMessages(messages int)
 	UnmuzzleRouteRegistrationLatency()
 	CaptureEndpointsPerPool(endpoints int, route string, lbAlgo string)
-	DeleteEndpointsPerPool(route, lbAlgo string)
+	UncaptureEndpointsPerPool(route, lbAlgo string)
 }
 
 type ComponentTagged interface {
@@ -238,9 +238,9 @@ func (m MultiMetricReporter) CaptureEndpointsPerPool(endpoints int, route string
 	}
 }
 
-func (m MultiMetricReporter) DeleteEndpointsPerPool(route, lbAlgo string) {
+func (m MultiMetricReporter) UncaptureEndpointsPerPool(route, lbAlgo string) {
 	for _, r := range m {
-		r.DeleteEndpointsPerPool(route, lbAlgo)
+		r.UncaptureEndpointsPerPool(route, lbAlgo)
 	}
 }
 
@@ -276,6 +276,6 @@ func (c *CompositeReporter) CaptureEndpointsPerPool(endpoints int, route string,
 	c.MetricReporter.CaptureEndpointsPerPool(endpoints, route, lbAlgo)
 }
 
-func (c *CompositeReporter) DeleteEndpointsPerPool(route, lbAlgo string) {
-	c.MetricReporter.DeleteEndpointsPerPool(route, lbAlgo)
+func (c *CompositeReporter) UncaptureEndpointsPerPool(route, lbAlgo string) {
+	c.MetricReporter.UncaptureEndpointsPerPool(route, lbAlgo)
 }

src/code.cloudfoundry.org/gorouter/metrics/fakes/fake_metricreporter.go

@@ -206,7 +206,7 @@ func (m *Metrics) CaptureEndpointsPerPool(endpoints int, route string, lbAlgo st
 }
 
 // Empty implementation here is to fulfil interface
-func (m *Metrics) DeleteEndpointsPerPool(route string, lbAlgo string) {
+func (m *Metrics) UncaptureEndpointsPerPool(route string, lbAlgo string) {
 }
 
 func getResponseCounterName(statusCode int) string {

src/code.cloudfoundry.org/gorouter/metrics/fakes/fake_subscriber.go

@@ -92,7 +92,7 @@ func NewMetrics(registry *mr.Registry, perRequestMetricsReporting bool) *Metrics
 		NATSDroppedMessages:         registry.NewGauge("total_dropped_messages", "number of total dropped messages in NATS"),
 		HTTPLatency:                 registry.NewGaugeVec("http_latency_seconds", "the latency of http requests from gorouter and back in sec", []string{"source_id"}),
 		perRequestMetricsReporting:  perRequestMetricsReporting,
-		EndpointsPerPool:            registry.NewGaugeVec("endpoints_per_pool", "number of endpoints per pool", []string{"route", "LB_algorithm"}),
+		EndpointsPerPool:            registry.NewGaugeVec("endpoints_per_pool", "number of endpoints per pool", []string{"route", "lb_algorithm"}),
 	}
 }
 
@@ -222,12 +222,12 @@ func (metrics *Metrics) CaptureHTTPLatency(d time.Duration, sourceID string) {
 	metrics.HTTPLatency.Set(float64(d)/float64(time.Second), []string{sourceID})
 }
 
-// CaptureEndpoints sets the number of endpoints for a given route and load balancing algorithm
+// CaptureEndpointsPerPool sets the number of endpoints for a given route and load balancing algorithm
 func (metrics *Metrics) CaptureEndpointsPerPool(count int, route string, loadBalancingAlgo string) {
 	metrics.EndpointsPerPool.Set(float64(count), []string{route, loadBalancingAlgo})
 }
 
-func (metrics *Metrics) DeleteEndpointsPerPool(route string, lbAlgo string) {
+func (metrics *Metrics) UncaptureEndpointsPerPool(route string, lbAlgo string) {
 	metrics.EndpointsPerPool.Delete([]string{route, lbAlgo})
 }
 

src/code.cloudfoundry.org/gorouter/metrics/fakes/fake_varzreporter.go

@@ -461,41 +461,41 @@ var _ = Describe("Metrics", func() {
 		It("reports the number of endpoints per pool with correct labels", func() {
 			m.CaptureEndpointsPerPool(5, "routeA", "round_robin")
 			metricsOutput := getMetrics(r.Port())
-			expected := "endpoints_per_pool{LB_algorithm=\"round_robin\",route=\"routeA\"} 5"
+			expected := "endpoints_per_pool{lb_algorithm=\"round_robin\",route=\"routeA\"} 5"
 			Expect(metricsOutput).To(ContainSubstring(expected))
 		})
 
 		It("updates the value for the same label combination", func() {
 			m.CaptureEndpointsPerPool(5, "routeA", "round_robin")
 			m.CaptureEndpointsPerPool(7, "routeA", "round_robin")
 			metricsOutput := getMetrics(r.Port())
-			expected := "endpoints_per_pool{LB_algorithm=\"round_robin\",route=\"routeA\"} 7"
+			expected := "endpoints_per_pool{lb_algorithm=\"round_robin\",route=\"routeA\"} 7"
 			Expect(metricsOutput).To(ContainSubstring(expected))
 		})
 
 		It("reports multiple values for different label combinations", func() {
 			m.CaptureEndpointsPerPool(5, "routeA", "round_robin")
 			m.CaptureEndpointsPerPool(3, "routeB", "least_conn")
 			metricsOutput := getMetrics(r.Port())
-			expectedA := "endpoints_per_pool{LB_algorithm=\"round_robin\",route=\"routeA\"} 5"
-			expectedB := "endpoints_per_pool{LB_algorithm=\"least_conn\",route=\"routeB\"} 3"
+			expectedA := "endpoints_per_pool{lb_algorithm=\"round_robin\",route=\"routeA\"} 5"
+			expectedB := "endpoints_per_pool{lb_algorithm=\"least_conn\",route=\"routeB\"} 3"
 			Expect(metricsOutput).To(ContainSubstring(expectedA))
 			Expect(metricsOutput).To(ContainSubstring(expectedB))
 		})
 
 		It("deletes the metric for a given route and LB algorithm", func() {
 			m.CaptureEndpointsPerPool(5, "routeA", "round_robin")
-			Expect(getMetrics(r.Port())).To(ContainSubstring("endpoints_per_pool{LB_algorithm=\"round_robin\",route=\"routeA\"} 5"))
+			Expect(getMetrics(r.Port())).To(ContainSubstring("endpoints_per_pool{lb_algorithm=\"round_robin\",route=\"routeA\"} 5"))
 
-			m.DeleteEndpointsPerPool("routeA", "round_robin")
-			Expect(getMetrics(r.Port())).NotTo(ContainSubstring("endpoints_per_pool{LB_algorithm=\"round_robin\",route=\"routeA\"}"))
+			m.UncaptureEndpointsPerPool("routeA", "round_robin")
+			Expect(getMetrics(r.Port())).NotTo(ContainSubstring("endpoints_per_pool{lb_algorithm=\"round_robin\",route=\"routeA\"}"))
 		})
 
 		It("does nothing when deleting a non-existent label combination", func() {
 			m.CaptureEndpointsPerPool(5, "routeA", "round_robin")
 
-			m.DeleteEndpointsPerPool("routeX", "round_robin")
-			Expect(getMetrics(r.Port())).To(ContainSubstring("endpoints_per_pool{LB_algorithm=\"round_robin\",route=\"routeA\"} 5"))
+			m.UncaptureEndpointsPerPool("routeX", "round_robin")
+			Expect(getMetrics(r.Port())).To(ContainSubstring("endpoints_per_pool{lb_algorithm=\"round_robin\",route=\"routeA\"} 5"))
 		})
 	})
 })

src/code.cloudfoundry.org/gorouter/metrics/metricsreporter.go

@@ -104,7 +104,6 @@ func (r *RouteRegistry) Register(uri route.Uri, endpoint *route.Endpoint) {
 	}
 
 	switch poolPutResult {
-
 	case route.EndpointAdded:
 		if r.logger.Enabled(context.Background(), slog.LevelInfo) {
 			r.logger.Info("endpoint-registered", buildSlogAttrs(uri, endpoint)...)
@@ -424,6 +423,8 @@ func (r *RouteRegistry) pruneStaleDroplets() {
 
 	r.byURI.EachNodeWithPool(func(t *container.Trie) {
 		endpoints := t.Pool.PruneEndpoints()
+		lbAlgo := t.Pool.LoadBalancingAlgorithm
+		uri := t.ToPath()
 		if r.EmptyPoolResponseCode503 && r.EmptyPoolTimeout > 0 {
 			if time.Since(t.Pool.LastUpdated()) > r.EmptyPoolTimeout {
 				t.Snip()
@@ -442,11 +443,19 @@ func (r *RouteRegistry) pruneStaleDroplets() {
 				isolationSegment = "-"
 			}
 			r.logger.Info("pruned-route",
-				slog.String("uri", t.ToPath()),
+				slog.String("uri", uri),
 				slog.Any("endpoints", addresses),
 				slog.String("isolation_segment", isolationSegment),
 			)
 			r.reporter.CaptureRoutesPruned(uint64(len(endpoints)))
+
+			if lbAlgo == config.LOAD_BALANCE_HB {
+				if t.Pool == nil || t.Pool.NumEndpoints() == 0 {
+					r.reporter.UncaptureEndpointsPerPool(uri, config.LOAD_BALANCE_HB)
+				} else {
+					r.reporter.CaptureEndpointsPerPool(t.Pool.NumEndpoints(), uri, config.LOAD_BALANCE_HB)
+				}
+			}
 		}
 	})
 }
@@ -465,17 +474,23 @@ func (r *RouteRegistry) freshenRoutes() {
 	})
 }
 
+// reportEndpointsPerPool reports the endpoints_per_pool metric for hash-based (HB) route pools.
+// For non-HB endpoints, it deletes any stale HB metric entries (e.g. after switching away from HB).
 func (r *RouteRegistry) reportEndpointsPerPool(uri route.Uri, endpoint *route.Endpoint) {
-	if endpoint.LoadBalancingAlgorithm == config.LOAD_BALANCE_HB {
-		pool := r.byURI.Find(uri.RouteKey())
-		if pool == nil || pool.NumEndpoints() == 0 {
-			r.reporter.DeleteEndpointsPerPool(string(uri), config.LOAD_BALANCE_HB)
-			return
-		}
-		r.reporter.CaptureEndpointsPerPool(pool.NumEndpoints(), string(uri), config.LOAD_BALANCE_HB)
+	if endpoint.LoadBalancingAlgorithm != config.LOAD_BALANCE_HB {
+		r.reporter.UncaptureEndpointsPerPool(string(uri), config.LOAD_BALANCE_HB)
+		return
+	}
+
+	r.RLock()
+	pool := r.byURI.Find(uri.RouteKey())
+	r.RUnlock()
+
+	if pool == nil || pool.NumEndpoints() == 0 {
+		r.reporter.UncaptureEndpointsPerPool(string(uri), config.LOAD_BALANCE_HB)
+		return
 	}
-	// Delete stale entries for routes that have switched from HB to another load balancing algorithm.
-	r.reporter.DeleteEndpointsPerPool(uri.String(), config.LOAD_BALANCE_HB)
+	r.reporter.CaptureEndpointsPerPool(pool.NumEndpoints(), string(uri), config.LOAD_BALANCE_HB)
 }
 
 func splitHostAndContextPath(uri route.Uri) (string, string) {