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

How to set the default logger programatically? #5938

Closed
maxott opened this issue Nov 13, 2023 · 6 comments · Fixed by #5944
Closed

How to set the default logger programatically? #5938

maxott opened this issue Nov 13, 2023 · 6 comments · Fixed by #5944
Labels
needs info 📭 Requires more information

Comments

@maxott
Copy link

maxott commented Nov 13, 2023

I'm using Caddy as an embedded component in a service and would like to be able to set at least the default logger to the
zap logger the service has already configured by the time Caddy is started.

The primary reason for that is that setting up the logger in Caddy's config file is missing certain capabilities (primarily related to the encoder). But from management point of view, I want to avoid having to spread/duplicate logger configuration across multiple places (my service & Caddy config).

I guess the easiest place is to "extend" the Log function in logging.go. Being able to set the defaultLogger variable is most likely not going to work as it seems to be "recreated" in setupNewDefault based on its CustomLog settings - which lack what I need.

Or is there already a way to achieve that without any further code changes?

@francislavoie
Copy link
Member

The primary reason for that is that setting up the logger in Caddy's config file is missing certain capabilities

What do you mean, exactly? Couldn't we fix that instead? What are your needs?

This is sounding kinda like an XY Problem.

@mholt
Copy link
Member

mholt commented Nov 13, 2023

I'd be interested to know what about the encoder we could fix or improve to meet your needs. What are your requirements?

@francislavoie francislavoie added the needs info 📭 Requires more information label Nov 13, 2023
@maxott
Copy link
Author

maxott commented Nov 15, 2023

As for the XY Problem, I thought I already stated the problem - I'm using Caddy as an embedded component in a service and already have a configured zap logger by the time I initialise Caddy. Otherwise, I would need to ensure that the logger configuration from the service configuration is kept in sync with the caddy config file. Therefore, I was looking for a solution on how to also use that logger for Caddy.

As to what I'm missing:

  • Setting EncodeTime:TimeEncoder in EncoderConfig
    • not sure on how I could define that in JSON as TimeEncoder is a function
    • our 'primary' service is using zapcore.ISO8601TimeEncoder
  • Adding zap.WithCaller to zap.Builder

I would really like to find a solution to this as I'm now having two different logging formats interspersed in the logs which complicates processing. Not a real show stopper, just an eye sore :)

Otherwise, I'm a very happy user!

@francislavoie
Copy link
Member

francislavoie commented Nov 15, 2023

You can set TimeFormat in the config (aka time_format in JSON), which sets up TimeEncoder. See

// time format
var timeFormatter zapcore.TimeEncoder
switch lec.TimeFormat {
case "", "unix_seconds_float":
timeFormatter = zapcore.EpochTimeEncoder
case "unix_milli_float":
timeFormatter = zapcore.EpochMillisTimeEncoder
case "unix_nano":
timeFormatter = zapcore.EpochNanosTimeEncoder
case "iso8601":
timeFormatter = zapcore.ISO8601TimeEncoder
default:
timeFormat := lec.TimeFormat
switch lec.TimeFormat {
case "rfc3339":
timeFormat = time.RFC3339
case "rfc3339_nano":
timeFormat = time.RFC3339Nano
case "wall":
timeFormat = "2006/01/02 15:04:05"
case "wall_milli":
timeFormat = "2006/01/02 15:04:05.000"
case "wall_nano":
timeFormat = "2006/01/02 15:04:05.000000000"
case "common_log":
timeFormat = "02/Jan/2006:15:04:05 -0700"
}
timeFormatter = func(ts time.Time, encoder zapcore.PrimitiveArrayEncoder) {
var time time.Time
if lec.TimeLocal {
time = ts.Local()
} else {
time = ts.UTC()
}
encoder.AppendString(time.Format(timeFormat))
}
}
cfg.EncodeTime = timeFormatter

We can add an option to enable WithCaller. 99% of the time it's not useful for users because the line numbers won't mean anything to them, which is why we didn't include it. But yeah, no harm in adding it as an option.

@francislavoie
Copy link
Member

See #5944, I think this'll do what you need?

@maxott
Copy link
Author

maxott commented Nov 16, 2023

Thanks, that will allow me to solve my problem by duplicating logging information.

I understand that the majority of folks are using Caddy as a "closed" application, configured by Caddy's own config file. And maybe, I'm the only one using it differently, but it can be used differently and having extension points, like providing one own's logger would make it even more useful.

In any case, thanks for the help.

@maxott maxott closed this as completed Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs info 📭 Requires more information
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants