Request to Remove Internal Logging from Library

[ryancurrah]

I would like to bring up an issue regarding the use of an internal logger within the library. It's generally advised that libraries should avoid directly writing logs. This practice can lead to several challenges, such as cluttered log outputs and potential conflicts with the application's own logging strategy.

https://github.com/search?q=repo%3Astartreedata%2Fpinot-client-go+log.Error+NOT+path%3Aexamples%2F+NOT+path%3AREADME.md&type=code

Thank you for your work on this project, and I hope this suggestion contributes positively to its development.

[xiangfu0]

Thanks @ryancurrah for reporting this!

Agreed that the logger should be separated.

In terms of improving this, what we can do is:

1. Create a Logger interface

type Logger interface {
    Debug(args ...interface{})
    Info(args ...interface{})
    Warn(args ...interface{})
    Error(args ...interface{})
}

2. Implement default Logger interface for NoLogger and LogrusLogger

import (
    "github.com/sirupsen/logrus"
)

// LogrusLogger is an adapter for the logrus logger to satisfy the Logger interface.
type LogrusLogger struct {
    *logrus.Logger
}

func (l LogrusLogger) Debug(args ...interface{}) {
    l.Logger.Debug(args...)
}

func (l LogrusLogger) Info(args ...interface{}) {
    l.Logger.Info(args...)
}

func (l LogrusLogger) Warn(args ...interface{}) {
    l.Logger.Warn(args...)
}

func (l LogrusLogger) Error(args ...interface{}) {
    l.Logger.Error(args...)
}

3. Use the adapter during init the client:

// Initialize logrus logger
pinotClient, err := pinot.NewFromZookeeper([]string{"localhost:2123"}, "", "QuickStartCluster")
pinotClient.setLogger(LogrusLogger{Logger: logrus.New()})

[ryancurrah]

Logger interface is sufficient, I would also lean towards compatibility with the slog logging interface https://go.dev/blog/slog.