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.

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

feat(metrics): Adding metrics for nfs-provisioner #51

Merged
merged 4 commits into from
Jun 24, 2021

Conversation

mynktl
Copy link
Contributor

@mynktl mynktl commented Jun 21, 2021

Signed-off-by: mayank [email protected]

Why is this PR required? What issue does it fix?:

What this PR does?:
This PR exposes metrics about nfs provisioning requests (create/delete). Following metrics are exposed:

persistentvolume_delete_total
persistentvolume_delete_failed_total
persistentvolume_create_total
persistentvolume_create_failed_total

Does this PR require any upgrade changes?: No

If the changes in this PR are manually verified, list down the scenarios covered::

Checklist:

  • Fixes #
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Has the change log section been updated?
  • Commit has unit tests
  • Commit has integration tests
  • (Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
  • (Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them:

@mynktl mynktl requested review from kmova and mittachaitu June 21, 2021 07:30
mayank added 2 commits June 21, 2021 18:32
Signed-off-by: mayank <[email protected]>
Signed-off-by: mayank <[email protected]>
Copy link
Contributor

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/metrics/metrics.go Outdated Show resolved Hide resolved
Co-authored-by: sai chaithanya <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jun 23, 2021

Codecov Report

Merging #51 (1164d7b) into develop (489da0d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop      #51   +/-   ##
========================================
  Coverage    48.46%   48.46%           
========================================
  Files           18       18           
  Lines         1595     1595           
========================================
  Hits           773      773           
  Misses         778      778           
  Partials        44       44           

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 23d6dfa...1164d7b. Read the comment docs.

prometheus.CounterOpts{
Subsystem: NfsVolumeProvisionerSubsystem,
Name: "persistentvolume_delete_total",
Help: "Total number of persistent volumes deleted",
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Total" here refers to the total number of volumes deleted - by this provisioner pod.

I can think of one usecase for these metrics - to check on the health of the provisioner to see if provisioner is able to successfully create volume or if the failures are increasing.

Are there other usecase for these metrics?

Another interesting metric can be to expose the average duration taken for creation or deletion of the volumes. Probably these can be covered in CSI.

Overall good to merge after getting a response to the usecase query.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will help to check stale pv count also from persistentvolume_delete_failed_total

@kmova kmova merged commit 36d13e5 into openebs-archive:develop Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants