go-spatial / tegola

Tegola is a Mapbox Vector Tile server written in Go
http://tegola.io/
MIT License
1.25k stars 193 forks source link

feat: structured logs with uber/zap #848

Closed iwpnd closed 2 years ago

iwpnd commented 2 years ago

Hi 👋

As discussed in #831 I added the option to use structured logs with uber/zap while falling back to the currently implemented default if --logger is not specifically used. Log levels conform to the current implementation with the exception of log level TRACE that is not supported by zap. I use Debug as a fall back here.

Looking forward for your feedback

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 3f60b86ad-PR-848


Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/log/log.go 5 10 50.0%
cmd/tegola/cmd/root.go 2 18 11.11%
internal/log/zap.go 0 45 0.0%
<!-- Total: 7 73 9.59% -->
Totals Coverage Status
Change from base Build 7ac37b942: -0.2%
Covered Lines: 5583
Relevant Lines: 12319

💛 - Coveralls
iwpnd commented 2 years ago

The main thing I want to think through here is, do we really want to support 2 types of loggers?

While zap is fast, text logs have the edge in terms of performance because json serialization has an overhead. Giving the user the option is only fair as not every users wants to reap the benefits of json logs, or doesn't even care at all.

But ultimately that's your decision to make of course. :)

iwpnd commented 2 years ago

Do you want to move forward with this or drop @ARolek @gdey ?

ARolek commented 2 years ago

@iwpnd we should move forward with it! I know others have asked about this as well. Let me bump @gdey so he can provide some input as well.