Skip to content

Remove unregister_vllm_metrics no-op patch - #4396

Merged
hsliuustc0106 merged 5 commits into
vllm-project:mainfrom
vraiti:unpatch-unregister-metrics
Jun 18, 2026
Merged

hsliuustc0106 merged 5 commits into
vllm-project:mainfrom
vraiti:unpatch-unregister-metrics

Conversation

@vraiti

@vraiti vraiti commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

PLEASE FILL IN THE PR DESCRIPTION HERE.

Purpose

Previously, vLLM unregister_vllm_metrics would cause VLLMPrometheusMetrics to destroy vllm_omni:-namespaced metrics upon construction. This has been fixed with https://github.com/vllm-project/vllm/pull/42331, so the monkey-patch and subsequent warning

Test Plan

vLLM Version:
v0.22.0

vLLM-Omni Commit:
54a9d24

Test Result

Current /metrics:
metrics-before.txt

/metrics after patch:
metrics-after.txt

Server boot logs:
vllm-omni-logs-after.txt

BEFORE SUBMITTING: read CONTRIBUTING.md and run the precheck-pr skill with the code agent for a self-check against project conventions.
(anything written below this line will be removed by GitHub Actions)

Signed-off-by: vraiti <vraiti@redhat.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

Signed-off-by: vraiti <vraiti@redhat.com>
@vraiti
vraiti requested a review from lishunyang12 as a code owner June 12, 2026 18:59
hsliuustc0106

This comment was marked as off-topic.

@hsliuustc0106 hsliuustc0106 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@hsliuustc0106 hsliuustc0106 added frontend code related to entrypoint ready label to trigger buildkite CI labels Jun 15, 2026
@hsliuustc0106

Copy link
Copy Markdown
Collaborator

@vraiti

vraiti commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

fix ci https://buildkite.com/vllm/vllm-omni/builds/11448/list

The referenced CI failures seem preexisting? My code doesn't touch anything to do with Diffusers SP

@hsliuustc0106 hsliuustc0106 added ready label to trigger buildkite CI and removed ready label to trigger buildkite CI labels Jun 15, 2026

Copy link
Copy Markdown
Collaborator

CI triage for Buildkite build #11472: this looks introduced by this PR.

Evidence:

  • The only failing step in #11472 is Simple · Other Test.
  • The failing command is pytest -sv -m 'core_model and cpu' --ignore=tests/diffusion --ignore=tests/model_executor ....
  • The failures are all in tests/metrics/test_prometheus.py: 8 failed, 1601 passed.
  • The failed assertions are all missing vllm:omni_* Prometheus metric families/samples, e.g. vllm:omni_num_requests_running, vllm:omni_requests_success_total, and vllm:omni_e2e_request_latency_s_count.
  • This PR changes only metrics-related code: removes the scoped unregister_vllm_metrics patch and changes METRIC_PREFIX from vllm:omni_ to vllm_omni:.
  • The same test_prometheus.py failures were already present in earlier PR build #11448, so this is not from the later merge-from-main commit. Build #11448 also had unrelated diffusion sp marker collection errors, but #11472’s remaining failure is the metrics prefix/registration issue.

Likely fix: either update the Prometheus tests and verify the new vllm_omni: pipeline metrics are actually emitted, or keep the old vllm:omni_ prefix / scoped unregister behavior until the CI vLLM dependency definitely includes the upstream unregister fix.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: vraiti <vraiti@redhat.com>
@vraiti
vraiti requested a review from yenuo26 as a code owner June 17, 2026 15:55
@hsliuustc0106 hsliuustc0106 added ready label to trigger buildkite CI and removed ready label to trigger buildkite CI labels Jun 18, 2026
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
@hsliuustc0106
hsliuustc0106 merged commit 77d3951 into vllm-project:main Jun 18, 2026
4 of 5 checks passed
Nughm3 pushed a commit to Nughm3/vllm-omni that referenced this pull request Jun 18, 2026
Signed-off-by: vraiti <vraiti@redhat.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
alex-jw-brooks pushed a commit to alex-jw-brooks/vllm-omni that referenced this pull request Jun 23, 2026
Signed-off-by: vraiti <vraiti@redhat.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
@vraiti
vraiti deleted the unpatch-unregister-metrics branch July 20, 2026 13:02
pdtgct pushed a commit to pdtgct/vllm-omni that referenced this pull request Jul 29, 2026
…ing sections

Reverts b9fe67b's edits to inherited documentation content (lead direction: the metrics change should only ADD our own documentation, not rewrite pages this fork inherits -- prefix rewrites, gating-claim rewrites, and sentence deletions all restored byte-exact). The usage page's Streaming Session Metrics section and the design page's Path 5 remain, now purely additive: both files diff against the pre-docs state with zero deletions.

Research recorded for the prefix question (verified against upstream vllm-project/vllm-omni and the live exposition): vllm:omni_ was the ORIGINAL family prefix (upstream vllm-project#3576, 2026-06-01) and was renamed to vllm_omni: upstream (vllm-project#4396, 2026-06-17, which also removed the unregister_vllm_metrics patch) with a regression test pinning the legacy names as dropped (tests/metrics/test_transfer.py::test_legacy_ms_family_names_dropped on upstream main). Our pin base (2026-07-06) postdates the rename; a live scrape shows 65 upstream vllm: engine families plus 37 vllm_omni: omni families and zero vllm:omni_. Upstream's own docs still say vllm:omni_ -- they lag their code; that drift is upstream's to fix, not this fork's. The streaming families build every name from definitions.METRIC_PREFIX, the same constant every sibling omni family uses, so they conform to the existing convention by construction; the added sections state this explicitly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016vBNWkPjg5HgJoK2rKfuYY
Signed-off-by: Pete Tanski <YOUR_ID+pdtgct@users.noreply.github.com>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm-omni that referenced this pull request Sep 25, 2026
Signed-off-by: vraiti <vraiti@redhat.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend code related to entrypoint ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants