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

adding push metrics as a defer in the upload method to make sure metrics are pushed even on failure #1423

Merged
merged 1 commit into from Mar 3, 2023

Conversation

francoispqt
Copy link
Contributor

Database name

All of them

Pull request description

The metrics feature works fine to record uploads when they are successful but metrics are never pushed when an upload fails because Fatal logging exits directly and prevents the PostRun cobra hook from running.

To solve that I propose to call PushMetrics directly from the uploader.Upload method.

Describe what this PR fix

It fixes #1422

@francoispqt francoispqt requested a review from a team as a code owner January 26, 2023 17:26
@usernamedt
Copy link
Member

usernamedt commented Mar 3, 2023

Somehow I missed this PR. Ok, let's merge it as a temporary solution. But we need to solve the root cause eventually - handle fatal errors gracefully.

Thanks for your contribution!

@usernamedt usernamedt merged commit d52bde1 into wal-g:master Mar 3, 2023
@francoispqt
Copy link
Contributor Author

Yes I agree, my take would be to stop using log.Fatal and bubble up errors but that's way more work

usernamedt added a commit that referenced this pull request Mar 9, 2023
#1423 causes unit tests to fail so I propose to revert it.
@usernamedt usernamedt mentioned this pull request Mar 9, 2023
usernamedt added a commit that referenced this pull request Mar 9, 2023
#1423 causes unit tests to fail.
@jurim76
Copy link

jurim76 commented May 4, 2023

Any plans to fix it?

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.

Metrics are not pushed on upload failures because of Fatal logging
3 participants