[
https://issues.apache.org/jira/browse/THRIFT-5744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17787349#comment-17787349
]
Duru Can Celasun commented on THRIFT-5744:
------------------------------------------
I haven't had a chance to use slog at scale (we use zap) but it's clearly an
improvement over status quo for thrift. Let's go for it.
> Proposal: Switch all go logging in the library to slog
> ------------------------------------------------------
>
> Key: THRIFT-5744
> URL: https://issues.apache.org/jira/browse/THRIFT-5744
> Project: Thrift
> Issue Type: Task
> Components: Go - Library
> Reporter: Yuxuan Wang
> Assignee: Yuxuan Wang
> Priority: Major
>
> In the go library, we used to use the stdlib [log|https://pkg.go.dev/log]
> library to do logging. Then in THRIFT-4985 we added a simple wrapper
> interface, Logger, to replace all the logging usage in the library code.
> This Proposal is that we switch all internal logging to stdlib
> [slog|https://pkg.go.dev/log/slog] (after go 1.22 release so our minimal
> supported go version is at least 1.21 and has slog in stdlib), and
> deprecate/remove the Logger wrapper interface.
> In the current go library code, there are 2 places logging is used:
> # TDebugProtocol: with this proposal we should switch them to
> slog.DebugContext
> # TSimpleServer: we log errors from processRequests, so with this proposel we
> should switch them to slog.ErrorContext, and also add a SetBaseContext api to
> TSimpleServer so a base context can be set to be used in that logging
> The original, stdlib log approach, didn't work out well because the API of it
> is too limited (no log level, no context, no structured logging/kv pair
> ability, etc.), resulting in a lot of third-party logging library
> implementations cannot be adapted to the stdlib log api (even when some of
> them, for example zap, provided a way to replace the default log logger to be
> zap backend, because of the limitation of the api a lot of the features like
> log level and kv pairs are still unusable with log api), resulting in mixed
> logging in applications.
> The Logger wrapper interface resolved the mixed logging issue, but its API is
> still very limited (it's a common denominator approach), so it still have
> issues like lack of fine grain control of the logging, and performance (see
> THRIFT-5539, because the lack of log level we cannot skip the Sprintf used by
> TDebugProtocol when we don't need the logging, resulting in us forking out
> TDuplicateToProtocol).
> The new slog stdlib is go team's answer to the fragmentation of logging
> library issue in the go ecosystem, and it does have a really good chance to
> really unite all the logging libraries once and for all:
> * The context in logging calls provides the ability to: 1) attach context kv
> pairs automatically (trace id, etc.); 2) control minimal log level (you can
> provide a ctx before calling thrift code that could do potential logging to
> raise/lower minimal log level as needed); 3) or even do additional log
> suppressing logic based on the context
> * Even if some developers still prefer zap/zerolog/etc. for their performance
> (zap still claims to be faster than slog, for example), there are wrapper
> libraries to set the default slog handler to a zap/zerolog/etc.
> implementation so they still have uniformed logging, and the new API from
> slog means that they can still preserve the majority of the features from
> those third party logging libraries.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
