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

Logging revamp in general #221

Open
KristianLyng opened this issue Mar 22, 2022 · 0 comments
Open

Logging revamp in general #221

KristianLyng opened this issue Mar 22, 2022 · 0 comments
Labels
core/config Anything dealing with the configuration subsystem core/docs Core documentation, either actual documentation of technical tooling for documentation core/modules Anything dealing with the general module support enhancement New feature or request question Any question related to Skogul, how it works, "is this the intended behaviour" etc.

Comments

@KristianLyng
Copy link
Collaborator

This is a "dev-note"/discussion issue. Please add ANY AND ALL feedback even remotely related to logging here, unless there is already an obvious issue that covers it (e.g. #70). I am trying to get a good overall logging approach.

We introduced logrus to standardize logging, but I'm honestly not very impressed. So far, the main benefit is implementing log levels. For actual logs, the output tends to be a bit convulted for my taste, and it's difficult to write clean code that gracefully handles nested error situations.

In Logrus' defence, since it's creation, wrapped error messages have become a real thing that actually works.

In addition to this, the skogul.Error{} data type doesn't really do us any favors. The idea might've been sensible, but it never materialized. As such, I am gradually removing it as I'm working with the code.

I am not entirely giving up on logrus, but this issue is left as a little "dev note". I am currently focusing on cleaning up our actual real life logs and making them useful for debugging. To a large extent, this means reducing reliance on logrus "fields", and instead using fairly plain text log messages, wrapped.

A few notes, also found in docs/:

  1. We should NEVER both log an error, and return that same error. It creates confusing log spam.
  2. If we feel a need for trace logging in individual modules, which has been a thing in the past, we need to determine why a wrapped error doesn't do the trick first, and actually comment on it.
  3. For things that "eat" errors, that thing should be configurable to (optionally) log.
  4. Anything dealing with metrics or containers should use the new Describe() function - that function should be improved if the output isn't good enough.
  5. All modules should identify themself! skogul.Identity[] works great and dramatically improves debugging.
  6. Need to figure out how to deal with timestamps when they go in the syslog/journal, since right now we get duplicated timestamps from both systemd/journald and skogul/logrus.
  7. I intend to add a global configuration section to supplement some of the command line options, and log-related options will go there to make it easier to switch log level etc.
@KristianLyng KristianLyng added enhancement New feature or request core/config Anything dealing with the configuration subsystem core/modules Anything dealing with the general module support core/docs Core documentation, either actual documentation of technical tooling for documentation question Any question related to Skogul, how it works, "is this the intended behaviour" etc. labels Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core/config Anything dealing with the configuration subsystem core/docs Core documentation, either actual documentation of technical tooling for documentation core/modules Anything dealing with the general module support enhancement New feature or request question Any question related to Skogul, how it works, "is this the intended behaviour" etc.
Projects
None yet
Development

No branches or pull requests

1 participant