feat(controller): opt-in Prometheus scrape annotations on inference pods

[brandonliuli]

Upstream LLMKube proposal — inference-pod metrics discoverable by annotation scrapers

Problem

LLMKube runtimes expose Prometheus metrics on the inference container's app port
(llama.cpp llamacpp:*, vLLM vllm:*, TGI tgi_*) at /metrics. But making a
cluster-wide metrics agent scrape them today requires either:

• the inferencePodMonitor (needs prometheus-operator, which many clusters don't run), or
• hand-adding prometheus.io/* annotations to every InferenceService's spec.podAnnotations.

There's also a real footgun: the inference container's metrics port is named
http (the app port), which does not match the port-name regexes most
annotation-scrape configs use (metrics|http-metrics|metrics-port). So even
annotation-based discovery misses it unless prometheus.io/port is set explicitly.

Net: no zero-config path to inference metrics without prometheus-operator.

Proposal — two shapes (seeking maintainer preference)

Both extend the existing operator-default pattern on InferenceServiceReconciler
(cf. DefaultFSGroup, ModelCacheMode) and the merge point buildPodAnnotations
in internal/controller/deployment_builder.go. In both, a user's explicit
spec.podAnnotations value always wins.

Option A — opt-in scrape-annotation emission (reference PR implements this)

A --emit-scrape-annotations flag (chart metrics.emitScrapeAnnotations, default
off). When on, the operator adds prometheus.io/scrape=true, path=/metrics, and
port=<resolved spec.endpoint.port> to every inference pod.

• Pro: the operator knows the endpoint port, so prometheus.io/port is always
  correct — this dissolves the http-port-name footgun at the source, across all
  runtimes. Zero per-CR boilerplate.
• Con: opinionated toward prometheus.io-annotation scraping (though it's a
  no-op unless explicitly enabled).

Option B — generic operator-level default pod annotations

A --default-pod-annotations key=val,... flag (chart defaultPodAnnotations) that
seeds buildPodAnnotations at lowest precedence. The operator sets nothing metrics-
specific; the platform supplies the prometheus.io/* values (including a port).

• Pro: fully generic, reusable for any default annotation, not just metrics.
• Con: the port is static config — wrong for a fleet where InferenceServices
  use different endpoint.port values; the platform has to know/guess the port. Does
  not solve the port-name footgun on its own.

Recommendation

Option A — it fixes the actual discovery problem (the port) at the one place that
authoritatively knows the port, and stays a no-op until enabled. Option B is a nice
generic primitive but leaves the port problem to the caller. (They're not mutually
exclusive; A could later sit on top of a generic B.)

Reference PR

Implements Option A: reconciler field + --emit-scrape-annotations flag + chart
value/schema/arg + table-driven unit test (TestBuildPodAnnotations: emit off,
explicit port, default-8080 fallback, user-port-wins). go build, go vet, gofmt,
and the targeted test are green locally (envtest-gated tests need setup-envtest
binaries, unrelated).

---

AI-assisted: this change and text were drafted with an AI coding agent (Claude) and
are being reviewed, tested, and submitted by a human who takes DCO accountability for
the result.

[brandonliuli]

Design discussion + in-depth problem writeup (both options): #1367

[Defilan]

brandonliuli, welcome to LLMKube, and thanks for two solid PRs on your first day! The two-option framing here is genuinely useful and Option A is the right call. The opt-in wiring is clean end to end (chart to flag to reconciler to buildPodAnnotations), correctly off by default, and using a reconciler flag instead of a CRD field is the right pattern (no CRD regen needed). DCO and the PR-body AI disclosure both meet our policy.

One blocker, and it's small: the new endpointPort() helper reinvents port resolution and gets it wrong, which undercuts the PR's own thesis. The real container port is resolved in constructDeployment (deployment_builder.go:251-255) as ContainerPort -> Endpoint.Port -> backend.DefaultPort(), and DefaultPort() is runtime-specific: 80 for TGI, 8000 for vLLM, 30000 for SGLang, 8998 for personaplex, and only 8080 for llama.cpp/generic. endpointPort() (deployment_builder.go:310-315) ignores spec.containerPort and hardcodes 8080, so the zero-config path emits a wrong prometheus.io/port for every non-llama.cpp runtime, and mis-annotates any service using spec.containerPort, the exact footgun this PR exists to fix.

The fix is elegant: the fully-resolved port local already exists at deployment_builder.go:251 and is in scope at the buildPodAnnotations call (:342). Thread it through, buildPodAnnotations(isvc, r.EmitScrapeAnnotations, port), and delete endpointPort(). Please also add test cases for (a) a non-llama.cpp backend default and (b) spec.containerPort set, both currently produce a wrong annotation, so they'll pin the fix. And a note in values.yaml that this is an alternative to the PodMonitor, not to run alongside it against the same Prometheus (double-scrape). Happy to merge once the port comes from the resolved value.

[codecov]

Codecov Report

❌ Patch coverage is 71.42857% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cmd/main.go	0.00%	6 Missing ⚠️

📢 Thoughts on this report? Let us know!