fix(metrics): cap distinct HTTP path labels per container

common/path_limiter.go

@@ -0,0 +1,92 @@
+package common
+
+import (
+	"sync"
+
+	"github.com/coroot/coroot-node-agent/flags"
+	"k8s.io/klog/v2"
+)
+
+// OtherPath is the bucket every path beyond the per-container cap collapses
+// into. Requests are still counted, they just stop minting new series.
+const OtherPath = "{other}"
+
+// PathLimiter bounds the number of distinct HTTP `path` label values a single
+// container may contribute to container_http_requests_total.
+//
+// `path` is the only L7 label an outside party controls. An internet-facing
+// ingress is probed for vulnerabilities around the clock, and every unique junk
+// path (/axds.php, /HNAP1, ...) becomes a permanent series. Normalization cannot
+// collapse them because they are genuinely distinct literals, so the series count
+// grows without bound for as long as the scanners keep scanning.
+//
+// Paths seen before the cap is reached always pass through, so a steady
+// application keeps reporting its real routes; only paths first observed after
+// the cap fills are folded into OtherPath. Real applications serve far fewer
+// routes than the default cap, which should only ever engage on scanner traffic.
+//
+// The zero value is not usable; construct with NewPathLimiter.
+type PathLimiter struct {
+	mu   sync.RWMutex
+	seen map[string]struct{}
+	full bool // latched once the cap is hit, so the hot path skips the map entirely
+
+	owner string // container_id, for the one-shot warning
+}
+
+func NewPathLimiter(owner string) *PathLimiter {
+	return &PathLimiter{seen: make(map[string]struct{}), owner: owner}
+}
+
+// Limit returns path if it may be reported as-is, or OtherPath if admitting it
+// would push this container past flags.MaxHttpPathsPerContainer. Callers should
+// normalize the path first so that a parameterized route consumes one slot
+// rather than one per parameter value.
+// A nil receiver is tolerated (returns path unchanged) so that an L7Stats built
+// as a zero value rather than through its constructor degrades to the old
+// unbounded behaviour instead of panicking in the L7 hot path.
+func (l *PathLimiter) Limit(path string) string {
+	limit := flags.GetInt(flags.MaxHttpPathsPerContainer)
+	// An empty path is what non-HTTP protocols and invalid-UTF8 requests report;
+	// it must not consume a slot.
+	if l == nil || limit <= 0 || path == "" {
+		return path
+	}
+
+	l.mu.RLock()
+	_, seen := l.seen[path]
+	full := l.full
+	l.mu.RUnlock()
+	if seen {
+		return path
+	}
+	if full {
+		return OtherPath
+	}
+
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	// Re-check: another request may have admitted this path, or filled the cap,
+	// while we waited for the write lock.
+	if _, seen := l.seen[path]; seen {
+		return path
+	}
+	if len(l.seen) >= limit {
+		if !l.full {
+			l.full = true
+			klog.Warningf("HTTP path cardinality cap (%d) reached for %s, further unseen paths reported as %s",
+				limit, l.owner, OtherPath)
+		}
+		return OtherPath
+	}
+	l.seen[path] = struct{}{}
+	return path
+}
+
+// Len reports how many distinct paths have been admitted. For tests and
+// diagnostics.
+func (l *PathLimiter) Len() int {
+	l.mu.RLock()
+	defer l.mu.RUnlock()
+	return len(l.seen)
+}

common/path_limiter_test.go

@@ -0,0 +1,121 @@
+package common
+
+import (
+	"fmt"
+	"sync"
+	"testing"
+
+	"github.com/coroot/coroot-node-agent/flags"
+)
+
+// withPathCap sets the cap for the duration of a test. Flags are not parsed in
+// test binaries (flags.init bails out on the .test suffix), so the pointer holds
+// a zero value rather than the declared default.
+func withPathCap(t *testing.T, limit int) {
+	t.Helper()
+	prev := *flags.MaxHttpPathsPerContainer
+	*flags.MaxHttpPathsPerContainer = limit
+	t.Cleanup(func() { *flags.MaxHttpPathsPerContainer = prev })
+}
+
+func TestPathLimiterAdmitsUpToCapThenCollapses(t *testing.T) {
+	withPathCap(t, 3)
+	l := NewPathLimiter("/k8s/test/pod/app")
+
+	for _, p := range []string{"/a", "/b", "/c"} {
+		if got := l.Limit(p); got != p {
+			t.Fatalf("Limit(%q) = %q, want it admitted", p, got)
+		}
+	}
+
+	if got := l.Limit("/d"); got != OtherPath {
+		t.Errorf("Limit(%q) = %q, want %q once the cap is reached", "/d", got, OtherPath)
+	}
+
+	// Paths admitted before the cap keep reporting under their real value — a
+	// steady application must not lose its routes to scanner traffic that arrives
+	// later.
+	if got := l.Limit("/b"); got != "/b" {
+		t.Errorf("Limit(%q) = %q, want the already-admitted path", "/b", got)
+	}
+}
+
+func TestPathLimiterUnlimitedWhenCapIsZero(t *testing.T) {
+	withPathCap(t, 0)
+	l := NewPathLimiter("/k8s/test/pod/app")
+
+	for i := 0; i < 500; i++ {
+		p := fmt.Sprintf("/p%d", i)
+		if got := l.Limit(p); got != p {
+			t.Fatalf("Limit(%q) = %q, want no cap when the flag is 0", p, got)
+		}
+	}
+}
+
+func TestPathLimiterPassesEmptyPathThrough(t *testing.T) {
+	withPathCap(t, 1)
+	l := NewPathLimiter("/k8s/test/pod/app")
+
+	// Non-HTTP protocols and invalid-UTF8 requests report "", which must neither
+	// consume a cap slot nor be rewritten to OtherPath.
+	for i := 0; i < 10; i++ {
+		if got := l.Limit(""); got != "" {
+			t.Fatalf("Limit(\"\") = %q, want \"\"", got)
+		}
+	}
+	if got := l.Limit("/real"); got != "/real" {
+		t.Errorf("Limit(%q) = %q, want it admitted — empty paths must not fill the cap", "/real", got)
+	}
+}
+
+func TestPathLimiterIsBoundedUnderConcurrency(t *testing.T) {
+	const cap = 50
+	withPathCap(t, cap)
+	l := NewPathLimiter("/k8s/test/pod/app")
+
+	var wg sync.WaitGroup
+	for w := 0; w < 8; w++ {
+		wg.Add(1)
+		go func(worker int) {
+			defer wg.Done()
+			for i := 0; i < 200; i++ {
+				l.Limit(fmt.Sprintf("/w%d/p%d", worker, i))
+			}
+		}(w)
+	}
+	wg.Wait()
+
+	// The whole point of the cap: concurrent admission must never overshoot it,
+	// otherwise the series bound is not actually a bound.
+	if n := l.Len(); n > cap {
+		t.Errorf("admitted %d distinct paths, want at most %d", n, cap)
+	}
+}
+
+func TestPathLimiterNilReceiverIsSafe(t *testing.T) {
+	withPathCap(t, 1)
+	var l *PathLimiter
+
+	// Limit runs on every HTTP request; a zero-value L7Stats must degrade to
+	// unbounded rather than panic.
+	if got := l.Limit("/a"); got != "/a" {
+		t.Errorf("(*PathLimiter)(nil).Limit(%q) = %q, want it returned unchanged", "/a", got)
+	}
+	if got := l.Limit("/b"); got != "/b" {
+		t.Errorf("(*PathLimiter)(nil).Limit(%q) = %q, want it returned unchanged", "/b", got)
+	}
+}
+
+func TestPathLimiterIsPerInstance(t *testing.T) {
+	withPathCap(t, 1)
+	a, b := NewPathLimiter("container-a"), NewPathLimiter("container-b")
+
+	// One noisy container filling its cap must not silence another's routes.
+	a.Limit("/only")
+	if got := a.Limit("/second"); got != OtherPath {
+		t.Errorf("a.Limit(%q) = %q, want %q", "/second", got, OtherPath)
+	}
+	if got := b.Limit("/second"); got != "/second" {
+		t.Errorf("b.Limit(%q) = %q, want it admitted on an independent limiter", "/second", got)
+	}
+}

containers/l7.go

@@ -106,6 +106,11 @@ type L7Stats struct {
 	latency         map[l7.Protocol]*prometheus.HistogramVec
 	initialized     map[l7.Protocol]bool
 	promConstLabels prometheus.Labels // container_id, app_id, machine_id, system_uuid, az, region
+
+	// pathLimiter caps the distinct `path` label values this container can emit.
+	// Scanner traffic against a public ingress otherwise grows the series count
+	// without bound; see common.PathLimiter.
+	pathLimiter *common.PathLimiter
 }
 
 func NewL7Stats(constLabels prometheus.Labels) L7Stats {
@@ -114,6 +119,7 @@ func NewL7Stats(constLabels prometheus.Labels) L7Stats {
 		latency:         make(map[l7.Protocol]*prometheus.HistogramVec),
 		initialized:     make(map[l7.Protocol]bool),
 		promConstLabels: constLabels,
+		pathLimiter:     common.NewPathLimiter(constLabels["container_id"]),
 	}
 }
 
@@ -155,7 +161,7 @@ func (s *L7Stats) observe(protocol l7.Protocol, status, method, path string, dur
 		counterLabelValues = append(counterLabelValues, labelInterner.intern(method))
 	case l7.ProtocolHTTP:
 		if ValidUtf8([]byte(path)) {
-			counterLabelValues = append(counterLabelValues, labelInterner.intern(normalizeHttpPath(path)))
+			counterLabelValues = append(counterLabelValues, labelInterner.intern(s.pathLimiter.Limit(normalizeHttpPath(path))))
 		} else {
 			counterLabelValues = append(counterLabelValues, "")
 		}

flags/flags.go

@@ -75,6 +75,17 @@ var (
 
 	HttpPathNormalizationRules = kingpin.Flag("http-path-normalization-rules", "Custom HTTP path normalization rules in format 'pattern1:replacement1,pattern2:replacement2'").Envar("HTTP_PATH_NORMALIZATION_RULES").String()
 
+	// MaxHttpPathsPerContainer bounds the number of distinct `path` label values a
+	// single container may contribute to container_http_requests_total. Path is the
+	// only L7 label an outside party controls: an internet-facing ingress gets probed
+	// for vulnerabilities around the clock, and every unique junk path (/axds.php,
+	// /HNAP1, ...) becomes a permanent series that normalization cannot collapse
+	// because the paths are genuinely distinct literals. Past the cap, further unseen
+	// paths collapse to "{other}" so the request count stays correct while the series
+	// count stops growing. Real applications serve far fewer than the default; the
+	// cap should only ever engage on scanner traffic.
+	MaxHttpPathsPerContainer = kingpin.Flag("max-http-paths-per-container", "Max distinct HTTP path label values per container, excess collapses to {other} (0 = unlimited)").Default("1000").Envar("MAX_HTTP_PATHS_PER_CONTAINER").Int()
+
 	AggregateEphemeralWorkloads = kingpin.Flag("aggregate-ephemeral-workloads", "Aggregate metrics for bare pods and standalone Jobs using standard labels to reduce series cardinality").Default("true").Envar("AGGREGATE_EPHEMERAL_WORKLOADS").Bool()
 
 	// CollapseInternalDestinations replaces the raw IP:port value of the
@@ -98,6 +109,13 @@ func GetString(fl *string) string {
 	return *fl
 }
 
+func GetInt(fl *int) int {
+	if fl == nil {
+		return 0
+	}
+	return *fl
+}
+
 func init() {
 	if strings.HasSuffix(os.Args[0], ".test") {
 		return