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.

Sign up for GitHub

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

Jump to bottom

Create new metric which conform to our naming conventions #103799

Merged
merged 1 commit into from Aug 18, 2021
Merged

Create new metric which conform to our naming conventions #103799

merged 1 commit into from Aug 18, 2021

Conversation

jyz0309
Copy link
Contributor

@jyz0309 jyz0309 commented Jul 20, 2021 •
edited

Signed-off-by: jyz0309 45495947@qq.com

add comment

Signed-off-by: jyz0309 45495947@qq.com

What type of PR is this?

/kind cleanup
/kind deprecation

What this PR does / why we need it:

According to the #103441 , we want to deprecate apiserver_longrunning_gauge and apiserver_register_watchers metrics and create a new one which actually does conform to our naming conventions probably something like apiserver_longrunning_requests.
And #103793 have deprecated metrics, and this pr will create a new metric to replace apiserver_longrunning_gauge and conform to our naming conventions.

Which issue(s) this PR fixes:

Fixes #103441

Special notes for your reviewer:

Does this PR introduce a user-facing change?

add apiserver_longrunning_requests metric to replace the soon to be deprecated apiserver_longrunning_gauge metric.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 20, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @jyz0309. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 20, 2021
@k8s-ci-robot k8s-ci-robot added area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 20, 2021
@jyz0309
Copy link
Contributor Author

jyz0309 commented Jul 20, 2021

/sig instrumentation

@k8s-ci-robot k8s-ci-robot added the sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. label Jul 20, 2021
@jyz0309
Copy link
Contributor Author

jyz0309 commented Jul 20, 2021 •
edited

@jimmidyson @wojtek-t @logicalhan PTAL~

@fedebongio
Copy link
Contributor

/assign @logicalhan
/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 20, 2021
@dgrisonnet
Copy link
Member

@jyz0309 I think it would be great to add a release note to mention that the apiserver_longrunning_requests metric is replacing the soon to be deprecated apiserver_longrunning_gauge metric.

@dgrisonnet
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jul 22, 2021
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. release-note-none Denotes a PR that doesn't merit a release note. labels Jul 22, 2021
@jyz0309
Copy link
Contributor Author

jyz0309 commented Jul 22, 2021

/retest-required

@jyz0309
add new metric
97ba475 
Signed-off-by: jyz0309 <45495947@qq.com>

add comment

Signed-off-by: jyz0309 <45495947@qq.com>

add promlint

Signed-off-by: jyz0309 <45495947@qq.com>

address comment

Signed-off-by: jyz0309 <45495947@qq.com>

go fmt code

Signed-off-by: jyz0309 <45495947@qq.com>
@jyz0309
Copy link
Contributor Author

jyz0309 commented Aug 6, 2021

/retest-required

@dgrisonnet
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2021
@jyz0309
Copy link
Contributor Author

jyz0309 commented Aug 17, 2021

It seem that the relate pr has been merged.#103793 @dgrisonnet Can you help me merge this pr?

@dgrisonnet
Copy link
Member

We would need an approver to move forward with this PR. @logicalhan can you have look?

logicalhan
logicalhan approved these changes Aug 18, 2021
View reviewed changes
Copy link
Member

@logicalhan logicalhan left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jyz0309, logicalhan

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

The pull request process is described 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 approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 18, 2021
@k8s-ci-robot k8s-ci-robot merged commit 58617e1 into kubernetes:master Aug 18, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Aug 18, 2021
@alecgarza96
Copy link

Came across issues #102545 #103441 and a couple others referencing duplicate watch count metrics. These issues all seem to be linked, however, I am new to open source contributions. Are duplicate watch count metrics still an issue?

Based off my reading, it seems as if it was deemed to risky to remove the duplicates and instead they were chosen to be deprecated and a new standard implemented.

Essentially, I'd like to contribute to this issue, but I'm not sure what direction to move in.

Thank you

@arapulido arapulido mentioned this pull request Jun 23, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgrisonnet dgrisonnet dgrisonnet left review comments

@logicalhan logicalhan logicalhan approved these changes

@jimmidyson jimmidyson Awaiting requested review from jimmidyson

@wojtek-t wojtek-t Awaiting requested review from wojtek-t

Assignees

@logicalhan logicalhan

@dgrisonnet dgrisonnet

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.23
Development

Successfully merging this pull request may close these issues.

Deprecate apiserver_longrunning_guage and apiserver_registered_watchers in favor of a new metric
6 participants
@jyz0309 @k8s-ci-robot @fedebongio @dgrisonnet @alecgarza96 @logicalhan