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

chore(log): Tweaking logging again, making everything consistent #101

Merged
merged 3 commits into from
Nov 6, 2023

Conversation

elliotcourant
Copy link
Collaborator

This is another logging change to make everything consistent. Log
key-value pairs are now done via slog's Attr format based on the notes
at the tail end here: https://go.dev/blog/slog

While this might be a more opinionated change, I see one major benefit.
If the user of the library is wrapping the logging with their own logger
that they are using (zap, logrus or any else) they would have to parse
both the alternating key value pairs and the Attr structs. By only using
the Attr structs the user would only need to handle that one object type
now in their own logging layer; hopefully making integration easier for
them to have consistent logs.


I'm opening this specifically because I'm using logrus for my application and I found that it was unnecessarily
complicated to try to convert both the Attr and the alternating key-value pairs to the logrus fields nicely?
Ultimately I'm pushing the problem upstream to this library lol, but hopefully in such a way that it would make it
easier for anyone to also implement their own logging consistently using this library.

Please let me know what your thoughts are, this is a somewhat opinionated PR in terms of logging changes. While the
output will be identical for the default logger the code has been changed throughout.

This is another logging change to make everything consistent. Log
key-value pairs are now done via slog's Attr format based on the notes
at the tail end here: https://go.dev/blog/slog

While this might be a more opinionated change, I see one major benefit.
If the user of the library is wrapping the logging with their own logger
that they are using (zap, logrus or any else) they would have to parse
both the alternating key value pairs and the Attr structs. By only using
the Attr structs the user would only need to handle that one object type
now in their own logging layer; hopefully making integration easier for
them to have consistent logs.
@elliotcourant
Copy link
Collaborator Author

I've broken tests, moving this to a draft while I sort that out

@elliotcourant elliotcourant marked this pull request as ready for review November 6, 2023 02:40
Copy link
Owner

@acaloiaro acaloiaro left a comment

Choose a reason for hiding this comment

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

@elliotcourant elliotcourant merged commit e6ece3c into main Nov 6, 2023
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants