Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

feat(metrics): add osm_reconciliation_total metric #4788

Merged
merged 1 commit into from
Jun 6, 2022
Merged

feat(metrics): add osm_reconciliation_total metric #4788

merged 1 commit into from
Jun 6, 2022

Conversation

nojnhuh
Copy link
Contributor

@nojnhuh nojnhuh commented Jun 3, 2022

Description:
This change adds an osm_reconciliation_total metric counting how many
times the reconciler has been triggered for each kind.

Fixes #4568

Example:

# HELP osm_reconciliation_total Counter of resource reconciliations invoked
# TYPE osm_reconciliation_total counter
osm_reconciliation_total{kind="ValidatingWebhookConfiguration"} 9

Testing done:

  • Updated unit tests
  • Did some manual smoke testing with the whole control plane running while changing/deleting reconciled resources

Affected area:

Functional Area
New Functionality [ ]
CI System [ ]
CLI Tool [ ]
Certificate Management [ ]
Control Plane [ ]
Demo [ ]
Documentation [ ]
Egress [ ]
Ingress [ ]
Install [ ]
Networking [ ]
Observability [X]
Performance [ ]
SMI Policy [ ]
Security [ ]
Sidecar Injection [ ]
Tests [ ]
Upgrade [ ]
Other [ ]

Please answer the following questions with yes/no.

  1. Does this change contain code from or inspired by another project? No

    • Did you notify the maintainers and provide attribution?
  2. Is this a breaking change? No

  3. Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)? add osm_reconciliation_total metric osm-docs#380

This change adds an `osm_reconciliation_total` metric counting how many
times the reconciler has been triggered for each kind.

Fixes #4568

Example:

    # HELP osm_reconciliation_total Counter of resource reconciliations invoked
    # TYPE osm_reconciliation_total counter
    osm_reconciliation_total{kind="ValidatingWebhookConfiguration"} 9

Signed-off-by: Jon Huhn <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #4788 (304e90f) into main (c24012f) will increase coverage by 0.01%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##             main    #4788      +/-   ##
==========================================
+ Coverage   68.91%   68.92%   +0.01%     
==========================================
  Files         224      224              
  Lines       16388    16402      +14     
==========================================
+ Hits        11293    11305      +12     
- Misses       5043     5045       +2     
  Partials       52       52              
Flag Coverage Δ
unittests 68.92% <85.71%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/osm-bootstrap/osm-bootstrap.go 47.50% <0.00%> (-0.18%) ⬇️
cmd/osm-controller/osm-controller.go 13.87% <0.00%> (-0.06%) ⬇️
pkg/metricsstore/metricsstore.go 92.89% <100.00%> (+0.22%) ⬆️
pkg/reconciler/crd_handler.go 76.00% <100.00%> (+1.00%) ⬆️
pkg/reconciler/mutating_webhook_handler.go 79.06% <100.00%> (+1.02%) ⬆️
pkg/reconciler/validating_webhook_handler.go 79.06% <100.00%> (+1.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c24012f...304e90f. Read the comment docs.

@shalier
Copy link
Contributor

shalier commented Jun 6, 2022

@nojnhuh for my understanding, why would we want to know the number of reconciliations that occurred?

@nojnhuh
Copy link
Contributor Author

nojnhuh commented Jun 6, 2022

@nojnhuh for my understanding, why would we want to know the number of reconciliations that occurred?

Ideally, the reconciler would never be invoked since that would mean the resources are always in the state we expect. Any time the reconciler runs could indicate a bug or other abnormal behavior, so having metrics for those scraped by Prometheus would allow users to track that over time and add alerts.

@nojnhuh nojnhuh merged commit 7de17d7 into openservicemesh:main Jun 6, 2022
@nojnhuh nojnhuh deleted the reconcile-metrics branch June 6, 2022 20:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update control plane metrics
5 participants