Repository navigation
[Bugfix][Doc] fix Prometheus per-stage audio metrics and drop RTF #4798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c8c04cd
125f5c2
d0a931e
59c728b
78bf724
5c4972d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,7 +43,6 @@ Emitted at request finalize, except for `audio_ttfp_s` (streaming-hook at the fi | |
| |--------|------|-------------|-------------| | ||
| | `vllm:omni_audio_ttfp_s` | Histogram | — | Time from request arrival to first audio packet/frame | | ||
| | `vllm:omni_audio_duration_s` | Histogram | — | Audio content duration (`audio_frames / sample_rate`) | | ||
| | `vllm:omni_audio_rtf` | Histogram | — | Real-time factor (`stage_gen_time_s / audio_duration_s`); streaming TTS SLO red line `< 1`; uses `RTF_BUCKETS` | | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same |
||
| | `vllm:omni_audio_frames_total` | Counter | — | Cumulative audio frame count; throughput via `rate()` | | ||
| | `vllm:omni_audio_underrun_s` | Histogram | — | Per-request worst-case player deficit; `> 0` indicates listener heard silent gaps | | ||
| | `vllm:omni_audio_continuity_ok_total` | Counter | `threshold_ms` | Incremented when the request's worst underrun stayed below `threshold_ms` | | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import pytest | ||
| import torch | ||
| from prometheus_client import REGISTRY, generate_latest | ||
|
|
||
| from vllm_omni.engine.mm_outputs import MultimodalPayload | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] The new import added by this PR at tests/metrics/test_modality.py:7, `from vllm… Evidence and suggested fixThe new import added by this PR at tests/metrics/test_modality.py:7, Evidence: Trigger: pytest collecting the PR-added tests/metrics/test_modality.py executes line 7 Suggestion: from vllm_omni.outputs.mm_outputs import MultimodalPayload |
||
| from vllm_omni.metrics import definitions as defs | ||
| from vllm_omni.metrics.modality import ( | ||
| OmniModalityMetrics, | ||
|
|
@@ -37,7 +39,6 @@ def _sample_value(output: str, line_prefix: str) -> float | None: | |
| _EXPECTED_FAMILIES = [ | ||
| defs.AUDIO_TTFP_S, | ||
| defs.AUDIO_DURATION_S, | ||
| defs.AUDIO_RTF_METRIC, | ||
| defs.AUDIO_FRAMES_METRIC, | ||
| defs.AUDIO_UNDERRUN_S, | ||
| defs.AUDIO_CONTINUITY_OK_METRIC, | ||
|
|
@@ -50,7 +51,6 @@ def test_all_locked_families_present(self, mod: OmniModalityMetrics) -> None: | |
| # Trigger at least one observation per family so the registry exposes them. | ||
| mod.observe_audio_ttfp("s", "r", 0.1) | ||
| mod.observe_audio_duration("s", "r", 1.0) | ||
| mod.observe_audio_rtf("s", "r", 0.5) | ||
| mod.inc_audio_frames("s", "r", 1) | ||
| mod.observe_audio_underrun("s", "r", 0.01) | ||
| mod.inc_audio_continuity_ok("s", "r", 100) | ||
|
|
@@ -81,13 +81,6 @@ def test_audio_duration_observed(self, mod: OmniModalityMetrics) -> None: | |
| prefix = f'{defs.AUDIO_DURATION_S}_sum{{model_name="{_MODEL}",replica="{replica}",stage="{stage}"}}' | ||
| assert _sample_value(out, prefix) == 3.5 | ||
|
|
||
| def test_audio_rtf_observed(self, mod: OmniModalityMetrics) -> None: | ||
| stage, replica = "talker_rtf", "0" | ||
| mod.observe_audio_rtf(stage, replica, 0.45) | ||
| out = generate_latest(REGISTRY).decode() | ||
| prefix = f'{defs.AUDIO_RTF_METRIC}_sum{{model_name="{_MODEL}",replica="{replica}",stage="{stage}"}}' | ||
| assert _sample_value(out, prefix) == 0.45 | ||
|
|
||
| def test_audio_frames_inc(self, mod: OmniModalityMetrics) -> None: | ||
| stage, replica = "talker_frames", "0" | ||
| mod.inc_audio_frames(stage, replica, 240) | ||
|
|
@@ -171,9 +164,6 @@ def inc_audio_frames(self, s, r, n): | |
| def observe_audio_duration(self, s, r, d): | ||
| self.calls.append(("observe_audio_duration", s, r, d)) | ||
|
|
||
| def observe_audio_rtf(self, s, r, rtf): | ||
| self.calls.append(("observe_audio_rtf", s, r, rtf)) | ||
|
|
||
| def observe_audio_underrun(self, s, r, u): | ||
| self.calls.append(("observe_audio_underrun", s, r, u)) | ||
|
|
||
|
|
@@ -192,8 +182,9 @@ def __init__(self, **kw): | |
| class TestObserveModalityAtFinalize: | ||
| def test_audio_path_full(self): | ||
| stub = _StubModMetrics() | ||
| stage_metrics = _Bag(stage_gen_time_ms=500.0, audio_generated_frames=24000) | ||
| engine_outputs = _Bag(multimodal_output={"audio_sample_rate": 24000}) | ||
| stage_metrics = _Bag(stage_gen_time_ms=500.0, audio_generated_frames=0) | ||
| payload = MultimodalPayload(tensors={"audio": torch.zeros(24000)}) | ||
| engine_outputs = _Bag(multimodal_output=payload) | ||
|
|
||
| observe_modality_at_finalize( | ||
| stub, | ||
|
|
@@ -203,15 +194,33 @@ def test_audio_path_full(self): | |
| stage_metrics=stage_metrics, | ||
| engine_outputs=engine_outputs, | ||
| ) | ||
| # 24000 frames / 24000 Hz = 1.0s duration; gen 0.5s → rtf 0.5 | ||
| # 24000 frames / 24000 Hz = 1.0s duration | ||
| assert ("inc_audio_frames", "1", "0", 24000) in stub.calls | ||
| assert ("observe_audio_duration", "1", "0", 1.0) in stub.calls | ||
| assert ("observe_audio_rtf", "1", "0", 0.5) in stub.calls | ||
| # Continuity/underrun NOT emitted from finalize — they come from the | ||
| # streaming hook because they need the per-chunk arrival timeline. | ||
| assert not any(c[0] == "observe_audio_underrun" for c in stub.calls) | ||
| assert not any(c[0] == "inc_audio_continuity_ok" for c in stub.calls) | ||
|
|
||
| def test_audio_path_nested_multimodal_payload(self): | ||
| stub = _StubModMetrics() | ||
| payload = MultimodalPayload(tensors={"audio": torch.zeros(24000)}) | ||
| engine_outputs = _Bag( | ||
| multimodal_output=MultimodalPayload(), | ||
| outputs=[_Bag(multimodal_output=payload)], | ||
| ) | ||
|
|
||
| observe_modality_at_finalize( | ||
| stub, | ||
| output_type="audio", | ||
| stage_id=1, | ||
| replica_id=0, | ||
| stage_metrics=_Bag(stage_gen_time_ms=500.0, audio_generated_frames=0), | ||
| engine_outputs=engine_outputs, | ||
| ) | ||
| assert ("inc_audio_frames", "1", "0", 24000) in stub.calls | ||
| assert ("observe_audio_duration", "1", "0", 1.0) in stub.calls | ||
|
|
||
| def test_audio_path_zero_frames_emits_skipped(self): | ||
| stub = _StubModMetrics() | ||
| observe_modality_at_finalize( | ||
|
|
@@ -220,11 +229,10 @@ def test_audio_path_zero_frames_emits_skipped(self): | |
| stage_id=1, | ||
| replica_id=0, | ||
| stage_metrics=_Bag(stage_gen_time_ms=300.0, audio_generated_frames=0), | ||
| engine_outputs=_Bag(multimodal_output={}), | ||
| engine_outputs=_Bag(multimodal_output=MultimodalPayload()), | ||
| ) | ||
| assert ("inc_audio_frames", "1", "0", 0) in stub.calls | ||
| assert not any(c[0] == "observe_audio_duration" for c in stub.calls) | ||
| assert not any(c[0] == "observe_audio_rtf" for c in stub.calls) | ||
| assert ("inc_audio_skipped", "1", "0", "no_audio_data") in stub.calls | ||
|
|
||
| def test_audio_uses_resolved_sample_rate_from_multimodal_output(self): | ||
|
|
@@ -376,13 +384,6 @@ def test_replica_none_skipped(self): | |
|
|
||
|
|
||
| class TestBucketSelection: | ||
| def test_audio_rtf_uses_rtf_buckets(self, mod: OmniModalityMetrics) -> None: | ||
| stage, replica = "talker_buckets", "0" | ||
| mod.observe_audio_rtf(stage, replica, 0.5) | ||
| out = generate_latest(REGISTRY).decode() | ||
| rtf_marker = f'{defs.AUDIO_RTF_METRIC}_bucket{{le="0.9"' | ||
| assert rtf_marker in out, "audio_rtf should use RTF_BUCKETS containing le=0.9" | ||
|
|
||
| def test_audio_ttfp_uses_seconds_buckets(self, mod: OmniModalityMetrics) -> None: | ||
| stage, replica = "talker_seconds", "0" | ||
| mod.observe_audio_ttfp(stage, replica, 0.1) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] This diff removes vllm:omni_audio_rtf entirely at head — vllm_omni/metrics/defi…
Evidence and suggested fix
This diff removes vllm:omni_audio_rtf entirely at head — vllm_omni/metrics/definitions.py:128-130 now lists only AUDIO_TTFP_S/AUDIO_DURATION_S/AUDIO_FRAMES_METRIC (no AUDIO_RTF_METRIC, RTF_BUCKETS, or observe_audio_rtf anywhere in vllm_omni/), and docs/design/metrics.md:170 reads '### Audio (6)' — yet the maintainer thread is not resolved: @Gaohan123 said 'I think here it is the general metric rather than stage-level. So we should keep it' and the inline reply 'reverted' does not match the head state, where the family is still fully gone. Observability consequence: after this change no /metrics family exposes audio-stage generation time — stage_gen_time_ms surfaces only in the stats logger (vllm_omni/metrics/stats.py:473), not in prometheus.py — so the streaming-TTS SLO (<1) is no longer computable server-side; RTF survives only bench-side via defs.AUDIO_RTF/defs.compute_audio_rtf (vllm_omni/benchmarks/metrics/metrics.py:117, vllm_omni/benchmarks/patch/patch.py:968; benchmarks/tts selects it as a percentile metric at bench_tts.py:129). Either restore RTF as a stage-global metric per the maintainer request, or record explicit maintainer sign-off on dropping the server-side family in the PR description.
Evidence: Anchor at PR-time head: docs/design/metrics.md:170
### Audio (6); its table rows (176-181) list ttfp_s, duration_s, frames_total, underrun_s, continuity_ok_total, skipped_requests_total — no audio_rtf row. Removal complete: vllm_omni/metrics/definitions.py:128-130AUDIO_TTFP_S = METRIC_PREFIX + AUDIO_TTFP + "_s"/AUDIO_DURATION_S = METRIC_PREFIX + AUDIO_DURATION + "_s"/AUDIO_FRAMES_METRIC = METRIC_PREFIX + AUDIO_FRAMES(AUDIO_RTF_METRIC absent); repo-wide grep finds zero hits for AUDIO_RTF_METRIC, RTF_BUCKETS, or observe_audio_rtf under vllm_omni/ at head. Observability half — unchanged by this diff, present at PR-time head: vllm_omni/metrics/stats.py:473defs.STAGE_GEN_TIME_MS: float(evt.stage_gen_time_ms),(stats logger only), while vllm_omni/metrics/prometheus.py:7-39 registers only running/waiting/requests_success/e2e_latency/prompt+generation-token families, so no /metrics family carries audio-stage generation time and RTF<1 cannot be computed server-side. Bench-side RTF survives: vllm_omni/metrics/definitions.py:31AUDIO_RTF = "audio_rtf", definitions.py:254def compute_audio_rtf(audio_generation_latency_s: float, audio_duration_s: float) -> float:, consumed at vllm_omni/benchmarks/patch/patch.py:968output.audio_rtf = defs.compute_audio_rtf(audio_generate_time, audio_duration). The quoted maintainer thread is external PR-review data from the fetch step (not in-tree); the head-state half of the claim was verified by reading the tree.