[Feature]: [V1] Parallel sampling should support metrics

[afeldman-nm]

🚀 The feature, motivation and pitch

PR #10980 added parallel sampling support. Currently, the v1 engine's parallel sampling capability does not support metrics: a request with n>1 returns a request output with null/None in the metrics field.

The desired behavior is for parallel sampling request outputs to include metrics; this may require defining how we want metrics to appear for parallel sampling requests in v1.

Alternatives

Currently a user must account for parallel sampling request outputs having no metrics.

Additional context

I was responsible for #10980 ; metrics support was challenging because each child request passes through the engine separately & accrues its own metrics. It was unclear how to synthesize one parent request metrics data structure from n child request metrics data structures, in part because I was not clear on the spec for how metrics are supposed to appear for parallel sampling requests.

Before submitting a new issue...

• Make sure you already searched for relevant issues, and asked the chatbot living at the bottom right corner of the documentation page, which can answer lots of frequently asked questions.

[afeldman-nm]

@markmc @WoosukKwon @robertgshaw2-redhat @njhill

[robertgshaw2-redhat]

Can you implement it?

[afeldman-nm]

Yes. Just wanted to track this issue & make relevant stakeholders who have worked on metrics aware that I will be looking for feedback.

[heheda12345]

@wanmok seems to be interested in implementing this feature. @afeldman-nm Can you share more background with him?

[markmc]

a request with n>1 returns a request output with null/None in the metrics field.

This is independent of parallel sampling. The RequestOutput.metrics field is not set in V1, it's not particular to parallel sampling.

#17010 is proposed as an alternate API for metrics

[wanmok]

a request with n>1 returns a request output with null/None in the metrics field.

This is independent of parallel sampling. The RequestOutput.metrics field is not set in V1, it's not particular to parallel sampling.

#17010 is proposed as an alternate API for metrics

Do you mean that it is not the metrics missing due to the parallel sampling but V1 has not supported metrics yet? which is being addressed in #17010 ?

[markmc]

a request with n>1 returns a request output with null/None in the metrics field.

This is independent of parallel sampling. The RequestOutput.metrics field is not set in V1, it's not particular to parallel sampling.
#17010 is proposed as an alternate API for metrics

Do you mean that it is not the metrics missing due to the parallel sampling but V1 has not supported metrics yet? which is being addressed in #17010 ?

V1 has not supported metrics though the API yet. Metrics are supported via the API server. And yes, #17010 proposes an API to access the metrics, in a way that is more consistent with the metrics supported via the API server.

[Sophie8]

@markmc Hi Mark and all, I am new to the repo and browsing first time feature requests to learn more about the code. Based on my research, I can see the parallel sampling metrics described at https://docs.vllm.ai/en/v0.8.5/design/v1/metrics.html#parallel-sampling looks implemented already.
iteration_stats records the max generated tokens for the same parent request id and
iteration_stats.n_params_iter also records the parameter n used for each group of children requests:

vllm/vllm/v1/engine/parallel_sampling.py

Line 131 in 9528e3a

iteration_stats.max_num_generation_tokens_iter.append(

at V0:
It saves the metrics thru logger do_log_stats:

vllm/vllm/engine/llm_engine.py

Line 1552 in 9528e3a

def do_log_stats(self,

It connects to and publish metrics thru Prometheus at:

vllm/vllm/engine/llm_engine.py

Line 362 in 9528e3a

self.stat_loggers = {

vllm/vllm/engine/metrics.py

Line 569 in 9528e3a

stats.max_num_generation_tokens_requests)

but at V1:
the logger connection to Prometheus is not fully implemented yet (?):

vllm/vllm/v1/engine/llm_engine.py

Line 62 in 9528e3a

if stat_loggers is not None:

with #17010 as the starting point for the migration.

so the remaining work is continue 17010 to fully implement and migrate the metrics support to V1 instead of the original description of this issue? Could people on the issue help confirming this is the case and is it still a good onboarding task to take?

Thank you very much!

[github-actions]

This issue has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this issue should remain open. Thank you!

[markmc]

I think the key question here is:

how to synthesize one parent request metrics data structure from n child request metrics data structures

AFAIR a request with n>1 is currently treated by the metrics recording as if it was simply n independent requests. I think @afeldman-nm opened this issue because he was concerned that this might not be the desired behavior. Personally, I haven't heard or I don't recall a reason it's not ok.