8000 feat(metrics): adding `certmanager_certificate_challenge_status` metric by hjoshi123 · Pull Request #7736 · cert-manager/cert-manager · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(metrics): adding certmanager_certificate_challenge_status metric #7736

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
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

feat(metrics): adding certmanager_certificate_challenge_status metric #7736

wants to merge 4 commits into from

Conversation

hjoshi123
Copy link
@hjoshi123 hjoshi123 commented May 6, 2025 •
edited
Loading

Pull Request Motivation

feature

Fixes #7700. This PR adds the new challenge status metric with a controller which monitors the challenges.

/kind

Release Note

Added `certmanager_certificate_challenge_status` prometheus metric.

@hjoshi123
adding unit test and controller for cert challenge
dd5c269 
Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>
@cert-manager-prow
Copy link
Contributor

@hjoshi123: The label(s) kind/release, kind/note cannot be applied, because the repository doesn't have them.

In response to this:

Pull Request Motivation

Feature

#7700. This PR adds the new challenge status metric with a controller which monitors the challenges.

/kind

Release Note

Added `certmanager_certificate_challenge_status` prometheus metric.

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-sigs/prow repository.

@cert-manager-prow cert-manager-prow bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. labels May 6, 2025
@cert-manager-prow
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign inteon 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

@cert-manager-prow cert-manager-prow bot added area/acme Indicates a PR directly modifies the ACME Issuer code needs-kind Indicates a PR lacks a `kind/foo` label and requires one. area/monitoring Indicates a PR or issue relates to monitoring needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 6, 2025
@cert-manager-prow
Copy link
Contributor

Hi @hjoshi123. Thanks for your PR.

I'm waiting for a cert-manager 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-sigs/prow repository.

@cert-manager-prow cert-manager-prow bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 6, 2025
@hjoshi123
Copy link
Author

/kind feature

@cert-manager-prow cert-manager-prow bot added kind/feature Categorizes issue or PR as related to a new feature. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels May 6, 2025
ThatsMrTalbot
ThatsMrTalbot reviewed May 15, 2025
View reviewed changes
erikgb
erikgb reviewed May 15, 2025
View reviewed changes
@ThatsMrTalbot
Copy link
Contributor

/ok-to-test

@cert-manager-prow cert-manager-prow bot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 15, 2025
ThatsMrTalbot
ThatsMrTalbot reviewed May 20, 2025
View reviewed changes
ThatsMrTalbot
ThatsMrTalbot reviewed May 20, 2025
View reviewed changes
@cert-manager-prow cert-manager-prow bot added area/testing Issues relating to testing dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. and removed dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. labels May 20, 2025
@hjoshi123
added uid for deletion and removed value
501a401 
Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>
@hjoshi123 hjoshi123 force-pushed the feat/cert-challenge-status-metrics branch from 2812dfc to 501a401 Compare May 20, 2025 14:47
@cert-manager-prow cert-manager-prow bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels May 20, 2025
@hjoshi123
added license to acme_test
2e50b3d 
Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>
@hjoshi123
Copy link
Author

@ThatsMrTalbot there was also talk of adding age of challenge as a label.. I was thinking of a different. metric for that since this metric is only supposed to tell the status.. what's your take on that?

@ThatsMrTalbot
Copy link
Contributor

@ThatsMrTalbot there was also talk of adding age of challenge as a label.. I was thinking of a different. metric for that since this metric is only supposed to tell the status.. what's your take on that?

Age should not be a label, age is a constantly changing value, having it as a label would introduce high cardinality

@hjoshi123
Copy link
Author

@ThatsMrTalbot there was also talk of adding age of challenge as a label.. I was thinking of a different. metric for that since this metric is only supposed to tell the status.. what's your take on that?

Age should not be a label, age is a constantly changing value, having it as a label would introduce high cardinality

Ah yes I didnt think of that.. so how would we solve it if we wanted to? it doesnt have to be this PR I am guessing but in general.

@hjoshi123
fixing license issues
d9b1594 
Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>
@ThatsMrTalbot
Copy link
Contributor

Ah yes I didnt think of that.. so how would we solve it if we wanted to? it doesnt have to be this PR I am guessing but in general.

The way I have seen time metrics work in the past is having a metric that contains a unix timestamp, for example kube_cronjob_status_last_schedule_time in kube-state-metric. These kind of metrics can be used with the time function to get relative time (time() - metric value).

@ThatsMrTalbot
Copy link
Contributor

The code looks good, I want to run it locally, make sure it behaves as the code reads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erikgb erikgb erikgb left review comments

@ThatsMrTalbot ThatsMrTalbot ThatsMrTalbot left review comments

Assignees
No one assigned
Labels
area/acme Indicates a PR directly modifies the ACME Issuer code area/monitoring Indicates a PR or issue relates to monitoring area/testing Issues relating to testing dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/feature Categorizes issue or PR as related to a new feature. ok-to-test release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expose metrics regarding challenge status
3 participants
@hjoshi123 @ThatsMrTalbot @erikgb
0