Skip to content

[Metrics] Hide deprecated metrics with gpu_ prefix#24245

Merged
mgoin merged 1 commit intovllm-project:mainfrom
markmc:hide-deprecated-gpu-metrics
Sep 16, 2025
Merged

[Metrics] Hide deprecated metrics with gpu_ prefix#24245
mgoin merged 1 commit intovllm-project:mainfrom
markmc:hide-deprecated-gpu-metrics

Conversation

@markmc
Copy link
Copy Markdown
Member

@markmc markmc commented Sep 4, 2025

In #18354, these metrics were deprecated, and the change was included in the v0.9.2 release.

We probably should only deprecate things in a v0.N.0 minor release, so let's say these were deprecated in v0.10.0.

According to https://docs.vllm.ai/en/latest/usage/metrics.html:

Note: when metrics are deprecated in version X.Y, they are hidden in
version X.Y+1 but can be re-enabled using the
--show-hidden-metrics-for-version=X.Y escape hatch, and are then
removed in version X.Y+2.

The deprecated metrics should be hidden in the v0.11.0 release, but with a --show-hidden-metrics-for-version=0.10 escape hatch.

They should then be removed in the v0.12.0 release.

@markmc
Copy link
Copy Markdown
Member Author

markmc commented Sep 4, 2025

/cc @sahelib25

@mergify mergify bot added the v1 label Sep 4, 2025
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request aims to hide deprecated metrics with the gpu_ prefix, making them available only via an escape hatch, and updates the tests accordingly. The logic for hiding the metrics is sound, but it introduces a critical bug where the application would crash if the hidden metrics are not shown, due to uninitialized attributes. My review includes a fix for this bug by providing no-op metric objects when the deprecated metrics are hidden. This ensures the program runs correctly in all configurations.

@markmc markmc force-pushed the hide-deprecated-gpu-metrics branch 2 times, most recently from 92bf525 to 3ff01b6 Compare September 4, 2025 11:44
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) September 4, 2025 12:51
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 4, 2025
@simon-mo
Copy link
Copy Markdown
Collaborator

simon-mo commented Sep 4, 2025

metrics test failed

@simon-mo simon-mo added this to the v0.10.2 milestone Sep 4, 2025
auto-merge was automatically disabled September 5, 2025 09:38

Head branch was pushed to by a user without write access

@markmc markmc force-pushed the hide-deprecated-gpu-metrics branch from 3ff01b6 to 4c4972e Compare September 5, 2025 09:38
@markmc markmc removed this from the v0.10.2 milestone Sep 5, 2025
@markmc
Copy link
Copy Markdown
Member Author

markmc commented Sep 5, 2025

This is intended to be merged in 0.11.0, not the 0.10.x series

In vllm-project#18354, these metrics were deprecated, and the change
was included in the v0.9.2 release.

We probably should only deprecate things in a v0.N.0 minor
release, so let's say these were deprecated in v0.10.0.

According to https://docs.vllm.ai/en/latest/usage/metrics.html:

> Note: when metrics are deprecated in version X.Y, they are hidden in
>  version X.Y+1 but can be re-enabled using the
>  --show-hidden-metrics-for-version=X.Y escape hatch, and are then
>  removed in version X.Y+2.

The deprecated metrics should be hidden in the v0.11.0 release,
but with a --show-hidden-metrics-for-version=0.10 escape hatch.

They should then be removed in the v0.12.0 release.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
@markmc markmc force-pushed the hide-deprecated-gpu-metrics branch from 4c4972e to 7d68d9b Compare September 9, 2025 10:34
@markmc
Copy link
Copy Markdown
Member Author

markmc commented Sep 9, 2025

Rebased because of flaky structured output tests

@markmc markmc added this to the v0.10.2 milestone Sep 9, 2025
@markmc
Copy link
Copy Markdown
Member Author

markmc commented Sep 9, 2025

Adding back to v0.10.2 milestone, in case it becomes v0.11.0

@markmc markmc changed the title [Metrics] Hide deprecated metrics with gpu_ prefix [For v0.11.0] [Metrics] Hide deprecated metrics with gpu_ prefix Sep 9, 2025
@DarkLight1337
Copy link
Copy Markdown
Member

DarkLight1337 commented Sep 10, 2025

This is intended to be merged in 0.11.0, not the 0.10.x series

0.10.2 is confirmed, so please re-assign this to 0.11.0

@markmc markmc removed this from the v0.10.2 milestone Sep 10, 2025
@markmc
Copy link
Copy Markdown
Member Author

markmc commented Sep 10, 2025

This is intended to be merged in 0.11.0, not the 0.10.x series

0.10.2 is confirmed, so please re-assign this to 0.11.0

Thanks! No 0.11.0 milestone yet, and I can't create one. Removed from 0.10.2

@markmc markmc changed the title [For v0.11.0] [Metrics] Hide deprecated metrics with gpu_ prefix [Metrics] Hide deprecated metrics with gpu_ prefix Sep 10, 2025
@DarkLight1337 DarkLight1337 added this to the v0.11.0 milestone Sep 13, 2025
@mgoin mgoin merged commit 2942970 into vllm-project:main Sep 16, 2025
39 checks passed
@DarkLight1337
Copy link
Copy Markdown
Member

DarkLight1337 commented Sep 19, 2025

We have decided the next release to be v0.10.3 instead of v0.11.0, should we revert this in the release branch?

@DarkLight1337
Copy link
Copy Markdown
Member

cc @simon-mo

markmc added a commit to markmc/vllm that referenced this pull request Sep 22, 2025
…ect#24245)"

This change is intended for 0.11.0.

This reverts commit 2942970.
@markmc
Copy link
Copy Markdown
Member Author

markmc commented Sep 22, 2025

We have decided the next release to be v0.10.3 instead of v0.11.0, should we revert this in the release branch?

Yes, see #25392

FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
choprahetarth pushed a commit to Tandemn-Labs/vllm that referenced this pull request Oct 11, 2025
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants