Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add custom logger for producer #294

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

0xJacky
Copy link

@0xJacky 0xJacky commented Oct 14, 2024

No description provided.

@CLAassistant
Copy link

CLAassistant commented Oct 14, 2024

CLA assistant check
All committers have signed the CLA.

@@ -27,7 +27,9 @@ func logConfig(producerConfig *ProducerConfig) log.Logger {
}
}
var logger log.Logger
if producerConfig.IsJsonType {
if producerConfig.Logger != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If custom logger is not nil, maybe we can just skip logConfig?

var logger log.Logger
if producerConfig.Logger != nil {
     logger = producerConfig.Logger
} else {
    logger = logConfig(producerConfig)
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we need the level filter in logConfig.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's ok, you could wrapper it yourself before passing to producerConfig, NewFilter also returns a new logger.
You can do anything you like

yourLogger := log.NewJSONLogger(writer)
yourLogger = level.NewFilter(yourLogger, level.AllowDebug())
yourLogger = log.With(yourLogger, "time", log.DefaultTimestampUTC, "caller", log.DefaultCaller)
yourLogger = log.With(yourLogger, "hello", "this is my machine 1.1.1.1")

Then you can pass it to producerConfig

producerConfig := GetDefaultProducerConfig()
producerConfig.Logger = yourLogger

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, if we modify the logger, it may override your settings unexpectedly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants