Skip to content

feat: add option for injecting additional log fields #62

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dominicbarnes
Copy link

@dominicbarnes dominicbarnes commented May 25, 2021

I wanted the ability to inject some additional log fields at init time. Building on Option seemed like a logic fit for this functionality, which I've called "additional fields".

This should allow a user to use something like this when initializing their logger:

// multiple fields
WithAdditionalFields(map[string]interface{}{"hello", "world"})

// single field
WithAdditionalField("hello", "world")

It should be noted that I intentionally append the user-defined fields first so that in the event of a conflict, the internal ones will take precedence, which seemed like the right thing to do behaviorally.

@simukti
Copy link
Owner

simukti commented May 25, 2021

I wanted the ability to inject some additional log fields at init time. Building on Option seemed like a logic fit for this functionality, which I've called "additional fields".

Hi @dominicbarnes, I think it's better (and simpler) that you create a custom Logger implementation rather than put it as an option. The goal is the same, you need to add additional fields in every log but not replacing internal log content if it has the same key.

Let's take zerolog adapter as a base and modify it:

type zerologAdapter struct {
	logger zerolog.Logger
	additionalFields map[string]interface{}
}

func NewWithAdditionalFields(logger zerolog.Logger, additionalFields map[string]interface{}) sqldblogger.Logger {
	return &zerologAdapter{logger: logger, additionalFields: additionalFields}
}

func (zl *zerologAdapter) Log(_ context.Context, level sqldblogger.Level, msg string, data map[string]interface{}) {
	var lvl zerolog.Level
	switch level {
	case sqldblogger.LevelError:
		lvl = zerolog.ErrorLevel
	case sqldblogger.LevelInfo:
		lvl = zerolog.InfoLevel
	case sqldblogger.LevelDebug:
		lvl = zerolog.DebugLevel
	case sqldblogger.LevelTrace:
		lvl = zerolog.TraceLevel
	default:
		lvl = zerolog.DebugLevel
	}
	for k, v := range zl.additionalFields {
		if _, ok := data[k]; !ok {
			data[k] = v
		}
	}
	zl.logger.WithLevel(lvl).Fields(data).Msg(msg)
}

@dominicbarnes
Copy link
Author

I think both options would be valuable, seems like being able to generalize for any logger is preferred to needing to inject for each individual logger. That being said, you would only ever need 1 logger for any implementation, so maybe it's a moot point.

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.

2 participants