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 when EnablePrometheusMetricsPath is set, so a PodMonitor can target it by
name. Leave the Service alone — deliberately, per the existing docs guidance that
the diagnostics port must not be routed through anything internet-facing.
- Verifiability: wrap the transport-port
/metrics handler (mounted at
pkg/runner/runner.go around the mountPrometheusHandlerOnTransportPort call)
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:
#6371 (review)
Part of #6271. Related: #6384.
Summary
Step 1 of the migration in
docs/observability.mdsays to point a scraper at thediagnostics port and confirm metrics arrive. Under the operator today that step isn't
actually performable:
mcpserver_controller.go:1467declares only the proxycontainerPort, and:1561only the proxyServicePort. The diagnostics listener isgenuinely up on
9464(or whateverprometheusPortresolves to) — it's justundiscoverable. A
PodMonitorhas no named port to key on, andprometheusPortisintentionally absent from
MCPTelemetryConfigSpec(see the drift-table justificationin
telemetry_drift_test.go), so there's no field to even declare it explicitly.Separately, step 2 ("set
metricsOnTransportPort: falseand confirm nothing else wasstill scraping it") isn't verifiable either. The startup
WARNreports configuration,not usage — it fires because the flag is on, not because a scraper actually hit the
endpoint. And the transport-port
/metricsmux 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
containerPort(e.g.metrics) on the workloadpod when
EnablePrometheusMetricsPathis set, so aPodMonitorcan target it byname. Leave the
Servicealone — deliberately, per the existing docs guidance thatthe diagnostics port must not be routed through anything internet-facing.
/metricshandler (mounted atpkg/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:
#6371 (review)
Part of #6271. Related: #6384.