Repository navigation
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
This pull request has merge conflicts that must be resolved before it can be |
robertgshaw2-redhat
added
the
ready
|
/ci run |
|
✅ @GuyStone, CI is now available for this PR.
|
|
✅ Triggered Buildkite CI #87914 for commit |
|
Hi @GuyStone, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
|
This pull request has merge conflicts that must be resolved before it can be |
|
+1 for this one. We also see the need for closer buckets in the 0 -> 300 ms range. Is there any way I can help to push this forward? |
ce38c83 to
3486940
Compare
|
/ci run |
|
✅ Triggered Buildkite CI #91005 for commit |
|
❌ This PR is 1 commit behind upstream |
|
/ci run |
|
✅ Triggered Buildkite CI #91129 for commit |
|
Hi folks, @GuyStone! Any chance we can get this in the next release? Anything I can do to help? I'm new to vLLM but I'd like to contribute. |
Hi @isacandrei! Hopefully we can get this into the next release. The build is now green, so the next step is maintainer review and, assuming everything looks good, merging. I believe @markmc is going to help with the review. |
markmc
left a comment
markmc
left a comment
There was a problem hiding this comment.
This is really excellent, well done. And apologies that it took so long to be reviewed 👍
|
❌ This PR is 1 commit behind upstream |
5b8de81 to
79ce274
Compare
|
/ci run |
|
✅ Triggered Buildkite CI #92191 for commit |
CI selector (shadow): 39 test steps (64 jobs) instead of 70 (90 jobs)Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works. Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.
Selector would run (39)
Would skip (today's rules run them) (47)
Would add (today's rules do not run them) (16)
AMD mirrors: would skip (47)
AMD mirrors: would add (9)
CI results (2026-09-30 16:52 UTC)143 passed, 1 failed, 30 pending. No misses: every failed job the selector would skip was also failing on main.
8 changed files · base |
|
Rebase to pick up #59476 |
Add custom_histogram_buckets to ObservabilityConfig: a mapping from bucket-family key to a replacement list of bucket upper bounds, exposed on the CLI as --custom-histogram-buckets taking a JSON object. A present family key replaces the default boundaries verbatim for every histogram in that family; unset or unlisted families keep today's defaults, so default metric output is unchanged. Values are validated when the config is built: unknown family keys, empty lists, boolean, non-finite, or non-positive bounds, and non-strictly-increasing sequences are all rejected with messages naming the field, the offending key or value, and the violated rule. Booleans are rejected in a before-mode validator because pydantic would otherwise silently coerce them to 1.0/0.0. histogram_buckets() gains an overrides parameter and remains the single place bucket lists are resolved. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Guy Stone <guys@spotify.com>
Cover the validation matrix for custom_histogram_buckets (unknown family, empty list, boolean, non-finite, non-positive, and non-increasing bounds), override precedence in histogram_buckets(), end-to-end application through PrometheusStatLogger with KV cache metrics enabled, and the CLI round-trip including malformed JSON and unknown-family rejection. The end-to-end test overrides every family at once, each with a distinct list, so a histogram left wired to its defaults because its histogram_buckets() call is missing the overrides argument fails the test rather than passing unnoticed. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Guy Stone <guys@spotify.com>
Describe the --custom-histogram-buckets option: a usage example, the mapping from family keys to affected metrics, the validation rules, and a warning that finer buckets multiply Prometheus time-series cardinality per label combination. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Guy Stone <guys@spotify.com>
79ce274 to
0a6b43f
Compare
|
/ci run |
… families (vllm-project#48867) The core engine histograms ship with default bucket boundaries tuned for typical serving workloads. The `--custom-histogram-buckets` option replaces the boundaries of one or more bucket families. Use it, for example, to track sub-300ms latency SLOs with the request-phase histograms, whose smallest default boundary is 0.3s. Signed-off-by: Guy Stone <guys@spotify.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Note
Stacked on #48866. GitHub cannot base a cross-fork PR on another fork branch, so this PR's diff necessarily includes #48866's two commits. To review only this PR's changes, use the incremental diff —
histogram-buckets-01-consolidate...histogram-buckets-02-custom-override— or the last 3 commits (515a37f18,bfd031dcd,5d493a5da). Once #48866 merges, the diff here collapses to just those commits.[Metrics] Add --custom-histogram-buckets to override histogram bucket families
Purpose
Fixes #34370.
ObservabilityConfig.custom_histogram_bucketsand a--custom-histogram-bucketsCLI flag taking a JSON mapping from bucket-family key to replacement boundaries. Listed families replace their defaults forevery histogram in the family; unlisted families keep defaults:
vllm serve Qwen/Qwen3-0.6B \ --custom-histogram-buckets '{"request_latency": [0.01, 0.05, 0.1, 0.25, 0.5, 1.0, 5.0, 30.0]}'Why this does not duplicate existing PRs
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.