[Misc][Metrics] expose requests preemptions in logger

[kingsmad]

Summary

The purpose of this PR is to store the num preemptions per step in the logger class locally thus it can be leveraged in children classes.

Currently when no new blocks available from each step, we already record this as request events and set it back to engine client by EngineCoreResponses which later got aggregated in the iteration stats.

Test Plan

1. Added a simple unit tests for iteration stats.
2. Run locally, saturate cache usage to 100%, we are able to see "llm.vllm.request.preemptions" popped up

Differential Revision: D82650207

[facebook-github-bot]

@kingsmad has exported this pull request. If you are a Meta employee, you can view the originating diff in D82650207.

[gemini-code-assist]

Code Review

This pull request aims to expose request preemption counts by tracking them in LoggingStatLogger. While the counter num_preempted_reqs is correctly added and updated, a critical issue exists where this value is reset within the log method before it is ever used. This bug prevents the preemption count from being exposed, defeating the purpose of the change. My review includes a comment detailing this issue.

[houseroad]

We probably need a better name for this PR, and add some unittest.

[yeqcharlotte]

please fix links in description to be public. also let's see tests to be added in https://github.com/vllm-project/vllm/blob/9607d5eb449711b349d4c2bee0a9c94afcc7ed14/tests/v1/metrics/test_engine_logger_apis.py

[kingsmad]

Addressed comments, thanks for the review!

1. Changed the PR title.
2. Added a simple unit test for recording iteration stats.
3. Updated the summary to use public code links.

[yeqcharlotte]

we seem to already have counter_num_preempted_reqs? can we use that? cc: @markmc

[kingsmad]

@yeqcharlotte Good point but currently the counter_num_preempted_reqs is in the PrometheusStatLogger and our predictor use our own loggers.

[yeqcharlotte]

since this change is relatively small. it's ok to let it through. we can follow up to see if we can reuse more.

[markmc]

I don't think we should add data into LoggingStatLogger if it is not used by LoggingStatLogger itself - there's no reason to incur this overhead in the upstream logger

Something like this would be equivalent

class PreemptionTrackingLogger(LoggingStatLogger):
    def __init__(self, vllm_config: VllmConfig, engine_index: int = 0):
        super().__init__(vllm_config, engine_index)
        self.total_preempted_reqs = 0

    def record(self,
               scheduler_stats: Optional[SchedulerStats],
               iteration_stats: Optional[IterationStats],
               engine_idx: int = 0):
        # Call parent record logic
        super().record(scheduler_stats, iteration_stats, engine_idx)

        # Track preempted requests
        if iteration_stats is not None:
            self.total_preempted_reqs += iteration_stats.num_preempted_reqs

    def log(self):
        # Run base logging first
        super().log()

        # Add preempted requests info
        logger.info(
            "Engine %03d: Total preempted requests so far: %d",
            self.engine_index,
            self.total_preempted_reqs,
        )