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

Implement efficient structured logging #50

Open
1 of 4 tasks
daniel-abramov opened this issue Nov 17, 2022 · 4 comments
Open
1 of 4 tasks

Implement efficient structured logging #50

daniel-abramov opened this issue Nov 17, 2022 · 4 comments
Assignees
Labels
good first issue Good for newcomers T-Enhancement New feature or request

Comments

@daniel-abramov
Copy link
Contributor

daniel-abramov commented Nov 17, 2022

We do use a framework that allows structured logging, however currently we use different formats to log stuff in other places. We must unify them in order to have structured Grafana-compatible logging.

  • Log important key=value pairs when receiving signaling messages or when things happen
  • Use a format that would be readable and compatible with Grafana
  • Replace logrus with a more efficient framework. Unfortunately logrus falls short when it comes to performance 🙁
  • Group common logs together to reduce the amount of lines that we log.
@daniel-abramov daniel-abramov added T-Enhancement New feature or request good first issue Good for newcomers T-Task labels Nov 17, 2022
@daniel-abramov daniel-abramov changed the title Define structured logging Implement structured logging Nov 21, 2022
@daniel-abramov daniel-abramov changed the title Implement structured logging Implement **efficient** structured logging Nov 28, 2022
@daniel-abramov daniel-abramov changed the title Implement **efficient** structured logging Implement efficient structured logging Nov 28, 2022
daniel-abramov added a commit that referenced this issue Nov 28, 2022
It seems like it does not really make logs appear more readable
unfortunately, especially in docker environment. We may want to switch
to a different structured logging instead.

See #50
daniel-abramov added a commit that referenced this issue Dec 5, 2022
It seems like it does not really make logs appear more readable
unfortunately, especially in docker environment. We may want to switch
to a different structured logging instead.

See #50
@daniel-abramov
Copy link
Contributor Author

FWIW: Also, I've noticed that with logrus log timestamps are not always going up. I.e. several go-routines logging via logrus may result in the following log entries in the file:

18:35:00
18:35:01
18:35:02
18:35:01
18:35:03
18:35:02

Which is not very convenient.

@daniel-abramov
Copy link
Contributor Author

After investigating several logging frameworks and while working on #141 it seems like the best option would be to use zerolog (it's faster and a bit more ergonomic than zap), although we would need to implement a hook for the event handling in order to produce OpenTelemetry span events when e.g. info logs are logged. For more info see updated description of #141

@speatzle
Copy link

After investigating several logging frameworks and while working on #141 it seems like the best option would be to use zerolog (it's faster and a bit more ergonomic than zap)

Have you looked at slog?
slog is the new "standard" golang logging package which has been proposed to be added to the standard library.
One of its main goals is to be performant.

I have been using slog in my personal and company project's over the past couple of months and am very pleased with it.

You can find the design doc here.

For now it is available in golang.org/x/exp/slog but since the proposal was accepted by rsc about a day ago it should be in the standard library for the next go release.

@daniel-abramov
Copy link
Contributor Author

daniel-abramov commented Mar 16, 2023

There are two slog packages: one of which (the "official" one) is the package that you linked (I was not aware of that one as I validated another slog package).

The ideas and design goals for the slog look great! Perhaps we could use it instead of zerolog by implementing our own log handler (hopefully it won't be too cumbersome) in order to trigger OpenTelemetry span events when certain things are logged. There is a very young experimental integration with OpenTelemetry though. My only concern with slog is that it's a very young project that is not battle-tested yet and it's part of the experimental package. Not to mention that its primary development seems to be happening outside of GitHub atm (so not particularly easy to submit PRs and link them to our issues should we need to submit any changes).

Using something that is future-proof and could become a part of a standard library is a compelling argument though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers T-Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants