configcat / go-sdk

ConfigCat SDK for Go. ConfigCat is a hosted feature flag service: https://configcat.com. Manage feature toggles across frontend, backend, mobile, desktop apps. Alternative to LaunchDarkly. Management app + feature flag SDKs.
https://configcat.com/docs/sdk-reference/go
MIT License
21 stars 6 forks source link

Support slog as custom logger #67

Closed dirsigler closed 11 months ago

dirsigler commented 11 months ago

Is your feature request related to a problem? Please describe.

The Golang ConfigCat SDK allows me to specify a custom logger here: https://configcat.com/docs/sdk-reference/go/#logging As the documentation states by default ConfigCat uses logrus as its logging mechanism.

For my projects I use the (currently experimental) slog package to log application functions and HTTP server calls.

Unfortunately it seems (or I am unable due to missing knowledge) that there is no support for the slog package as it is missing the Debugf method.

cannot use logger (variable of type *slog.Logger) as configcat.Logger value in struct literal: *slog.Logger does not implement configcat.Logger (missing method Debugf)

Describe the solution you'd like

For me it would make sense if ConfigCat does not enforce logrus on peoples project and instead uses the native log or slog package. Consumers of the SDK should be able to specify their own logger, including support for slog.

At least I would appreciate to make further use of my implemented logger via slog and use that for my ConfigCat client.

Describe alternatives you've considered

None

Additional context

Some code snippet:

func main() {
var logger *slog.Logger

logger = slog.New(slog.NewJSONHandler(os.Stdout, nil))

configcat := configcat.NewCustomClient(
    configcat.Config{
    SDKKey: "hunter2",
    Logger: logger,   // does not work
    },
)
defer configcat.Close()
}
z4kn4fein commented 11 months ago

Hi @dirsigler, thank you for reaching out!

I have plans to remove the logrus reference from the SDK and replace it with a more generic logging solution. However, it seems inevitable that due to the internal logging structure, we must enforce some kind of interface that the external log frameworks must satisfy or adapt to. Also, we can't conform each logging framework's API, some will need an adapter to work with the SDK. This is also the case now with slog.

Here's an example adapter for slog with the current SDK version:

type ConfigCatSlogAdapter struct {
    log   *slog.Logger
    level configcat.LogLevel
}

func NewAdapter(log *slog.Logger, level configcat.LogLevel) *ConfigCatSlogAdapter {
    return &ConfigCatSlogAdapter{log: log, level: level}
}

func (l *ConfigCatSlogAdapter) GetLevel() configcat.LogLevel {
    return l.level
}

func (l *ConfigCatSlogAdapter) Debugf(format string, args ...interface{}) {
    l.log.Debug(format, args)
}

func (l *ConfigCatSlogAdapter) Infof(format string, args ...interface{}) {
    l.log.Info(format, args)
}

func (l *ConfigCatSlogAdapter) Warnf(format string, args ...interface{}) {
    l.log.Warn(format, args)
}

func (l *ConfigCatSlogAdapter) Errorf(format string, args ...interface{}) {
    l.log.Error(format, args)
}

func main() {
    logger := slog.New(slog.NewJSONHandler(os.Stdout, nil))
    adapter := NewAdapter(logger, configcat.LogLevelWarn) // choose how verbose the ConfigCat SDK's log should be

    client := configcat.NewCustomClient(configcat.Config{
        SDKKey: "<sdk-key>",
        Logger: adapter,
    })
}

Let me know if this does not work for you or if you have further questions!

dirsigler commented 11 months ago

Thank you very much @z4kn4fein, I did not expect this level of detailed solution. This is fantastic!

I'll implement these changes and test them out.

dirsigler commented 11 months ago

Hey @z4kn4fein ,

so in theory it works with the solution provided above. In my case if I run my application I'll receive following log lines during initialisation of the ConfigCat client:

[...]
{"time":"2023-11-15T10:19:16.49332+01:00","level":"INFO","msg":"[0] fetching from %v","!BADKEY":["https://cdn-global.configcat.com"]}
{"time":"2023-11-15T10:20:16.492664+01:00","level":"INFO","msg":"[0] fetching from %v","!BADKEY":["https://cdn-global.configcat.com"]}
{"time":"2023-11-15T10:21:16.492115+01:00","level":"INFO","msg":"[0] fetching from %v","!BADKEY":["https://cdn-global.configcat.com"]}
[...]

To my knowledge that "!BADKEY" comes from slog because the provided message to it uses the fmt package syntax of "This is my dummy text. Hello %v", someVariable". slog in contrary uses something like key:value combinations to add further content like: "This is my dummy text.", "greeting", someVariable".

z4kn4fein commented 11 months ago

Hey @dirsigler,

Yes, it seems the issue is what you described. A quick solution could be manually constructing the message with fmt like:

func (l *ConfigCatSlogAdapter) Debugf(format string, args ...interface{}) {
    l.log.Debug(fmt.Sprintf(format, args))
}

Would this be a suitable solution for you?

dirsigler commented 11 months ago

This format has to applied to each severity level "construction".

like this:


func (l *ConfigCatSlogAdapter) Debugf(format string, args ...interface{}) {
    l.log.Debug(fmt.Sprintf(format, args...))
}

func (l *ConfigCatSlogAdapter) Infof(format string, args ...interface{}) {
    l.log.Info(fmt.Sprintf(format, args...))
}

func (l *ConfigCatSlogAdapter) Warnf(format string, args ...interface{}) {
    l.log.Warn(fmt.Sprintf(format, args...))
}

func (l *ConfigCatSlogAdapter) Errorf(format string, args ...interface{}) {
    l.log.Error(fmt.Sprintf(format, args...))
}

The solution itself works then as expected.

Example:

{"time":"2023-11-16T10:23:53.261944+01:00","level":"INFO","msg":"[0] fetching from https://cdn-global.configcat.com"}
{"time":"2023-11-16T10:23:53.262208+01:00","level":"ERROR","msg":"[1103] config fetch failed: unexpected error occurred while trying to fetch config JSON: Get \"https://cdn-global.configcat.com/configuration-files/<YOUR_SDK_KEY>/config_v5.json\": context canceled"}

Thank you very much for the help!