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

Improve failover coordinator error logging #4811

Merged
merged 1 commit into from
May 11, 2022

Conversation

mantas-sidlauskas
Copy link
Contributor

What changed?
Adding err tags to log messages

Why?
This will help diagnose log messages as actual error now will be visible

How did you test it?

Potential risks

Release notes

Documentation Changes

@coveralls
Copy link

coveralls commented May 4, 2022

Pull Request Test Coverage Report for Build 0ee5cd5b-3729-4938-b1ff-f4cae49db778

  • 10 of 17 (58.82%) changed or added relevant lines in 3 files are covered.
  • 97 unchanged lines in 15 files lost coverage.
  • Overall coverage decreased (-0.06%) to 56.933%

Changes Missing Coverage Covered Lines Changed/Added Lines %
common/cache/domainCache.go 6 7 85.71%
common/domain/failover_watcher.go 1 3 33.33%
service/history/failover/coordinator.go 3 7 42.86%
Files with Coverage Reduction New Missed Lines %
common/task/weightedRoundRobinTaskScheduler.go 1 88.6%
service/history/queue/timer_queue_processor_base.go 1 77.92%
client/history/client.go 2 38.58%
client/history/metricClient.go 2 45.3%
common/util.go 2 51.17%
common/types/history.go 4 24.89%
service/frontend/workflowHandler.go 4 60.31%
service/history/handler.go 4 47.6%
service/history/task/fetcher.go 4 86.15%
service/history/task/redispatcher.go 4 89.67%
Totals Coverage Status
Change from base Build 99be5982-7a3d-4a2c-a39a-1bc68f1a2f10: -0.06%
Covered Lines: 83955
Relevant Lines: 147462

💛 - Coveralls

@@ -183,13 +184,12 @@ func CleanPendingActiveState(
// this call has to be made
metadata, err := domainManager.GetMetadata(context.Background())
if err != nil {
return err
return fmt.Errorf("getting metadata: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a great picture in my head for what might be listening to this error. Is there anything that might be doing an exact match on this which needs to be changed to use errors.As ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. but no, this error is just being used for logging purposes in coordinator.go:309 and failower_watcher.go:160

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good then

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