fix: the operator's metrics reach nothing, and two comments say otherwise - #184
Merged
Conversation
…wise Five alert rules and the whole agent-operator dashboard read controller-runtime metrics from the eks-agent-platform operator. Those metrics have never reached Amazon Managed Prometheus, because nothing discovers the operator. charts/operator serves /metrics over HTTPS behind controller-runtime's authentication and authorization filter whenever metrics.secure is true, which is the chart default and is overridden nowhere. The chart then suppresses its own prometheus.io/scrape annotations while secure — deliberately, because an annotation scrape is plaintext and unauthenticated and that endpoint rejects it. Rendering the chart against this repo's values emits zero scrape annotations. The otel-agent's only pod job is annotation-gated, so it never sees the operator. The chart also renders a ServiceMonitor for the authenticated path, but this fleet installs prometheus-operator-crds and no controller that consumes one, so it is read by nothing. There is no AMP managed scraper either. The result is five rules and a dashboard wired to a metric that never arrives. Two comments in this repo asserted the opposite as fact — that the metrics "reach AMP via the operator pod's prometheus.io/scrape annotation". Both are corrected here. A comment describing a wiring that does not exist is the same defect as the wiring being absent, and harder to find, because it answers the question a reader would otherwise go and check. ─────────────────────────── the fix ─────────────────────────── A dedicated scrape job on the otel-agent that takes the authenticated route: HTTPS, the agent's own ServiceAccount token as the bearer credential, TLS verification skipped because controller-runtime generates a self-signed cert in memory and there is no CA to pin — the token is what gates access, which is the same reasoning the chart's own ServiceMonitor records. No new grant is needed, and that is worth stating precisely rather than assuming: the endpoint authorizes with a TokenReview plus a SubjectAccessReview on the /metrics nonResourceURL, and this collector's ClusterRole already carries `nonResourceURLs: ["/metrics"], verbs: ["get"]` for the kubelet scrape. So the agent presents its own token rather than mounting the chart's metrics-reader Secret, which lives in another namespace and could not be mounted anyway. metrics.secure stays true. Setting it false at this layer would have been one line and would have reopened exactly the info-disclosure gap the chart closed on purpose. Discovery is namespace-scoped and node-scoped, matching every other job here, so each operator pod is scraped exactly once by the agent co-located with it. The container-port keep is not cosmetic: pod discovery yields one target per declared container port, and without it the agent would also scrape the probe port on 8081 and log a failure every interval. Verified by rendering collector chart 0.166.0 against the committed values and extracting the emitted config: the job is present with the intended scheme, token file and relabel set. Those scrape configs were then checked with `promtool check config`, which passes — and the check was itself checked by injecting an invalid relabel action, which promtool rejects with `unknown relabel action`, so the pass means it parsed something.
CI Results
All checks passed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
Five alert rules and the whole
agent-operatordashboard read controller-runtime metrics from the eks-agent-platform operator. Those metrics have never reached AMP, because nothing discovers the operator.charts/operatorserves/metricsover HTTPS behind controller-runtime's authn/authz filter whenevermetrics.secureis true — the chart default, overridden nowhere. The chart then suppresses its ownprometheus.io/scrapeannotations while secure, deliberately, because an annotation scrape is plaintext and unauthenticated and that endpoint rejects it.Rendering the chart against this repo's values emits zero scrape annotations.
Every candidate path is closed:
prometheus-operator-crdsand no controller that consumes oneTwo comments in this repo asserted the opposite as fact — that metrics "reach AMP via the operator pod's prometheus.io/scrape annotation". Both are corrected here. A comment describing a wiring that does not exist is the same defect as the wiring being absent, and harder to find, because it answers the question a reader would otherwise go and check.
(This is partly self-announcing:
OperatorMetricsAbsent, severity page, fires on exactly this condition. It was the gap being silent in the docs, not in the alerting.)The fix
A dedicated scrape job on the otel-agent taking the authenticated route: HTTPS, the agent's own ServiceAccount token as bearer credential, TLS verification skipped because controller-runtime generates a self-signed cert in memory — there is no CA to pin, and the token is what gates access. Same reasoning the chart's own ServiceMonitor records.
No new grant is needed, and that is worth stating precisely rather than assuming. The endpoint authorizes with a TokenReview plus a SubjectAccessReview on the
/metricsnonResourceURL, and this collector's ClusterRole already carries:So the agent presents its own token rather than mounting the chart's
metrics-readerSecret — which lives in another namespace and could not be mounted anyway.metrics.securestays true. Setting it false at this layer would have been one line and would have reopened exactly the info-disclosure gap the chart closed on purpose.Discovery is namespace- and node-scoped, matching every other job here, so each operator pod is scraped exactly once by the agent co-located with it. The container-port keep is not cosmetic: pod discovery yields one target per declared container port, so without it the agent would also scrape the probe port on 8081 and log a failure every interval.
Verified
Rendered collector chart 0.166.0 against the committed values and extracted the emitted config — the job is present with the intended scheme, token file and relabel set:
Those scrape configs then passed
promtool check config. The check was itself checked — injecting an invalid relabel action makes promtool exit 1 withunknown relabel action "bogus_action", so the pass means it parsed something rather than skipping.task validatepasses.What this does not do
It does not make the four latency burn rules correct — they were fixed separately in #181. It makes them able to evaluate at all. Whether the reconcile rate clears the minimum-traffic gate added there is now an observable question rather than a theoretical one.
https://claude.ai/code/session_012iMnbboJuiUMSvu7n8oRhz