KEP: Refactoring of Prometheus metrics #4352
Conversation
aledbf
added
the
do-not-merge/work-in-progress
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf 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 |
k8s-ci-robot
added
the
approved
Codecov Report
@@ Coverage Diff @@
## master #4352 +/- ##
==========================================
- Coverage 58.48% 58.47% -0.02%
==========================================
Files 87 87
Lines 6528 6528
==========================================
- Hits 3818 3817 -1
+ Misses 2283 2282 -1
- Partials 427 429 +2
Continue to review full report at Codecov.
|
aledbf
added
the
do-not-merge/work-in-progress
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
lifecycle/rotten
aledbf
removed
the
lifecycle/rotten
|
@aledbf: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
the
lifecycle/stale
aledbf
removed
the
lifecycle/stale
|
|
||
| |Field name|Description| | ||
| | --- | --- | | ||
| |uid| Unique identifier of the ingress. This field is the key for the joins| |
There was a problem hiding this comment.
This should be ingress_uid to match the label name on the metrics themselves. Joining across metrics with mismatched label name requires using label_replace() to correct the difference at query time, or via relabelling to correct it at scrape time.
There was a problem hiding this comment.
Add a comment about issue #3818 that requests status_class or status_code as a label to be able to filter the metrics in databases using the prometheus format, but not using PromQL (an example: Datadog implements OpenMetrics format, but lacks PromQL support)
| - https://github.com/kubernetes/ingress-nginx/pull/4139 | ||
| - https://github.com/kubernetes/ingress-nginx/issues/2924 | ||
| - https://github.com/kubernetes/ingress-nginx/issues/3898 | ||
| - https://github.com/kubernetes/ingress-nginx/pull/4139 |
There was a problem hiding this comment.
An additional issue #3818 can be solved by incorporating a status_class:5xx in the metric implemented as part of this proposal
| - ingress_uid # label to join with the ingress info metric using group_left | ||
| - path # same ^^ | ||
| - method | ||
| - status |
There was a problem hiding this comment.
An additional issue #3818 can be solved by incorporating a status_class:5xx in the metric implemented as part of this proposal
|
|
||
| - ingress_uid | ||
| - method | ||
| - status |
There was a problem hiding this comment.
An additional issue #3818 can be solved by incorporating a status_class:5xx in the metric implemented as part of this proposal
| - controller_pod | ||
| - ingress_uid | ||
| - method | ||
| - status |
There was a problem hiding this comment.
An additional issue #3818 can be solved by incorporating a status_class:5xx in the metric implemented as part of this proposal
| - controller_pod | ||
| - ingress_uid | ||
| - method | ||
| - status |
There was a problem hiding this comment.
An additional issue #3818 can be solved by incorporating a status_class:5xx in the metric implemented as part of this proposal
|
|
||
| - controller_pod | ||
| - ingress_uid | ||
| - status |
There was a problem hiding this comment.
An additional issue #3818 can be solved by incorporating a status_class:5xx in the metric implemented as part of this proposal
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
lifecycle/rotten
|
Rotten issues close after 30d of inactivity. Send feedback to sig-contributor-experience at kubernetes/community. |
|
@fejta-bot: Closed this PR. In response to this:
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. |
|
Any chance we can get this merged? Not being able to look at canary-specific metrics makes canary analysis a lot harder. #364, #5650 and fluxcd/flagger#382 are affected by this. |
|
Can we help in any sort to move forward? |
|
/reopen |
|
@L3o-pold: You can't reopen an issue/PR unless you authored it or you are a collaborator. In response to this:
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. |
|
/reopen |
|
@iamNoah1: Failed to re-open PR: state cannot be changed. The master branch has been deleted. In response to this:
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. |
What this PR does / why we need it:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer: