Add support for json logging #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
jasonodonnell
suggested changes
Jan 21, 2020
deploy/injector-deployment.yaml
Outdated
| @@ -32,6 +32,8 @@ spec: | |||
| value: ":8080" | |||
| - name: AGENT_INJECT_LOG_LEVEL | |||
| value: "info" | |||
| - name: AGENT_INJECT_LOG_FORMAT | |||
| - value: "json" | |||
There was a problem hiding this comment.
I don't think he default should be json. It's nice that there's a configurable for this now, however, not everyone is going to want JSON by default.
There was a problem hiding this comment.
I agree, default log settings should be similar to hashicorp/vault. Fixed.
# Conflicts: # subcommand/injector/command.go
|
This is really great @lalbers ! Looking at the field names there is the "@" sign in front. Not that this breaks anything, but in the context of shipping stuff to an ElasticSearch this sometime needs extra treatment. Especially when looking at "@timestamp" ... Is there maybe an easy way to remove those? |
jasonodonnell
approved these changes
Feb 4, 2020
jasonodonnell
added
enhancement
jasonodonnell
suggested changes
Feb 4, 2020
subcommand/injector/flags.go
Outdated
| } | ||
|
|
||
| func (c *Command) init() { | ||
| c.flagSet = flag.NewFlagSet("", flag.ContinueOnError) | ||
| c.flagSet.StringVar(&c.flagListen, "listen", ":8080", "Address to bind listener to.") | ||
| c.flagSet.StringVar(&c.flagLogLevel, "log-level", DefaultLogLevel, "Log verbosity level. Supported values "+ | ||
| `(in order of detail) are "trace", "debug", "info", "warn", and "err".`) | ||
| c.flagSet.StringVar(&c.flagLogFormat, "log-level", DefaultLogFormat, "Log output format. "+ |
There was a problem hiding this comment.
Because log-level is redefined here (it should be log-format), this is causing the injector to panic.
There was a problem hiding this comment.
flag redefined: log-level
panic: flag redefined: log-level
goroutine 1 [running]:
flag.(*FlagSet).Var(0xc0001ea5a0, 0x1435de0, 0xc0001fa210, 0x1291171, 0x9, 0x12cd800, 0x3d)
/usr/local/Cellar/go/1.12.7/libexec/src/flag/flag.go:850 +0x4af
flag.(*FlagSet).StringVar(...)
/usr/local/Cellar/go/1.12.7/libexec/src/flag/flag.go:753
github.com/hashicorp/vault-k8s/subcommand/injector.(*Command).init(0xc0001fa1e0)
/Users/jasonodonnell/Git/vault-k8s/subcommand/injector/flags.go:59 +0x1b5
sync.(*Once).Do(0xc0001fa298, 0xc00006bdd0)
/usr/local/Cellar/go/1.12.7/libexec/src/sync/once.go:44 +0xb3
github.com/hashicorp/vault-k8s/subcommand/injector.(*Command).Run(0xc0001fa1e0, 0xc00003a0b0, 0x1, 0x1, 0x0)
/Users/jasonodonnell/Git/vault-k8s/subcommand/injector/command.go:53 +0xe1
github.com/mitchellh/cli.(*CLI).Run(0xc0001afe00, 0x9, 0xc0001d8400, 0x128fea8)
/Users/jasonodonnell/go/pkg/mod/github.com/mitchellh/cli@v1.0.0/cli.go:255 +0x1f1
main.main()
/Users/jasonodonnell/Git/vault-k8s/main.go:17 +0x16b
lalbers
commented
Feb 14, 2020
|
Fixed flag-name |
jasonodonnell
approved these changes
Feb 20, 2020
Merged
RemcoBuddelmeijer
pushed a commit
to RemcoBuddelmeijer/vault-k8s
that referenced
this pull request
Feb 22, 2022
* Add support for json logging * adjust default logging format to txt * add unit test for log_format env * correct code indents for command.go * Update flags.go
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to issue #47.
Enable support for configurable structured json logging.