✨: Add custom Prometheus metrics for controller observability - #3108
✨: Add custom Prometheus metrics for controller observability#3108MaxRink wants to merge 1 commit into
Conversation
|
Hi @MaxRink. Thanks for your PR. I'm waiting for a metal3-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
tuminoid
left a comment
There was a problem hiding this comment.
/ok-to-test
I think @fmuyassarov had something like this brewing as well?
/cc @lentzi90 @adilGhaffarDev @kashifest @fmuyassarov
180f512 to
2224fa7
Compare
Yes, I was doing some custom metrics addition. But this might be a good start. I will take a look today/tomorrow at this patch. |
|
Can you rebase before we start reviewing? |
|
ping @MaxRink |
There was a problem hiding this comment.
Pull request overview
Adds a centralized custom Prometheus metrics package and instruments CAPM3 controllers to improve observability into reconcile performance, provisioning workflows, and error rates.
Changes:
- Introduces
internal/metricswith CAPM3-specific counters/histograms/gauges and helper recording functions. - Instruments multiple controllers to record reconcile duration/result metrics, plus additional Metal3Machine provisioning and BMH association metrics.
- Adds metrics documentation (
docs/metrics.md) and basic unit tests for metric helper functions.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
internal/metrics/metrics.go |
Defines and registers CAPM3 Prometheus metrics plus helper recording functions. |
internal/metrics/metrics_test.go |
Adds unit tests covering metric helper entrypoints. |
docs/metrics.md |
Documents exposed metrics, labels, and example PromQL queries. |
go.mod |
Promotes prometheus/client_golang to a direct dependency. |
controllers/metal3machine_controller.go |
Adds reconcile + BMH association + provisioning duration instrumentation. |
controllers/metal3machine_controller_test.go |
Updates mocks/calls to accommodate additional provisioning/metrics logic. |
controllers/metal3cluster_controller.go |
Adds reconcile + error instrumentation. |
controllers/metal3data_controller.go |
Adds reconcile + error instrumentation. |
controllers/metal3datatemplate_controller.go |
Adds reconcile + error instrumentation. |
controllers/metal3remediation_controller.go |
Adds reconcile + error instrumentation. |
controllers/metal3machinetemplate_controller.go |
Adds reconcile + error instrumentation. |
controllers/metal3labelsync_controller.go |
Adds reconcile instrumentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@fmuyassarov needs your review |
98d3a43 to
8d8e2d2
Compare
adilGhaffarDev
left a comment
There was a problem hiding this comment.
/approved
thanks @MaxRink
One minor comment but not a blocker.
/hold
holding so others can review.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adilGhaffarDev The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
8404827 to
4e0dc65
Compare
|
/test metal3-centos-e2e-integration-test-main metal3-ubuntu-e2e-integration-test-main |
lentzi90
left a comment
There was a problem hiding this comment.
Looking good overall! I have a couple of comments on passing through the function to record reconciliation errors. This I would like to get rid of so we just do it with the defer. What do you think about this?
lentzi90
left a comment
There was a problem hiding this comment.
Could you please squash the commits?
|
/test metal3-ubuntu-e2e-integration-test-main metal3-centos-e2e-integration-test-main |
| @@ -0,0 +1,460 @@ | |||
| /* | |||
| Copyright 2024 The Kubernetes Authors. | |||
| @@ -0,0 +1,275 @@ | |||
| /* | |||
| Copyright 2024 The Kubernetes Authors. | |||
Add comprehensive Prometheus metrics to all CAPM3 controllers for visibility into reconciliation performance, provisioning workflows, and error rates. Metrics added: - Reconcile counters and duration histograms for all 7 controllers - BMH association duration tracking - Machine provisioning duration (creation to Ready) - Phase transition counters - Error counters by controller type Also adds metrics documentation with example PromQL queries and alerting rules. Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
New changes are detected. LGTM label has been removed. |
What this PR does
Adds custom Prometheus metrics to all CAPM3 controllers, addressing the lack of visibility into provisioning times, error rates, and controller performance.
Why we need it
Previously, CAPM3 only exposed default controller-runtime metrics, making it difficult to:
Changes
New files
internal/metrics/metrics.go- Central metrics package with 22 Prometheus metricsinternal/metrics/metrics_test.go- Unit tests for all helper functions (14 tests)docs/metrics.md- Comprehensive documentation with PromQL examplesModified controllers
All controllers instrumented with reconcile metrics:
metal3machine_controller.go- Also tracks BMH association and provisioning durationmetal3cluster_controller.gometal3data_controller.gometal3datatemplate_controller.gometal3remediation_controller.gometal3machinetemplate_controller.gometal3labelsync_controller.goMetrics overview
capm3_bmh_association_total,capm3_bmh_association_duration_secondscapm3_metal3machine_provisioning_duration_seconds(creation → Ready)capm3_reconcile_errors_totalby controllercapm3_metal3machine_phase_transitions_totalHow to test