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

refactor: consistent log message for new events #1534

Merged
merged 4 commits into from
May 28, 2020

Conversation

unnawut
Copy link
Contributor

@unnawut unnawut commented May 26, 2020

Overview

This fixes and makes consistent the event logging e.g.

1. inspect(Elixir.OMG.Status.Metric.StatsdMonitor) got event: {:clear_alarm, {:main_supervisor_halted, ...

2. Monitor got event: {:clear_alarm, {:main_supervisor_halted, ...

3. Elixir.OMG.Eth.EthereumHeightMonitor.AlarmHandler got event: {:clear_alarm, {:main_supervisor_halted, ...

by converting all into the last format with the full module name

Changes

  • Convert all event ignores to log with full module names

Testing

Check the logs. Some monitors will ignore some alarms so the log should appear within a few minutes after boot.

@unnawut unnawut self-assigned this May 26, 2020
@coveralls
Copy link

coveralls commented May 26, 2020

Coverage Status

Coverage remained the same at 78.231% when pulling 3c965ca on unnawut/event-log-message into e5735d9 on master.

Copy link
Contributor

@achiurizo achiurizo left a comment

Choose a reason for hiding this comment

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

Looks good, for a future PR, might be worth while to centralize our logging messages in some module?

@unnawut
Copy link
Contributor Author

unnawut commented May 27, 2020

Looks good, for a future PR, might be worth while to centralize our logging messages in some module?

True, the module name is easily helpful to be a json log attribute instead

@unnawut unnawut requested review from boolafish and jarindr and removed request for T-Dnzt and mederic-p May 28, 2020 10:53
@unnawut
Copy link
Contributor Author

unnawut commented May 28, 2020

Coming back reading again, this looks like a silly PR. The original issue is the unparsed inspect(Elixir.OMG.Status.Metric.StatsdMonitor) in the log that's been bothering me when log-skimming.

@unnawut unnawut merged commit 07774c8 into master May 28, 2020
@unnawut unnawut deleted the unnawut/event-log-message branch May 28, 2020 19:25
@unnawut unnawut added the enhancement New feature or request label Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants