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

Add severity logging levels, and send errors to syslog/Event Viewer (incl failed rollbacks for confirmed blocks) #1973

Merged
merged 13 commits into from
May 18, 2019

Conversation

wezrule
Copy link
Contributor

@wezrule wezrule commented May 10, 2019

When the rollback visitor tries to rollback a confirmed block, it currently just outputs to the log file. For greater visibility it now also gets sent to the system logger as priority error which should flag up any process monitoring applications.

Added severity_level to the logging, currently just with normal and error, where normal is the default unless otherwise specified. The sys log is filtered on equal or greater than the severity_logging::error, so that only crucial messages are sent to the sys log on Unix & Event log on Windows.

The Unix system logger output /var/log/syslog doesn't show priority by default. When testing I followed this page to check that the priority was set correctly. Using the new --debug_sys_logging CLI command the follow is output:

3,1,May 10 13:27:01,ubuntu-s-1vcpu-1gb-lon1-01,nano_node:, Error: 3: Testing system logger

Where 3 is the error priority based on RFC5424.

@wezrule wezrule added this to the V19.0 milestone May 10, 2019
@wezrule wezrule requested a review from cryptocode May 10, 2019 14:11
@wezrule wezrule self-assigned this May 10, 2019
Copy link
Contributor

@cryptocode cryptocode left a comment

Choose a reason for hiding this comment

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

LGTM pending formatting on CI.

On Windows the event log sink could be used, but that can be a separate PR if we want it.

@wezrule
Copy link
Contributor Author

wezrule commented May 10, 2019

So the event viewer was a bit more of a pain. From the boost docs

The library must be built as a dynamic library in order to use this backend flawlessly. Otherwise event description resources are not linked into the executable, and the Event Viewer is not able to display events properly.

Which we don't do, it is a static library. There are also some permissions problems as it needs to create a registry entry which fails unless the node is run as an admin, which we don't enforce. However checking the Event viewer on Windows 10 it still has the correct entry marked as an error. For instance, using using the new cli command (--debug_sys_logging) the description field contains:

The description for Event ID 259 from source Nano cannot be found. Either the component that raises this event is not installed on your local computer or the installation is corrupted. You can install or repair the component on the local computer.

If the event originated on another computer, the display information had to be saved with the event.

The following information was included with the event: 

Error: 58 Testing system logger

Which I think is good enough, it still contains the relevant log message.

@wezrule wezrule requested a review from cryptocode May 10, 2019 17:52
@wezrule wezrule changed the title Add severity logging levels, and send errors to sys logger (incl failed rollbacks for confirmed blocks) Add severity logging levels, and send errors to syslog/Event Viewer (incl failed rollbacks for confirmed blocks) May 10, 2019
@cryptocode cryptocode requested a review from SergiySW May 11, 2019 10:08
@SergiySW
Copy link
Contributor

Could it print to log file "Error: " as well?

@wezrule wezrule merged commit 6247260 into nanocurrency:master May 18, 2019
@wezrule wezrule deleted the confirmation_height_sys_logging branch May 18, 2019 07:49
@zhyatt zhyatt added documentation This item indicates the need for or supplies updated or expanded documentation and removed beta testing wanted documentation This item indicates the need for or supplies updated or expanded documentation labels Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants