-
Notifications
You must be signed in to change notification settings - Fork 49
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 ability to specify labels for all loggers #105
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ const ( | |
envLoggingURL = "GOLOG_URL" // url that will be processed by sink in the zap | ||
|
||
envLoggingOutput = "GOLOG_OUTPUT" // possible values: stdout|stderr|file combine multiple values with '+' | ||
envLoggingLabels = "GOLOG_LOG_LABELS" // comma-separated key-value pairs, i.e. "app,example_app,dc,sjc-1" | ||
) | ||
|
||
type LogFormat int | ||
|
@@ -61,6 +62,9 @@ type Config struct { | |
|
||
// URL with schema supported by zap. Use zap.RegisterSink | ||
URL string | ||
|
||
// Labels is a set of key-values to apply to all loggers | ||
Labels []string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a map (or a slice of |
||
} | ||
|
||
// ErrNoSuchLogger is returned when the util pkg is asked for a non existant logger | ||
|
@@ -122,6 +126,13 @@ func SetupLogging(cfg Config) { | |
} | ||
|
||
newPrimaryCore := newCore(primaryFormat, ws, LevelDebug) // the main core needs to log everything. | ||
|
||
if len(cfg.Labels) > 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This condition isn't necessary (the loop will catch it). |
||
for i := 0; i < len(cfg.Labels); i=i+2 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
newPrimaryCore = newPrimaryCore.With([]zap.Field{zap.String(cfg.Labels[i], cfg.Labels[i+1])}) | ||
} | ||
} | ||
|
||
if primaryCore != nil { | ||
loggerCore.ReplaceCore(primaryCore, newPrimaryCore) | ||
} else { | ||
|
@@ -293,5 +304,15 @@ func configFromEnv() Config { | |
} | ||
} | ||
|
||
labels := os.Getenv(envLoggingLabels) | ||
if labels != "" { | ||
labelKVs := strings.Split(labels, ",") | ||
if len(labelKVs)%2 != 0 { | ||
fmt.Fprint(os.Stderr, "odd number of args for GOLOG_LOG_LABELS; please specify complete key-value pairs") | ||
} else { | ||
cfg.Labels = labelKVs | ||
} | ||
} | ||
|
||
return cfg | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
app:example_app,dc=sjc-1
, orapp=example_app dc=sjc-1
, etc. The current syntax will be very easy to get wrong.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
app=example_app,dc=sjc-1
suitable? You mixed the assignment operator in the first example and I am not sure if that is on purpose.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sorry, that wasn't on purpose. Your solution also works. Really, anything where I can easily read the labels and understand what's going on.