Metrics migration steps aren't performable/verifiable under the operator
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
Summary
Step 1 of the migration in docs/observability.md says to point a scraper at the
diagnostics port and confirm metrics arrive. Under the operator today that step isn't
actually performable: mcpserver_controller.go:1467 declares only the proxy
containerPort, and :1561 only the proxy ServicePort. The diagnostics listener is
genuinely up on 9464 (or whatever prometheusPort resolves to) — it's just
undiscoverable. A PodMonitor has no named port to key on, and prometheusPort is
intentionally absent from MCPTelemetryConfigSpec (see the drift-table justification
in telemetry_drift_test.go), so there's no field to even declare it explicitly.
Separately, step 2 ("set metricsOnTransportPort: false and confirm nothing else was
still scraping it") isn't verifiable either. The startup WARN reports configuration,
not usage — it fires because the flag is on, not because a scraper actually hit the
endpoint. And the transport-port /metrics mux registration outranks the /
catch-all, so those requests bypass the telemetry middleware entirely; there's no
counter anywhere that would tell you whether traffic there is real.
Proposed fixes
- Discoverability: add a named
containerPort(e.g.metrics) on the workload
pod whenEnablePrometheusMetricsPathis set, so aPodMonitorcan target it by
name. Leave theServicealone — deliberately, per the existing docs guidance that
the diagnostics port must not be routed through anything internet-facing. - Verifiability: wrap the transport-port
/metricshandler (mounted at
pkg/runner/runner.goaround themountPrometheusHandlerOnTransportPortcall)
with a request counter, so step 2 has something concrete to check — "requests to
the transport-port copy since startup: N" — before an operator turns it off.
Neither blocks #6371; raised there as informational.
Raised by @jhrozek on #6371:
https://github.com/stacklok/toolhive/pull/6371#pullrequestreview-5027693972
Part of #6271. Related: #6384.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the migration steps in docs/observability.md, then inspect mcpserver_controller.go around lines 1467 and 1561 and telemetry_drift_test.go for the existing port constraints. Review pkg/runner/runner.go around mountPrometheusHandlerOnTransportPort for the transport-port metrics path. Done means the diagnostics port is discoverable by a named workload port and transport-port requests expose a count that makes the migration checks verifiable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes, prometheus
- Domain
- devops, observability-sre
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100