search

caddyserver / certmagic

Automatic HTTPS for any Go program: fully-managed TLS certificate issuance and renewal
https://pkg.go.dev/github.com/caddyserver/certmagic?tab=doc
Apache License 2.0
4.89k stars 278 forks source link

Feature Request: Use `log/slog` instead of Zap #254

Closed stephenafamo closed 8 months ago

stephenafamo commented 8 months ago

Now that structured logging is part of the standard libarary, are there any plans to switch to log/slog?

francislavoie commented 8 months ago

Since Caddy is the primary consumer of certmagic and the whole Caddy plugin ecosystem uses zap, it would be very difficult to change that at this point.

I could see a Caddy v3 causing us to change over, but I don't think we will take the effort to do that anytime soon.

stephenafamo commented 8 months ago

I guess the ideal situation is to have a way to create a zapcore.Core that pipes output to a slog.Handler and then create a *zap.Logger from that Core that will be passed to certmagic.

francislavoie commented 8 months ago

I guess so. I don't know how interop between them looks like, I haven't looked into it. But that sounds reasonable for now.

mholt commented 8 months ago

Are std libs lobs zero-allocation?

If they aren't, I don't think moving will have any performance benefit.

stephenafamo commented 8 months ago

For the user, it wouldn't be about performance but about interoperability.

For example, if I am starting a new project now, I will use slog for my structured logging. It would be nice that if I need to configure certmagic I can pass in my already existing logger.

I understand going all in on zap before slog was available, or even just in the interest of performance.

Thinking about this more, perhaps there is no need for certmagic and the rest of the Caddy ecosystem to change, there just needs to be an easy way to create a zap logger from slog.

mholt commented 8 months ago

Understood; but we can't impact the TLS server's performance like that -- logs that allocate would significantly reduce the performance of the server given how many logs we emit.

But, yeah, if what you're actually asking for is interop, then maybe a feature request for zap to integrate with slog in some way would be the best way to go. It wouldn't necessarily be as fast of course, but then CertMagic wouldn't have to change and you could still use your slog with CertMagic.

I'll close this but we can reopen if something becomes actionable for us :+1:

francislavoie commented 8 months ago

Did some quick googling...

slog recommends using the Logger.LogAttrs API for zero-allocation logging (instead of Logger.Log).

Apparently there's interop between slog can use zap as a logging backend if a Handler is provided. https://github.com/chanchal1987/zaphandler is an example.

francislavoie commented 8 months ago

Actually, looks like zap added some compat in https://github.com/uber-go/zap/pull/1246