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

refactor: add structured logging with slog #199

Merged
merged 5 commits into from
Aug 19, 2024
Merged

refactor: add structured logging with slog #199

merged 5 commits into from
Aug 19, 2024

Conversation

sbruens
Copy link

@sbruens sbruens commented Aug 15, 2024

This PR keeps the status quo of logging to the default logger. This doesn't change the fact that we log in the service library. Ideally the library part doesn't log, that should be up the client, but leaving it as a TODO to rethink that in a future PR.

Tint seems to be the go-to for tinted/colored logs if we want to keep that feature, but I could revert to just plain text logging.

Screenshot:

Screenshot

@sbruens sbruens changed the title refactor: add structure logging with slog refactor: add structured logging with slog Aug 16, 2024
@sbruens sbruens marked this pull request as ready for review August 16, 2024 15:59
@sbruens sbruens requested a review from a team as a code owner August 16, 2024 15:59
@sbruens sbruens requested a review from fortuna August 16, 2024 16:00
Copy link

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

Looks great!

service/tcp.go Outdated
// between Go's inlining/escape analysis and varargs functions like slog.Debug.
if slog.Default().Enabled(context.TODO(), slog.LevelDebug) {
args = append(args, slog.String("ID", cipherID))
slog.Debug(fmt.Sprintf("TCP: %s", template), args...)
Copy link

Choose a reason for hiding this comment

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

FYI. I guess it doesn't matter for debug, but LogAttrs is the most efficient way to log: https://pkg.go.dev/golang.org/x/exp/slog#hdr-Attrs_and_Values

Copy link
Author

Choose a reason for hiding this comment

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

Good point. Done, even if it's just for debug.

@sbruens sbruens merged commit 0f6ad5b into master Aug 19, 2024
5 checks passed
@sbruens sbruens deleted the sbruens/slog branch August 19, 2024 20:30
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