Skip to content
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

Graduate Metrics API to GA #5208

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ameukam
Copy link
Member

@ameukam ameukam commented Mar 17, 2025

  • One-line PR description:
  • Issue link:
  • Other comments:

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 17, 2025
@k8s-ci-robot k8s-ci-robot requested a review from dgrisonnet March 17, 2025 22:37
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ameukam
Once this PR has been reviewed and has the lgtm label, please assign logicalhan for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory label Mar 17, 2025
@k8s-ci-robot k8s-ci-robot added sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 17, 2025
@ameukam ameukam mentioned this pull request Mar 17, 2025
4 tasks
Copy link

@sftim sftim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try keps/sig-instrumentation/api-for-internal-metrics/

We try not to call features "Graduate X to GA";we just call them "X".

Comment on lines +3 to +4
name: Graduate metrics.k8s.io API to GA
title: Graduate metrics.k8s.io API to GA
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name: Graduate metrics.k8s.io API to GA
title: Graduate metrics.k8s.io API to GA
name: API for internal metrics integration
title: API for internal metrics integration

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"internal metrics" ? What do you mean ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can pick a different name; the key thing is to drop "Graduate" from the name of the enhancement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On several of those, I've tried to intervene to fix what I see as problematic KEP naming.

#5208 (comment)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KEP-19 for example is now titled CronJobs (previously ScheduledJobs)

Signed-off-by: Arnaud Meukam <[email protected]>
@BenTheElder
Copy link
Member

We try not to call features "Graduate X to GA";we just call them "X".

We actually do tend to call "graduate X" when we're plotting how to deal with existing functionality that has failed to graduate previously. There are many examples of htis.

We don't do this when it's a net-new feature.

I actually think it's more clear to note that we're not creating a new metrics API, we're discussing how to stabilize the current one.

Calling this "Metrics API" would be pretty misleading, IMHO. Calling it "API for Internal Metrics" isn't any better, it reads like we're creating a new API.

@ameukam
Copy link
Member Author

ameukam commented Mar 26, 2025

how to stabilize the current one.

The API metrics concerned here have been stable for a few years. Are there specific criteria we need to meet ?

@BenTheElder
Copy link
Member

The API metrics concerned here have been stable for a few years. Are there specific criteria we need to meet ?

Nothing specific -- I don't know why we haven't GA-ed yet, I mean to the point about framing the KEP, and past, similar KEPs.

@sftim
Copy link

sftim commented Mar 27, 2025

We actually do tend to call "graduate X" when we're plotting how to deal with existing functionality that has failed to graduate previously. There are many examples of htis.

Yes, but we shouldn't, because then when you promote it from beta to GA you end up with "Promote Graduate X to Stable
from beta to GA" as a PR title. Or other problems of conflating the graduation with the feature.

@BenTheElder
Copy link
Member

Yes, but we shouldn't, because then when you promote it from beta to GA you end up with "Promote Graduate X to Stable
from beta to GA" as a PR title. Or other problems of conflating the graduation with the feature.

We don't have to name the PRs that way.
I disagree, I think it's much clearer to note that the scope of the KEP is stabilizing the feature and the proposed alternatives make it sound like a new API.
Not to mention established conventions are familiar.

But that discussion should probably be taken to sig-architecture. I don't think we should be trying to establish a new pattern by bikeshedding individual KEP issue(s) / PR(s). I haven't seen any prior discussion that we should alter this pattern.

@sftim
Copy link

sftim commented Mar 28, 2025

Hmm; the pattern of "name KEP after the enhancement it provides" is IMO well established and documented, even if we don't always follow the pattern.

@sftim
Copy link

sftim commented Mar 28, 2025

the proposed alternatives make it sound like a new API.

Ah, here's the thing. I think of the audience, for KEPs, being "Kubernetes end users who want to know what the enhancement is", and maybe @BenTheElder you think of the audience being "Kubernetes contributors" or "subset of Kubernetes contributors".

I'm thinking of what https://www.kubernetes.dev/resources/keps/ should look like.

@sftim
Copy link

sftim commented Mar 28, 2025

I'm not proposing that we retrofit existing KEPs. For example, https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/95-custom-resource-definitions/kep.yaml doesn't gain metadata about alpha and beta stages.

Maybe we could do that as well, but that's not what I'm proposing. Just a change to the title of this provisional KEP.

@pohly
Copy link
Contributor

pohly commented Mar 28, 2025

Whatever it's going to be called, please use 5027-<title-with-no-spaces> as directory name.

The way it is now is just odd:

https://github.com/kubernetes/enhancements/tree/d6ee32ff9f5cf7d8f3748518e9d2a8ae58658c01/keps/sig-instrumentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants