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

Use journald loglevel on Linux systems #7353

Closed
potuz opened this issue Sep 26, 2020 · 9 comments · Fixed by #7429 or #7463
Closed

Use journald loglevel on Linux systems #7353

potuz opened this issue Sep 26, 2020 · 9 comments · Fixed by #7429 or #7463
Labels
Enhancement New feature or request Good First Issue Good for newcomers Help Wanted Extra attention is needed

Comments

@potuz
Copy link
Contributor

potuz commented Sep 26, 2020

Currently if running on the background on Linux using systemd, all events are logged at the default level (typically "info").

By prefixing the lines of output with angle brackets with the loglevel, for example

<4> This is a warning message
<6> This is an info message

Then a command like journalctl -u beacon-chain -p 4 will only print the warning message.

Arguably this is uncomfortable for those not using journald and/or piping the logs somewhere else. In this case a simple workaround is not to print if running in the foreground. For a more involved solution journald exposes the syslog API so that this can be handled there. Given that the vast majority of Linux systems already switched to systemd I think that simply prefixing the lines would satisfy most people while the trade for those running custom logs would be minimal.

@nisdas nisdas added the Enhancement New feature or request label Sep 26, 2020
@prestonvanloon
Copy link
Member

We could add this as another logging format. We already support JSON and fluentd, so another format would be acceptable if anyone finds it useful.

@prestonvanloon prestonvanloon added Good First Issue Good for newcomers Help Wanted Extra attention is needed labels Sep 26, 2020
@potuz
Copy link
Contributor Author

potuz commented Sep 26, 2020

Journald accepts JSON (as long as it's key=value pairs) so it may be simple to port already the JSON format

https://www.freedesktop.org/wiki/Software/systemd/json/

https://stackoverflow.com/questions/45604563/injecting-structured-json-logs-into-journald

@prestonvanloon
Copy link
Member

Try --log-format=json or --log-format=fluentd, fluentd is also JSON but a more standard format. JSON should be a single object with key value pairs for each entry, fluentd might have nested objects.

@potuz
Copy link
Contributor Author

potuz commented Sep 26, 2020

That just prints the json in the journal, to feed the json I think it has to be through the local socket or connecting with HTTP on port 19531 for example.

@potuz
Copy link
Contributor Author

potuz commented Oct 3, 2020

I know nothing about go, installed it today and started browsing your code, found that you're using logrus and found a very simple hook for journald

sirupsen/logrus@4d9b4f0
https://github.com/wercker/journalhook

Seems very simple to implement, if noone takes this in the next few days I may add it

@potuz
Copy link
Contributor Author

potuz commented Oct 4, 2020

I added a PR, but then I read that you guys do not accept PRs with new dependencies. Instructions were to open a new Issue, but perhaps this one is enough?

@prestonvanloon
Copy link
Member

@potuz Unfortunately, PR #7429 adds a dependency on core-os which does not work when building on windows.
We are reverting that PR and re-opening this issue.
Please see #7454

@prestonvanloon prestonvanloon reopened this Oct 7, 2020
@potuz
Copy link
Contributor Author

potuz commented Oct 7, 2020

I'll see if I can add a flag for linux, I'm surprised this got past the CI

@prestonvanloon
Copy link
Member

prestonvanloon commented Oct 7, 2020

I'm surprised this got past the CI

Yeah, looks like we don't test windows builds in the CI. I'll take a look into that as well.

potuz added a commit to potuz/prysm that referenced this issue Oct 8, 2020
prylabs-bulldozer bot pushed a commit that referenced this issue Oct 14, 2020
* Add journald option for logger

Fixes #7353

* fix docker images

* go mod tidy

Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Good First Issue Good for newcomers Help Wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants