vllm-project / vllm-project/aibrix
[Bug] Per-model metrics cross-talk on multi-model pods (engine_fetcher only parses Metric[0])
- Dominant language
- Go
- Stars
- 5.1k
- Forks
- 694
- Avg merge
- 1d 19h
- Merged PRs (30d)
- 98
Description
### What's wrong
`pkg/metrics/engine_fetcher.go` parses only the first instance of a metric family, then hands that one value to every model it found in the same family. So the moment a single pod's scrape carries the same metric under more than one `model_name`, every model points at the first one's value.
`parseMetricFromFamily` (engine_fetcher.go:301):
```go
firstMetric := metricFamily.Metric[0]
```
`FetchAllTypedMetrics` extracts all the model names and assigns the same `MetricValue` pointer to each key (lines 244-250):
```go
modelNames := ef.extractModelNamesFromMetrics(allMetrics, rawMetricName)
for _, modelName := range modelNames {
key := fmt.Sprintf("%s/%s", modelName, metricName)
result.ModelMetrics[key] = metricValue // same pointer every iteration
}
```
Given a scrape like:
```
vllm_num_requests_running{model_name="m1"} 3
vllm_num_requests_running{model_name="m2"} 5
```
both `m1/running_requests` and `m2/running_requests` end up pointing at the same `*SimpleMetricValue{Value:3, Labels:{model_name:"m1"}}`.
That breaks two ways. The number is wrong for every model except whichever one happened to be at `Metric[0]`, and `scorer.go` schedules on that number. And since it's one shared pointer, the labels read `model_name=m1` for all of them — `sanitizeMetricValueLabels` rewrites that shared map, and the Prometheus emit sets `RoutingContext.Model=m2` while the label still says `m1`.
### How likely is this to actually fire
I opened this assuming vLLM multi-LoRA would hit it. It doesn't, and I want to correct that here.
vLLM puts a single `model_name` on its core gauges (`num_requests_running`, `num_requests_waiting`, and so on) — the base served model. LoRA requests fold into that same gauge instead of getting their own `model_name`. Per-adapter counts sit in a separate metric, `vllm:lora_requests_info`, with the adapter names packed into a comma-separated string rather than real labels. So a vLLM pod exposes one `model_name` per family whether or not LoRA is in play, `extractModelNamesFromMetrics` returns one name, and `Metric[0]` is the right one.
This only bites if some engine exposes the same family with more than one `model_name` on a single pod's scrape. I don't have a confirmed engine that does this today — vLLM doesn't. So this is a latent bug, not something burning in production right now.
It's still worth fixing. `extractModelNamesFromMetrics` already loops over every instance in the family, so the surrounding code clearly expects the multi-model case — the `Metric[0]` shortcut just quietly contradicts it. And the shared-pointer label rewrite is a trap for the first engine that does emit multiple names.
### What correct looks like
Each `result.ModelMetrics["/"]` should hold the value from the `Metric[i]` whose `model_name` matches ``, with its own labels map — no shared pointer across models.
### Why tests miss it
The mocks in `engine_fetcher_test.go` only carry one `model_name` per family, so `Metric[0]` is the whole family and the bug never surfaces. A fixture with two `model_name` values in one family reproduces it.
### Environment
- AIBrix: main
- Internal bookkeeping bug — reproducible in a unit test, no specific engine/client required.
Contributor guide
Research direction
Start with pkg/metrics/engine_fetcher.go, especially parseMetricFromFamily and FetchAllTypedMetrics, then inspect engine_fetcher_test.go. Add a fixture containing two model_name values in one metric family and run the focused tests. Done when each model key receives its matching value and independent labels, with the tests passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- observability
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100