-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
[Metrics] Add --show-hidden-metrics-for-version
CLI arg
#13295
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
[Metrics] Add --show-hidden-metrics-for-version
CLI arg
#13295
Conversation
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
52c20bf
to
b02294b
Compare
Add some infrastructure to help us deprecate and remove metrics in a less user-hostile way. Our deprecation process will now be: 1) Deprecate the metric in 0.N.0 - document the deprecation in release notes, user-facing docs, and the help text in `/metrics` 2) Hide the metric in 0.N+1.0 - users can still re-enable the metrics using `--show-hidden-metrics-for-version=0.N.0` as an escape hatch 3) Remove the metric completely in 0.N+2.0 `--show-hidden-metrics` takes a version string argument so that users cannot fall into the habit of always enabling all deprecated metrics, which would defeat the purpose. This approach is copied directly from kubernetes/kubernetes#85270 Signed-off-by: Mark McLoughlin <[email protected]>
b02294b
to
b7ca51b
Compare
if __version_tuple__[0:2] == (0, 0): | ||
return True | ||
|
||
# Note - this won't do the right thing when we release 1.0! |
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.
Perhaps we could asset that the version is not 1.0 so that we catch this when upgrading?
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.
Can be done as a follow up.
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.
Very sensible
…ct#13295) Signed-off-by: Louis Ulmer <[email protected]>
Part of #10582 and discussed in #12745
Add some infrastructure to help us deprecate and remove metrics in a less user-hostile way.
Our deprecation process will now be:
release notes, user-facing docs, and the help text in
/metrics
metrics using
--show-hidden-metrics-for-version=0.N.0
as anescape hatch
--show-hidden-metrics
takes a version string argument so that users cannot fall into the habit of always enabling all deprecated metrics, which would defeat the purpose.This approach is copied directly from kubernetes/kubernetes#85270