From 5899651b7d3ed8a61a86ae571eedd2f0fb8234b9 Mon Sep 17 00:00:00 2001 From: weireweire <20922698+weireweire@users.noreply.github.com> Date: Tue, 6 Oct 2026 21:13:36 +0800 Subject: [PATCH] fix(server-metrics): tag the agentic warmup-start baseline scrape as warmup Root cause: the AGENTIC_REPLAY auto-warmup CreditPhaseConfig is synthesized without a phase_kind, and ServerMetricsManager.collect_baseline mapped any kind other than "warmup" to PROFILING. The warmup-start baseline scrape was therefore tagged PROFILING. For every counter series that already existed at that scrape (for example sglang:prompt_tokens on a prefill worker that served startup requests), the phase-filtered profiling series started at warmup start and its first profiling delta absorbed the whole warmup increment. Series that first appeared during warmup (sglang:cached_tokens, decode counters) were unaffected, so ratios such as the prefix cache hit rate were skewed: on a GB200 DeepSeek-V4 c1280 AgentX run, prefill prompt_tokens included exactly the warmup total (1,258,951,023 tokens), and the reported overall hit rate was 89.6% against 96.3% in the profiling window. Fix: give the synthesized agentic warmup phase_kind="warmup"; carry the credit phase on PhaseBaselineRequestMessage and let collect_baseline fall back to it when phase_kind is unset, instead of defaulting to PROFILING. Validation: new tests cover the agentic warmup kind and baseline phase tagging for set and unset kinds. tests/unit/server_metrics, tests/unit/timing and tests/unit/records: 2359 passed, 1 failed (test_records_manager::test_completed_excludes_analyzer_metrics, unrelated scenario MagicMock lookup; checked against the base commit). Signed-off-by: weireweire <20922698+weireweire@users.noreply.github.com> --- .../common/messages/baseline_messages.py | 6 ++- src/aiperf/server_metrics/manager.py | 14 ++++-- src/aiperf/timing/config.py | 3 ++ src/aiperf/timing/phase/publisher.py | 1 + .../test_server_metrics_manager.py | 49 ++++++++++++++++++- .../test_phase_config_agentic_replay.py | 3 ++ 6 files changed, 69 insertions(+), 7 deletions(-) diff --git a/src/aiperf/common/messages/baseline_messages.py b/src/aiperf/common/messages/baseline_messages.py index ffb1a91b57..bf564669af 100644 --- a/src/aiperf/common/messages/baseline_messages.py +++ b/src/aiperf/common/messages/baseline_messages.py @@ -6,7 +6,7 @@ from pydantic import Field -from aiperf.common.enums import BaselineKind, MessageType +from aiperf.common.enums import BaselineKind, CreditPhase, MessageType from aiperf.common.messages.base_messages import Message from aiperf.common.types import MessageTypeT, PhaseKind @@ -34,6 +34,10 @@ class PhaseBaselineRequestMessage(Message): phase_kind: PhaseKind | None = Field( default=None, description="Semantic phase kind: warmup or profiling." ) + phase: CreditPhase | None = Field( + default=None, + description="Credit phase of the boundary; used when phase_kind is unset.", + ) kind: BaselineKind = Field( ..., description="START before credits are issued; END after returns drain." ) diff --git a/src/aiperf/server_metrics/manager.py b/src/aiperf/server_metrics/manager.py index 583e441366..5beb990609 100644 --- a/src/aiperf/server_metrics/manager.py +++ b/src/aiperf/server_metrics/manager.py @@ -216,11 +216,15 @@ async def collect_baseline(self, message: PhaseBaselineRequestMessage) -> None: """Capture a one-shot server-metrics scrape for a phase boundary.""" if self._server_metrics_disabled or not self._collectors: return - boundary_phase = ( - CreditPhase.WARMUP - if message.phase_kind == "warmup" - else CreditPhase.PROFILING - ) + # Tag the scrape with its own phase. Falling back to PROFILING for an + # unset kind put a warmup-start scrape into the profiling series, so + # counters that already existed then counted the whole warmup. + if message.phase_kind == "warmup": + boundary_phase = CreditPhase.WARMUP + elif message.phase_kind == "profiling": + boundary_phase = CreditPhase.PROFILING + else: + boundary_phase = message.phase or CreditPhase.PROFILING errors: list[str] = [] for endpoint_url, collector in list(self._collectors.items()): try: diff --git a/src/aiperf/timing/config.py b/src/aiperf/timing/config.py index 50e662192f..bc6983fe79 100644 --- a/src/aiperf/timing/config.py +++ b/src/aiperf/timing/config.py @@ -643,6 +643,9 @@ def _build_agentic_warmup_config(phase: PhaseConfig) -> CreditPhaseConfig | None total_expected_requests = concurrency return CreditPhaseConfig( phase=CreditPhase.WARMUP, + # Synthesized phases have no PhaseConfig.kind; consumers that key on + # phase_kind (server-metrics phase-boundary baselines) need it set. + phase_kind="warmup", timing_mode=TimingMode.AGENTIC_REPLAY, # Duration mode is strategy-terminated by its timer. Count mode uses # the generic request-count stop condition as a global backstop while diff --git a/src/aiperf/timing/phase/publisher.py b/src/aiperf/timing/phase/publisher.py index 5064d0197b..eb8a615b94 100644 --- a/src/aiperf/timing/phase/publisher.py +++ b/src/aiperf/timing/phase/publisher.py @@ -74,6 +74,7 @@ async def publish_phase_baseline_request( phase_name=config.phase_name or f"{config.phase.value}_{config.phase_index}", phase_kind=config.phase_kind, + phase=config.phase, kind=kind, ) await self._pub_client.publish(msg) diff --git a/tests/unit/server_metrics/test_server_metrics_manager.py b/tests/unit/server_metrics/test_server_metrics_manager.py index 2bec3384e1..3a7c8616d7 100644 --- a/tests/unit/server_metrics/test_server_metrics_manager.py +++ b/tests/unit/server_metrics/test_server_metrics_manager.py @@ -21,7 +21,10 @@ CreditPhaseStartMessage, ) from aiperf.plugin.enums import EndpointType, TimingMode -from aiperf.server_metrics.manager import ServerMetricsManager +from aiperf.server_metrics.manager import ( + _SERVER_METRICS_SCRAPE_PHASE, + ServerMetricsManager, +) from aiperf.timing.config import CreditPhaseConfig from tests.unit.conftest import make_run_from_cli @@ -667,6 +670,50 @@ async def slow_scrape(): assert manager._active_phase == CreditPhase.WARMUP + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("phase_kind", "phase", "expected"), + [ + ("warmup", None, CreditPhase.WARMUP), + ("profiling", None, CreditPhase.PROFILING), + # Synthesized phases (agentic auto-warmup) may carry no kind; the + # credit phase must decide, not a PROFILING default. + (None, CreditPhase.WARMUP, CreditPhase.WARMUP), + (None, CreditPhase.PROFILING, CreditPhase.PROFILING), + ], + ) + async def test_baseline_scrape_is_tagged_with_boundary_phase( + self, + cfg_with_endpoint: CLIConfig, + phase_kind, + phase, + expected, + ): + manager = ServerMetricsManager( + run=make_run_from_cli(cfg_with_endpoint), + ) + seen: list[CreditPhase | None] = [] + + async def scrape(): + seen.append(_SERVER_METRICS_SCRAPE_PHASE.get()) + + mock_collector = MagicMock() + mock_collector.collect_and_process_metrics = AsyncMock(side_effect=scrape) + manager._collectors = {"http://localhost:8000/metrics": mock_collector} + + await manager.collect_baseline( + PhaseBaselineRequestMessage( + service_id="timing-manager", + phase_id="phase-0", + phase_name="warmup_0", + phase_kind=phase_kind, + phase=phase, + kind=BaselineKind.START, + ) + ) + + assert seen == [expected] + class TestDisabledServerMetrics: """Test server metrics disabled scenarios.""" diff --git a/tests/unit/timing/test_phase_config_agentic_replay.py b/tests/unit/timing/test_phase_config_agentic_replay.py index 7baf752b74..f97f2e93f4 100644 --- a/tests/unit/timing/test_phase_config_agentic_replay.py +++ b/tests/unit/timing/test_phase_config_agentic_replay.py @@ -35,6 +35,9 @@ def test_warmup_config_uses_agentic_replay_when_top_level_is_agentic_replay() -> assert warmup is not None assert warmup.timing_mode == TimingMode.AGENTIC_REPLAY assert warmup.phase == CreditPhase.WARMUP + # Server-metrics phase-boundary baselines key on phase_kind; an unset kind + # tagged the warmup-start scrape as profiling. + assert warmup.phase_kind == "warmup" def test_profiling_config_propagates_cap() -> None: