Move to zerolog

[kegsay]

We should move to use https://github.com/uber-go/zap or https://github.com/rs/zerolog instead of logrus because logrus does a loooooot of allocations, far more than the rest of Dendrite. I'd propose we use the sugared one since we're not looking for any specific improvement other than "why is logging consuming so much time/allocs". In terms of numbers:

• logrus 29501 ns/op
• zap 1250 ns/op (~24x speedup)

Allocations:

• logrus 125 allocs/op
• zap 11 allocs/op

[spaetz]

Sounds great, Just keep in mind:

Note that zap only supports the two most recent minor versions of Go.

So that is quite a change to your current comparability policy.

[kegsay]

I suspect when Go 1.18 comes out we will bump the min Go version to that so we can make liberal use of generics.

[S7evinK]

While a tedious task, this is probably a good way to get to know Dendrites code base. We decided to go with https://github.com/rs/zerolog as the replacement for logrus.

[genofire]

A json output format would be nice, to use parsing software on logs. Then i would like to add an Flow in the HelmChart for the kube-logging operator: https://kube-logging.github.io/docs/configuration/flow/

[9241304]

Hi guys. Is somebody working on this issue?

[S7evinK]

At least speaking for the core team - no, we're not working on this at the moment.

[mjholub]

While a tedious task, this is probably a good way to get to know Dendrites code base. We decided to go with https://github.com/rs/zerolog as the replacement for logrus.

tbh it's not that complex when compared to e.g. zap. A basic setup of a zerolog logger virtually on par with logrus is just that:

func InitLogger() zerolog.Logger {
        zerolog.TimeFieldFormat = zerolog.TimeFormatUnix
        logger := zerolog.
                         New(os.Stderr).
                         With().Caller().Timestamp().
                         Logger()

        return logger
}

Or a 'purer', more deterministic version:

func InitLoggerConfig(timeFormat string, output io.Writer) zerolog.Logger {
    zerolog.TimeFieldFormat = timeFormat
    logger := zerolog.
                     New(output).
                     With().Caller().Timestamp().
                     Logger()
    return logger
}

and an example test:

func TestInitLoggerConfig(t *testing.T) {
    // Prepare the test params
    timeFormat := zerolog.TimeFormatUnix
    var output bytes.Buffer

    logger := InitLoggerConfig(timeFormat, &output)

    // Log a test message
    logger.Info().Msg("Test message")

    assert.Contains(t, output.String(), "Test message")
    assert.Contains(t, output.String(), "INF")

    // Validate logger configuration
    assert.Equal(t, zerolog.TimeFormatUnix, zerolog.TimeFieldFormat)
}

so to speak seems like something I can catch up with the dendrite dev team on #dendrite-dev and do within a couple of days with ease.

[kuhnchris]

Hi there,

so I tried playing around with this, but it seems like dugong.NewFSHook also needs to first be adapted to zerolog aswell (seems to be a matrix-org wrapper for logrus) (from internal/log.go line 147, registering the hooks). I'll try if I see any more roadblocks, but do we need to fix this upstream first with dugong? https://github.com/matrix-org/dugong

[kuhnchris]

Also, i just saw that there is actually a util.GetLogger() (from matrix-org/util) - all other instances I saw up to now were using logrus directly, the first time i tripped over util.getLogger was in clientapi/routing/createRoom. Abstracting the logger object would be a good idea, so, should we actually migrate over to this while we change to zerolog, or should we keep this mashup of various ways of using the logger?

[S7evinK]

so I tried playing around with this, but it seems like dugong.NewFSHook also needs to first be adapted to zerolog aswell (seems to be a matrix-org wrapper for logrus) (from internal/log.go line 147, registering the hooks). I'll try if I see any more roadblocks, but do we need to fix this upstream first with dugong? https://github.com/matrix-org/dugong

IMHO we should get rid of dugong, there are plenty other methods to do log rotation/handle logging (e.g. for Docker). We shouldn't have to reinvent this. @kegsay What do you think?

Also, i just saw that there is actually a util.GetLogger() (from matrix-org/util) - all other instances I saw up to now were using logrus directly, the first time i tripped over util.getLogger was in clientapi/routing/createRoom. Abstracting the logger object would be a good idea, so, should we actually migrate over to this while we change to zerolog, or should we keep this mashup of various ways of using the logger?

zerolog also seems to provide something similar with zerolog.Ctx(ctx), so I'd probably just use that instead.

[lwileczek]

Zerolog is very performant; however, now that the standard library has structured logging, slog, it might be wise to use that instead. log/slog will be supported for the foreseeable future has few allocations and is rather simple. Zerolog is virtually dead although extremely popular.

Release post about slog: https://tip.golang.org/blog/slog

[ptman]

slog also provides a standard interface and implementations can be changed without touching that. But slog is fairly new and doesn't support older go versions well