lib/log: internalize logger flushing
Created by: bobheadxi
This PR removes Sync()
from the logger interface and adds a callback returned by log.Init
which should be called before application exit. The removal of Sync()
from the Logger
interface helps indicate that not every logger from log.Scoped
needs a call to Sync()
. For logtest
, we add a t.Cleanup
func to sync the global logger.
From https://github.com/sourcegraph/sourcegraph/pull/34320 I've noticed that log.Fatal
is used quite liberally. Even if we switch to error propagation, there's still the issue that os.Exit
will bypass any defer
calls and hence potentially miss Sync()
. To accommodate this pattern, I've decided to add back Fatal
to Logger
, which internally calls Sync()
before os.Exit(1)
. Also see https://github.com/uber-go/zap/issues/207 for more rationale for supporting Fatal
.
Background: All loggers are children of the root global logger that is instantiated once. As such, it is sufficient to just call Sync()
on the root logger - all child loggers just write to underlying loggers until they reach the root, which as far as I can tell is an os.File
that has Sync()
, which gets called by Zap, so it's not a no-op even if Zap itself does not do any buffering (at the moment, the global logger is not configured to do so)
Aside: in practice, it's honestly probably not a huge deal to never call Sync()
, but we should anyway just in case, so hopefully this balances the probable need to call Sync()
with ergonomics.
Part of https://github.com/sourcegraph/sourcegraph/issues/33192
Test plan
n/a