Skip to content

feat(metrics): add requests_per_prefix_total counter with handler-level test coverage#557

Closed
Copilot wants to merge 3 commits into
masterfrom
copilot/fix-code-for-review-comment
Closed

feat(metrics): add requests_per_prefix_total counter with handler-level test coverage#557
Copilot wants to merge 3 commits into
masterfrom
copilot/fix-code-for-review-comment

Conversation

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown

Adds a configurable per-prefix Prometheus counter on /render. Each request increments requests_per_prefix_total{prefix=...} once per configured prefix directly referenced by any target metric pattern (metric == prefix, or starts with <prefix>.).

What issue is this change attempting to solve?

The per-prefix counting logic in renderHandler was untested — existing tests only covered metricRefsPrefix and initRequestPrefixes in isolation, leaving the full target-parsing → prefix-matching → counter-increment pipeline vulnerable to silent regressions.

How does this change solve the problem? Why is this the best approach?

  • New config field requestsPerPrefixList []string on the API config; empty/glob entries dropped at startup via initRequestPrefixes.
  • New metric requests_per_prefix_total (CounterVec with prefix label) registered in PrometheusMetrics.
  • collectMatchedPrefixes(targets, configuredPrefixes) — extracted from renderHandler's inline loop into a standalone function that parses each target string, walks exp.Metrics(), and returns the matched prefix set. This makes the full pipeline unit-testable without a live HTTP handler.
  • renderHandler calls collectMatchedPrefixes before its main loop, then increments the counter for each matched prefix after all targets are processed.

Extraction into collectMatchedPrefixes is the minimal change that makes the logic independently testable while keeping renderHandler readable.

How can we be sure this works as expected?

TestCollectMatchedPrefixes covers the full pipeline (target string → parser.ParseExpr → Metrics() → metricRefsPrefix) with 10 cases:

  • No configured prefixes → nil result
  • Single/multiple targets matching one or more prefixes
  • Function expressions (e.g. sum(a.b.c.*))
  • Unparseable targets silently skipped; valid targets still counted
  • Dot-boundary enforcement (a.bc.d does not match prefix a.b)
  • Exact match (a.b matches prefix a.b)
  • Each prefix counted at most once across multiple targets

v-pap and others added 2 commits June 17, 2026 09:32
Adds a configurable per-prefix counter on /render. Each request
increments requests_per_prefix_total{prefix=...} once per configured
prefix that is directly referenced by one of its target metric
patterns (metric == prefix, or metric starts with '<prefix>.').

Configured via the new requestsPerPrefixList yaml key on the api
side. Empty/glob entries are dropped at startup. Field is nil by
default — feature stays silent until configured.
…est coverage

Address review comment #discussion_r3427005713: extract the per-prefix
prefix-matching loop from renderHandler into a new testable helper
collectMatchedPrefixes, and add TestCollectMatchedPrefixes with 10 cases
covering the full pipeline (target string → parser → Metrics() → prefix
matching), including function expressions, unparseable targets,
dot-boundary enforcement, exact matches, and deduplication.
Copilot AI changed the title [WIP] Fix code for review comment in PR #556 feat(metrics): add requests_per_prefix_total counter with handler-level test coverage Jun 17, 2026
Copilot AI requested a review from deniszh June 17, 2026 09:36
@deniszh deniszh closed this Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants