Configure a custom zap logger

[moul]

Hi,

We (@berty) are using zap extensively and improving our logging experience; we initialize our zap logger in a custom way:

• we like to use namespaces to manage subsystems (https://github.com/uber-go/zap/blob/v1.16.0/logger.go#L135)
• we have env-based and CLI-flags parameters to change the destination of the logs (stdout, stderr or a logfile)
• we have a custom driver for mobile to bridge the main zap logger with native loggers on iOS and Android (https://github.com/berty/berty/blob/bbe0fd3462911d98e7685032b828429e75ca8af6/go/framework/bertybridge/driver_logger.go, https://github.com/berty/berty/blob/1adfee0bfedec095c5efb7f1fc6ee284335cab95/js/packages/go-bridge/ios/GoBridge.swift)
• and I'm currently (feat: refactor logger berty/berty#2270) integrating a new filtering system based on namespaces -> https://github.com/moul/zapfilter

Our current integration with ipfs/go-log is bad: https://github.com/berty/berty/blob/295bef9a6cced69feebf7b5e16525a112e74cdf8/go/cmd/berty/opts.go#L235-L254

---

I can open a PR against your repo to add new helpers, but I need to know if you accept changing the current API of your package and what kind of change you prefer

What about adding a new SetZapLogger(*zap.Logger) function, that I would use like this:

ipfs_log.SetZapLogger(myLogger.Named("ipfs"))

Another solution is to accept custom Options so we can pass a WrapCore(customCore) that go-log can chain in the New or within a WithOption.

What do you think? any better idea?

[welcome]

Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review.
In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment.
Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

• "Priority" labels will show how urgent this is for the team.
• "Status" labels will show if this is ready to be worked on, blocked, or in progress.
• "Need" labels will indicate if additional input or analysis is required.

Finally, remember to use https://discuss.ipfs.io if you just need general support.

[jacobheun]

@lanzafame do you have any thoughts on this request/suggestion?

[BigLep]

@aschmahmann : are able to provide input on this?

[lanzafame]

@moul sorry, for the slow reply, I have been on paternity leave.

I am open to the addition of the SetZapLogger function.

[MDobak]

I wanted to do a PR for this issue, but from what I can see, it looks a bit more complicated. If I understand correctly, the proposed SetZapLogger method would only replace already created loggers from the loggers map. The problem is, that any logger created later by the Logger function would not use the new logger.

What do you think about adding a SetPrimaryCore function instead that could look like this:

func SetPrimaryCore(core zapcore.Core) {
	loggerMutex.Lock()
	defer loggerMutex.Unlock()

	if primaryCore != nil {
		loggerCore.ReplaceCore(primaryCore, core)
	} else {
		loggerCore.AddCore(core)
	}
	primaryCore = core
}

?

[lanzafame]

@MDobak The fix to this needs to both replace the existing and make sure that any new loggers use the provided zapcore.Core. Otherwise loggers that are created in dependencies will operate differently to loggers created in the application. The PR you created looks good except the missing functionality of updating existing loggers.

[MDobak]

@lanzafame The SetPrimaryCore function sets/replaces the core in the lockedMultiCore structure that is used by all loggers, effectively updating all existing loggers and any that will be created later. I've updated the test in my PR to cover this case.