Skip to content

Controller-manager Prometheus metrics - #245

Merged
zolug merged 6 commits into
masterfrom
metrics-cm
Sep 22, 2026
Merged

zolug merged 6 commits into
masterfrom
metrics-cm

Conversation

@zolug

@zolug zolug commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

#247

Adds custom Prometheus metrics for the controller-manager, plus the small controller-package streamlining
needed to expose shared state to the collectors.

Metrics

Pull-based prometheus.Collectors registered against the controller-runtime
metrics registry when --metrics-bind-address is set:

Metric Meaning
<prefix>_gateway_count Gateways with Accepted=True managed by this controller
<prefix>_gateway_programmed 0/1 per Gateway, from the Programmed condition
<prefix>_distributiongroup_ready 0/1 per DG, from the Ready condition
<prefix>_distributiongroup_endpoints endpoint count per DG per Gateway
<prefix>_distributiongroup_max_endpoints capacity per DG per Gateway (+Inf if unbounded)

Collectors read live from the manager's informer cache on scrape (no
background loop, no reconcile coupling); deleted objects simply stop
appearing and Prometheus marks the series stale.

Notable design points

  • Cache-sync gate: a scrape landing before the informer cache has synced
    fails fast with a single error instead of blocking or emitting a partial
    metric set. Collect has no context, so a --metrics-collect-timeout flag
    (default 5s, deliberately below Prometheus's 10s scrape_timeout) bounds
    the wait.

  • Label policy: separate namespace/gateway_namespace labels
    (kube-state-metrics convention) so all-namespace watches don't collide
    identically-named objects.

  • Reusable prefix-validation / Enabled helpers in internal/common/metrics
    for future component metrics (LB, router, sidecar).

  • gateway_programmed gating: emitted for every Gateway destined for this
    controller by its GatewayClass (spec.gatewayClassName
    GatewayClass.spec.controllerName), from the Programmed condition,
    independent of Accepted. This intentionally differs from gateway_count
    (Accepted-by-us): it reports 0/1 for the whole lifetime a Gateway is
    class-ours — including before acceptance or after an Accepted reset — so a
    Gateway stuck un-accepted is visible as programmed=0 rather than emitting no
    series. The stale case during a discouraged gatewayClassName change (a
    previous owner left Programmed=True) is reported as-is; the metric makes no
    data-plane-freshness claim.

    Open question: distributiongroup_ready semantics

distributiongroup_ready mirrors the DG Ready status condition as-is, which
today means "the DG has assigned endpoints" (len(desiredSlices) > 0), not
"the DG has endpoints that are ready to serve traffic". Per-endpoint
LoadBalancerEndpoint.Ready (Pod readiness) is not consulted when the condition is
set (buildReadyCondition in distributiongroup/status.go) — a DG whose endpoints
are all NotReady still reports Ready=True, and this metric will too.

This is a likely mismatch with what the observability study (#153) anticipates a
"ready" signal to mean for a distribution group. The collector deliberately reports
the existing condition rather than inventing a second, metrics-only definition of
readiness under the same name — surfacing the discrepancy instead of hiding it.

Resolving it (should Ready itself account for per-endpoint readiness, e.g. a
per-Gateway "has ready endpoints" status field?) is a DistributionGroup reconciler
design decision
, out of scope for this metrics PR. Flagging here so #153/#236
reviewers can decide whether the metric's meaning is acceptable as shipped or should
block on a readiness redesign.

Streamlining

Extracts Gateway status-condition helpers into internal/common/gatewayutil
(IsGatewayAcceptedByController, IsGatewayProgrammed) and de-methodizes the
DistributionGroup Gateway/route resolution helpers so the collectors can reuse
the exact reconciler semantics without depending on the controller packages.
Removes the duplicated Accepted-check between the gateway and distributiongroup
controllers. No behavior change.

Testing

  • Unit tests for the cache-sync gate, prefix validation, and collector
    registration; go build ./..., package tests, and make lint all clean.
  • Validated end-to-end against a Kind cluster:
    secure metrics served over HTTPS, scraped by an in-cluster Prometheus via a
    ServiceMonitor, all metrics confirmed with real values. Reproducible recipe —
    including the Prometheus/Grafana walkthrough — in docs/development/metrics-testing.md.

In-cluster metrics

  • The base deploy (config/default) now serves metrics securely over HTTPS on
    :8443
    by default, using a cert-manager-issued cert (metrics-server-cert),
    fronted by the controller-manager-metrics-service Service.
  • No ServiceMonitor ships in the base — it must not depend on the Prometheus
    Operator CRDs, so kubectl apply -k config/default works without the Operator.
    Wiring a Prometheus scrape (Helm stack + ServiceMonitor + verification) is
    documented in docs/development/metrics-testing.md rather than shipped in the
    manifest.
  • prometheus/client_golang promoted to a direct dependency.

@ljkiraly ljkiraly left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendations:

  1. Add Collect-level unit tests for both collectors (gateway_collector_test.go,
    distributiongroup_collector_test.go). Cover: Gateway-union resolution incl. empty-union fallback, per-Gateway endpoint counting with non-owned-slice filtering, resolveMaxEndpoints branches (incl. unknown-type +Inf), and gateway_count / per-Gateway programmed emission with an accepted-by-different-controller exclusion. Use fake client + WithIndex (existing pattern) and prometheus/testutil. This is the single item separating this from a clean approve.

Nice-to-have:

  1. Add a brief note in docs/controllers/gateway.md that the Accepted condition Message format is now a load-bearing contract (matched via gatewayutil.GatewayAcceptedMessagePrefix), so future edits to acceptedMessage must preserve the prefix. The code comments capture this, but the controller doc is where a maintainer editing status messages would look.
  2. Consider a follow-up to give Gateway "accepted by controller X" a machine-readable signal (e.g. a dedicated Reason or annotation) rather than relying on message-suffix matching. Not for this PR — it's a larger condition-contract change with existing tests asserting on message content — but worth a tracking issue.

Minor:

  1. The --metrics-collect-timeout flag help text is very long (a full paragraph). Consider trimming to one sentence and moving the race-condition rationale to the metrics doc, so --help output stays scannable.

@zolug
zolug requested a review from ljkiraly September 18, 2026 15:55

@ljkiraly ljkiraly left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any item resolved with well targeted tests.
Looks good.

@zolug
zolug requested a review from ljkiraly September 22, 2026 09:08
Move Gateway status-condition interpretation out of the gateway controller
into internal/common/gatewayutil (IsGatewayAcceptedByController,
IsGatewayProgrammed, GatewayAcceptedMessagePrefix), and de-methodize the
DistributionGroup Gateway/route resolution helpers (ListReferencedGateways,
listRoutesReferencingDG, getGatewayFrom*) so they take a client.Client +
namespace instead of a reconciler receiver. Export DistributionGroup IsReady
next to where the Ready condition is written.

Lets these be reused by external consumers (the controller-manager metrics
collectors) without depending on the reconciler packages, and removes the
duplicated Accepted-check logic between the gateway and distributiongroup
controllers.

Signed-off-by: Lugossy Zoltán <zoltan.lugossy@est.tech>
Add pull-based prometheus.Collectors for Gateway and DistributionGroup
state (gateway_count, gateway_programmed, distributiongroup_ready,
distributiongroup_endpoints, distributiongroup_max_endpoints), reading
from the manager's informer cache on scrape. Registered against the
controller-runtime metrics registry when --metrics-bind-address is set.

Adds a cache-sync gate so a scrape before the informer cache has synced
fails fast rather than blocking or emitting a partial set, and a
--metrics-collect-timeout flag bounding that wait. Includes prefix
validation/Enabled helpers in internal/common/metrics and a local
testing doc.

Signed-off-by: Lugossy Zoltán <zoltan.lugossy@est.tech>
Enable secure metrics serving by default in config/default (mount the
cert-manager metrics-server-cert, set --metrics-cert-path, substitute the
metrics Service name/namespace into the cert dnsNames). Log collector
registration in the controller-manager. Expand metrics-testing.md with the
in-cluster Prometheus/Grafana scrape recipe and an OTEL-migration note.

Signed-off-by: Lugossy Zoltán <zoltan.lugossy@est.tech>
Signed-off-by: Lugossy Zoltán <zoltan.lugossy@est.tech>
Cover Gateway-union resolution, non-owned-slice filtering, resolveMaxEndpoints
branches, and accepted-by-different-controller exclusion via fake client +
testutil. Export DistributionGroup's Ready condition constants for reuse in
these tests. Also trims --metrics-collect-timeout help text and clarifies the
Gateway Programmed condition message/docs.

Signed-off-by: Lugossy Zoltán <zoltan.lugossy@est.tech>
Emit gateway_programmed for every Gateway destined for this controller by its
GatewayClass (spec.gatewayClassName -> GatewayClass.spec.controllerName).
Independent of the Accepted condition, so a class-ours Gateway reports 0/1 for
its whole lifetime — including before acceptance or after an Accepted reset —
instead of emitting no series. gateway_count is unchanged (still Accepted-by-us).

Signed-off-by: Lugossy Zoltán <zoltan.lugossy@est.tech>
@zolug
zolug merged commit bd7ece1 into master Sep 22, 2026
12 checks passed
@zolug
zolug deleted the metrics-cm branch September 22, 2026 12:56
@zolug
zolug restored the metrics-cm branch September 22, 2026 15:47
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.

2 participants