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

Initialize empty schedule metrics on server init #1054

Merged
merged 1 commit into from
Nov 20, 2018

Conversation

cbeneke
Copy link
Contributor

@cbeneke cbeneke commented Nov 14, 2018

When backups are run manually (outside of a schedule) the metrics will be counted for ark_*{schedule=""}. To prevent partial NaN metrics they will be initialised on server init.

The current behaviour creates e.g. only ark_backup_failure_total or ark_backup_success_total but never both on the first manual run. Afaict the backupName shall be included in the metrics in the future, when this is implemented the initialisation must be moved to the point in time, where the backupName is already known (I guess the processBackup() function in pkg/controller/backup_controller.go), but so far the server init should be most sufficient for the metrics init.

When backups are run manually (outside of a schedule) the metrics
will be counted for ark_*{schedule=""}. To prevent partial NaN
metrics they will be initialised on server init.

Signed-off-by: Christian Beneke <[email protected]>
@nrb
Copy link
Contributor

nrb commented Nov 14, 2018

Thanks for this @cbeneke! I'll take a closer look this afternoon, but my initial thought is that this makes sense.

@nrb nrb requested a review from skriss November 14, 2018 22:40
Copy link
Contributor

@nrb nrb left a comment

Choose a reason for hiding this comment

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

This makes sense to me - it makes sure that manual, non-scheduled backups are treated the same as scheduled in terms of metrics.

@nrb nrb added this to the v0.10.1 milestone Nov 14, 2018
Copy link
Contributor

@skriss skriss left a comment

Choose a reason for hiding this comment

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

LGTM

@skriss skriss merged commit d27e4f7 into vmware-tanzu:master Nov 20, 2018
@cbeneke cbeneke deleted the master branch November 20, 2018 17:48
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.

3 participants