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

feat: Introduce logs in json format using log/slog #102

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

hiddenmarten
Copy link
Contributor

Summary

#99

Checklist

  • Tested and/or added tests to validate that the changes work as intended, if applicable.
  • Updated documentation in README.md, if applicable.

@hiddenmarten hiddenmarten changed the title Introduce logs in json format using logrus Introduce logs in json format using log/slog Dec 10, 2024
@hiddenmarten
Copy link
Contributor Author

Reimplemented from scratch to achieve a closer resemblance to the initial implementation and to avoid adding external dependencies.

@TwiN
Copy link
Owner

TwiN commented Dec 11, 2024

This is going to sound a little weird, but I'm actually in the process of migrating my Go projects to use my own logging library. I haven't had the opportunity to add support for JSON logs yet, but I'd be more than receptive to the idea of letting somebody else implement it if you're willing to take on the challenge.

That being said, since you re-implemented it in a way that makes the changes much smaller (as opposed to the logrus approach you initially took), I'm okay with merging the current implementation.

What do you think? Do you want the current implementation, or would you be willing to try implementing JSON logs on logr? Either way, I'm very thankful for the effort you're putting into this, so please don't hesitate if you'd rather just go for the current implementation - no hard feelings!

@hiddenmarten
Copy link
Contributor Author

hiddenmarten commented Dec 11, 2024

I'd suggest merging it as is, with a pinky promise from me to rework this using your library later. I should have time over the weekend. :)

Thank you so much for the super fast review - I really appreciate it!

@TwiN
Copy link
Owner

TwiN commented Dec 12, 2024

@hiddenmarten Pleasure's mine - and there's no need to impose such a short deadline on yourself. With Christmas and New Year just around the corner, feel free to look into it after the Holidays if you have other matters to take care of :)

@TwiN TwiN changed the title Introduce logs in json format using log/slog feat: Introduce logs in json format using log/slog Dec 12, 2024
@TwiN TwiN merged commit e843d11 into TwiN:master Dec 12, 2024
1 check 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