feat: add ability to specify labels for all loggers #105
Conversation
setup.go
Outdated
| @@ -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" | |||
There was a problem hiding this comment.
How about app:example_app,dc=sjc-1, or app=example_app dc=sjc-1, etc. The current syntax will be very easy to get wrong.
There was a problem hiding this comment.
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.
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.
setup.go
Outdated
| @@ -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.
This condition isn't necessary (the loop will catch it).
setup.go
Outdated
| @@ -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 { | |||
| for i := 0; i < len(cfg.Labels); i=i+2 { | |||
setup.go
Outdated
| @@ -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.
This should be a map (or a slice of Label{Key, Value string} structs). The format we use in environment variables shouldn't leak into the go API.
|
(well, except the tests, not sure what's up with those) |
|
I can confirm the tests pass locally. I haven't dug into what the circleci issue is... |
|
Hm. The CircleCI deploy key got deleted. |
Resolves #104.