Skip to content

Tweaks to the default format and format stability #74

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

Closed
KodrAus opened this issue Mar 9, 2018 · 4 comments
Closed

Tweaks to the default format and format stability #74

KodrAus opened this issue Mar 9, 2018 · 4 comments

Comments

@KodrAus
Copy link
Collaborator

KodrAus commented Mar 9, 2018

Our current env_logger format is pretty good, but I think there are a few tweaks we can make to it that would improve readability, parsability and compactness. Before that though, I think it would be good if we thought about whether or not we consider the default format as part of our semver'd API.

Changing the default format in patches

I think we should be mindful, but not against changing the default format in patches. We do eventually want it to be fairly stable, but if a user needs a truly stable format then they should use a custom format.

I'm keen to hear what other people think about this!

Improvements to our default format

I think we can make some tweaks to our current format that should be an improvement for the majority of users:

  • Wrap parts of the format that aren't the actual message in brackets so they're easier to distinguish from the message. This also means all logs (unless you disable all optional parts of the format) will start on a new line with the same character.
  • Shorten the log level so they're all the same length. So:
    • Trace => TRC
    • Debug => DBG
    • Info => INF
    • Warn => WRN
    • Error => ERR

To see how this might look, here's a few examples using our current format, and the changes I was thinking about:

Proposed format:
proposed-all

Current format:
current-all

Proposed format:
proposed-multi-line

Current format:
current-multi-line

This is also a bit subjective, so I'd like to know what you all think about the current and proposed format too!

@KodrAus
Copy link
Collaborator Author

KodrAus commented Mar 13, 2018

As suggested on reddit we could put the timestamp first in the format so that single-line log messages can be more naturally sorted by time.

@KodrAus
Copy link
Collaborator Author

KodrAus commented Mar 18, 2018

Based on some discussion on reddit, I think env_logger's default format will optimise for ad-hoc analysis, like physically eyeballing the output or once-off text processing tasks. The default format won't optimise for long-term stability, and explicitly makes no guarantees about the stability of its output across major, minor or patch version bumps during 0.x.

If you want to rely on a specific format for log records then you need to state what that format is. Otherwise things will change, and that's the case whether you explicitly opt-in to a new version of env_logger or not. This sort of implicit coupling is problematic for systems in the long-term because they're difficult to reason about. Changes can be introduced in ways that are non-obvious. So env_logger's docs will call out that the default format is not truly stable and offer examples of how folks can go about making a format dependable if they need it. By 1.x, we should offer an easy way to opt-in to some arbitrary stable format.

The alternative option would be to completely freeze the default format, since log records may be captured and persisted. If the format changes then the complete set of persisted log files might use multiple formats. This option is not ideal because it makes improvements to either log or env_logger much harder to discover.

Over time the default format will settle and probably not need to change much after 1.x anyway. Even if that's the case we should explicitly not encourage depending on the shape of the default format.

We should also revisit this before 1.x. By then we might want to limit format changes to at least minor version bumps.

@JustAPerson
Copy link

I like moving everything into brackets but dislike the 3 character abbreviations. I think padding info/warn is best. Saving 1-2 characters doesn't help output size much but seems to impede readability a fair bit. ERR/DBG are intuitive but INF/WRN are not.

Time first also seems preferable for sorting.

As an aside, I originally came to complain because the docs currently imply the output is not well aligned for different log levels, which would be highly preferable.

@KodrAus
Copy link
Collaborator Author

KodrAus commented Apr 16, 2018

I find the 3 letter abbreviations fairly intuitive, I think they're fairly common, but don't have super strong opinions about them. Here's how the format looks with the timestamp at the front and the log levels expanded:

new-env-logger-fmt

I've got a PR that implements this, but I think I'll leave it open for a while so there's plenty of time for feedback.

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

No branches or pull requests

2 participants