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

add hclogslog from github.com/evanphx/go-hclog-slog #144

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

nickethier
Copy link
Member

After a short discussion with @evanphx we decided to pull in the code from github.com/evanphx/go-hclog-slog as a separate go module under this repo. Since slog is becoming the community standard structured logger, it makes most sense to maintain an adapter for hclog.

Code author attribution is maintained and licenses are the same.

🚨 Once this PR is merged I will cut a hclogslog/v0.1.0 tag to release the module.

@nickethier nickethier requested a review from evanphx August 19, 2024 17:39
@nickethier
Copy link
Member Author

Builds fixed in #145

Copy link

@jooola jooola left a comment

Choose a reason for hiding this comment

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

I'd love to see this merged! Trying to push this forward with a small review.

Comment on lines +11 to +13
func Adapt(l hclog.Logger) slog.Handler {
return &Handler{l: l}
}
Copy link

Choose a reason for hiding this comment

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

Should this have a small documentation as entry point for the rest of the adapter?

// WithAttrs returns a new Handler whose attributes consist of
// both the receiver's attributes and the arguments.
// The Handler owns the slice: it may retain, modify or discard it.

Copy link

Choose a reason for hiding this comment

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

One line too much?

Comment on lines +192 to +201
prefix := h.prefix

if name != "" {
prefix = h.prefix + name + "."
}

return &Handler{
l: h.l,
prefix: prefix,
}
Copy link

Choose a reason for hiding this comment

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

We do not exactly return the receiver here, it's a pointer to a new handler with the same settings.

Maybe this should be:

if name == "" {
    return h
}
// ...

apricote pushed a commit to hetznercloud/fleeting-plugin-hetzner that referenced this pull request Nov 12, 2024
…g-plugin-hetzner!166)

We need a logger to be able to log warnings without failing hard.

The logger will be replaced by a slog logger once hclog has an adapter for slog: hashicorp/go-hclog#144
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.

4 participants