Skip to content

promslog: Make AllowedLevel concurrency safe. #754

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.

Sign up for GitHub

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

Merged
bwplotka merged 2 commits into from
Feb 3, 2025
Merged

promslog: Make AllowedLevel concurrency safe. #754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abb6c2d
promslog: Make AllowedLevel concurrency safe.
bwplotka Jan 31, 2025
d2c0a78
info is by default, so no need for the set in New
bwplotka Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions promslog/flag/flag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ var FormatFlagHelp = "Output format of log messages. One of: [" + strings.Join(p
// AddFlags adds the flags used by this package to the Kingpin application.
// To use the default Kingpin application, call AddFlags(kingpin.CommandLine)
func AddFlags(a *kingpin.Application, config *promslog.Config) {
config.Level = &promslog.AllowedLevel{}
config.Level = promslog.NewLevel()
a.Flag(LevelFlagName, LevelFlagHelp).
Default("info").HintOptions(promslog.LevelFlagOptions...).
SetValue(config.Level)

config.Format = &promslog.AllowedFormat{}
config.Format = promslog.NewFormat()
a.Flag(FormatFlagName, FormatFlagHelp).
Default("logfmt").HintOptions(promslog.FormatFlagOptions...).
SetValue(config.Format)
Expand Down
271 changes: 143 additions & 128 deletions promslog/slog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"time"
)

// LogStyle represents the common logging formats in the Prometheus ecosystem.
type LogStyle string

const (
Expand All @@ -38,115 +39,29 @@ const (
)

var (
LevelFlagOptions = []string{"debug", "info", "warn", "error"}
// LevelFlagOptions represents allowed logging levels.
LevelFlagOptions = []string{"debug", "info", "warn", "error"}
// FormatFlagOptions represents allowed formats.
FormatFlagOptions = []string{"logfmt", "json"}

callerAddFunc = false
defaultWriter = os.Stderr
goKitStyleReplaceAttrFunc = func(groups []string, a slog.Attr) slog.Attr {
key := a.Key
switch key {
case slog.TimeKey, "ts":
if t, ok := a.Value.Any().(time.Time); ok {
a.Key = "ts"

// This timestamp format differs from RFC3339Nano by using .000 instead
// of .999999999 which changes the timestamp from 9 variable to 3 fixed
// decimals (.130 instead of .130987456).
a.Value = slog.StringValue(t.UTC().Format("2006-01-02T15:04:05.000Z07:00"))
} else {
// If we can't cast the any from the value to a
// time.Time, it means the caller logged
// another attribute with a key of `ts`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to `logged_ts`.
a.Key = reservedKeyPrefix + key
}
case slog.SourceKey, "caller":
if src, ok := a.Value.Any().(*slog.Source); ok {
a.Key = "caller"
switch callerAddFunc {
case true:
a.Value = slog.StringValue(filepath.Base(src.File) + "(" + filepath.Base(src.Function) + "):" + strconv.Itoa(src.Line))
default:
a.Value = slog.StringValue(filepath.Base(src.File) + ":" + strconv.Itoa(src.Line))
}
} else {
// If we can't cast the any from the value to
// an *slog.Source, it means the caller logged
// another attribute with a key of `caller`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to
// `logged_caller`.
a.Key = reservedKeyPrefix + key
}
case slog.LevelKey:
if lvl, ok := a.Value.Any().(slog.Level); ok {
a.Value = slog.StringValue(strings.ToLower(lvl.String()))
} else {
// If we can't cast the any from the value to
// an slog.Level, it means the caller logged
// another attribute with a key of `level`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to `logged_level`.
a.Key = reservedKeyPrefix + key
}
default:
}

return a
}
defaultReplaceAttrFunc = func(groups []string, a slog.Attr) slog.Attr {
key := a.Key
switch key {
case slog.TimeKey:
if t, ok := a.Value.Any().(time.Time); ok {
a.Value = slog.TimeValue(t.UTC())
} else {
// If we can't cast the any from the value to a
// time.Time, it means the caller logged
// another attribute with a key of `time`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to `logged_time`.
a.Key = reservedKeyPrefix + key
}
case slog.SourceKey:
if src, ok := a.Value.Any().(*slog.Source); ok {
a.Value = slog.StringValue(filepath.Base(src.File) + ":" + strconv.Itoa(src.Line))
} else {
// If we can't cast the any from the value to
// an *slog.Source, it means the caller logged
// another attribute with a key of `source`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to
// `logged_source`.
a.Key = reservedKeyPrefix + key
}
case slog.LevelKey:
if _, ok := a.Value.Any().(slog.Level); !ok {
// If we can't cast the any from the value to
// an slog.Level, it means the caller logged
// another attribute with a key of `level`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to
// `logged_level`.
a.Key = reservedKeyPrefix + key
}
default:
}

return a
}
defaultWriter = os.Stderr
)

// AllowedLevel is a settable identifier for the minimum level a log entry
// must be have.
type AllowedLevel struct {
s string
// Level controls a logging level, with an info default.
// It wraps slog.LevelVar with string-based level control.
// Level is safe to be used concurrently.
type Level struct {
Copy link
Copy Markdown
Member Author

@bwplotka bwplotka Jan 31, 2025

Choose a reason for hiding this comment

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

As mentioned in the description - this (rename) is a breaking change, let me know if this is ok.

lvl *slog.LevelVar
}

func (l *AllowedLevel) UnmarshalYAML(unmarshal func(interface{}) error) error {
// NewLevel returns a new Level.
func NewLevel() *Level {
return &Level{
lvl: &slog.LevelVar{},
}
}

func (l *Level) UnmarshalYAML(unmarshal func(interface{}) error) error {
var s string
type plain string
if err := unmarshal((*plain)(&s)); err != nil {
Expand All @@ -155,55 +70,60 @@ func (l *AllowedLevel) UnmarshalYAML(unmarshal func(interface{}) error) error {
if s == "" {
return nil
}
lo := &AllowedLevel{}
if err := lo.Set(s); err != nil {
if err := l.Set(s); err != nil {
return err
}
*l = *lo
return nil
}

func (l *AllowedLevel) String() string {
return l.s
}

// Set updates the value of the allowed level.
func (l *AllowedLevel) Set(s string) error {
if l.lvl == nil {
l.lvl = &slog.LevelVar{}
// String returns the current level.
func (l *Level) String() string {
switch l.lvl.Level() {
case slog.LevelDebug:
return "debug"
case slog.LevelInfo:
return "info"
case slog.LevelWarn:
return "warn"
case slog.LevelError:
return "error"
default:
return ""
}
}

// Set updates the logging level with the validation.
func (l *Level) Set(s string) error {
switch strings.ToLower(s) {
case "debug":
l.lvl.Set(slog.LevelDebug)
callerAddFunc = true
case "info":
l.lvl.Set(slog.LevelInfo)
callerAddFunc = false
case "warn":
l.lvl.Set(slog.LevelWarn)
callerAddFunc = false
case "error":
l.lvl.Set(slog.LevelError)
callerAddFunc = false
default:
return fmt.Errorf("unrecognized log level %s", s)
}
l.s = s
return nil
}

// AllowedFormat is a settable identifier for the output format that the logger can have.
type AllowedFormat struct {
// Format controls a logging output format.
// Not concurrency-safe.
type Format struct {
Copy link
Copy Markdown
Member Author

@bwplotka bwplotka Jan 31, 2025

Choose a reason for hiding this comment

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

As mentioned in the description - this is a breaking change, let me know if this is ok.

Copy link
Copy Markdown
Member

@SuperQ SuperQ Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, OK this breaking change.

Any chance you would keep the old struct around and add a Deprecated comment so downstream Go users get a linter warning?

Copy link
Copy Markdown
Member

@SuperQ SuperQ Jan 31, 2025

Choose a reason for hiding this comment

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

Or, maybe it's better to just drop the struct so it's a compile error.

Copy link
Copy Markdown
Member Author

@bwplotka bwplotka Feb 3, 2025

Choose a reason for hiding this comment

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

For deprecated flow we would need to maintain a behaviour which means promslog.Config using old thing, we can but until we are v1 and this is not too painful we can break.

s string
}

func (f *AllowedFormat) String() string {
// NewFormat creates a new Format.
func NewFormat() *Format { return &Format{} }

func (f *Format) String() string {
return f.s
}

// Set updates the value of the allowed format.
func (f *AllowedFormat) Set(s string) error {
func (f *Format) Set(s string) error {
switch s {
case "logfmt", "json":
f.s = s
Expand All @@ -215,18 +135,113 @@ func (f *AllowedFormat) Set(s string) error {

// Config is a struct containing configurable settings for the logger
type Config struct {
Level *AllowedLevel
Format *AllowedFormat
Level *Level
Format *Format
Style LogStyle
Writer io.Writer
}

func newGoKitStyleReplaceAttrFunc(lvl *Level) func(groups []string, a slog.Attr) slog.Attr {
return func(groups []string, a slog.Attr) slog.Attr {
key := a.Key
switch key {
case slog.TimeKey, "ts":
if t, ok := a.Value.Any().(time.Time); ok {
a.Key = "ts"

// This timestamp format differs from RFC3339Nano by using .000 instead
// of .999999999 which changes the timestamp from 9 variable to 3 fixed
// decimals (.130 instead of .130987456).
a.Value = slog.StringValue(t.UTC().Format("2006-01-02T15:04:05.000Z07:00"))
} else {
// If we can't cast the any from the value to a
// time.Time, it means the caller logged
// another attribute with a key of `ts`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to `logged_ts`.
a.Key = reservedKeyPrefix + key
}
case slog.SourceKey, "caller":
if src, ok := a.Value.Any().(*slog.Source); ok {
a.Key = "caller"
switch lvl.String() {
case "debug":
a.Value = slog.StringValue(filepath.Base(src.File) + "(" + filepath.Base(src.Function) + "):" + strconv.Itoa(src.Line))
default:
a.Value = slog.StringValue(filepath.Base(src.File) + ":" + strconv.Itoa(src.Line))
}
} else {
// If we can't cast the any from the value to
// an *slog.Source, it means the caller logged
// another attribute with a key of `caller`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to
// `logged_caller`.
a.Key = reservedKeyPrefix + key
}
case slog.LevelKey:
if lvl, ok := a.Value.Any().(slog.Level); ok {
a.Value = slog.StringValue(strings.ToLower(lvl.String()))
} else {
// If we can't cast the any from the value to
// an slog.Level, it means the caller logged
// another attribute with a key of `level`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to `logged_level`.
a.Key = reservedKeyPrefix + key
}
default:
}
return a
}
}

func defaultReplaceAttr(_ []string, a slog.Attr) slog.Attr {
key := a.Key
switch key {
case slog.TimeKey:
if t, ok := a.Value.Any().(time.Time); ok {
a.Value = slog.TimeValue(t.UTC())
} else {
// If we can't cast the any from the value to a
// time.Time, it means the caller logged
// another attribute with a key of `time`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to `logged_time`.
a.Key = reservedKeyPrefix + key
}
case slog.SourceKey:
if src, ok := a.Value.Any().(*slog.Source); ok {
a.Value = slog.StringValue(filepath.Base(src.File) + ":" + strconv.Itoa(src.Line))
} else {
// If we can't cast the any from the value to
// an *slog.Source, it means the caller logged
// another attribute with a key of `source`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to
// `logged_source`.
a.Key = reservedKeyPrefix + key
}
case slog.LevelKey:
if _, ok := a.Value.Any().(slog.Level); !ok {
// If we can't cast the any from the value to
// an slog.Level, it means the caller logged
// another attribute with a key of `level`.
// Prevent duplicate keys (necessary for proper
// JSON) by renaming the key to
// `logged_level`.
a.Key = reservedKeyPrefix + key
}
default:
}
return a
}

// New returns a new slog.Logger. Each logged line will be annotated
// with a timestamp. The output always goes to stderr.
func New(config *Config) *slog.Logger {
if config.Level == nil {
config.Level = &AllowedLevel{}
_ = config.Level.Set("info")
config.Level = NewLevel()
}

if config.Writer == nil {
Expand All @@ -236,11 +251,11 @@ func New(config *Config) *slog.Logger {
logHandlerOpts := &slog.HandlerOptions{
Level: config.Level.lvl,
AddSource: true,
ReplaceAttr: defaultReplaceAttrFunc,
ReplaceAttr: defaultReplaceAttr,
}

if config.Style == GoKitStyle {
logHandlerOpts.ReplaceAttr = goKitStyleReplaceAttrFunc
logHandlerOpts.ReplaceAttr = newGoKitStyleReplaceAttrFunc(config.Level)
}

if config.Format != nil && config.Format.s == "json" {
Expand Down
Loading