Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-5744

Proposal: Switch all go logging in the library to slog

    XMLWordPrintableJSON

Details

    • Task
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 0.20.0
    • Go - Library
    • None

    Description

      In the go library, we used to use the stdlib 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 (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:

      1. TDebugProtocol: with this proposal we should switch them to slog.DebugContext
      2. 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.

      Attachments

        Issue Links

          Activity

            People

              fishywang Yuxuan Wang
              fishywang Yuxuan Wang
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 1h
                  1h