Format Logs and add timestamp to logging output option #5898
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
a-palchikov
approved these changes
Mar 9, 2021
a-palchikov
reviewed
Mar 9, 2021
lib/config/configuration.go
Outdated
| for _, match := range tf.InputFormat { | ||
| switch match { | ||
| case "level": | ||
| // level |
|
LGTM once existing comments are resolved. |
xacrimon
approved these changes
Mar 9, 2021
a-palchikov
reviewed
Mar 10, 2021
lib/config/fileconf.go
Outdated
| @@ -459,6 +459,8 @@ type Log struct { | |||
| Output string `yaml:"output,omitempty"` | |||
| // Severity defines how verbose the log will be. Possible valus are "error", "info", "warn" | |||
| Severity string `yaml:"severity,omitempty"` | |||
| // Format defines the format in which each log line will be outputted. Example format: [timestamp, component, message] | |||
There was a problem hiding this comment.
Suggested change
| // Format defines the format in which each log line will be outputted. Example format: [timestamp, component, message] | |
| // Format lists the mandatory output fields from TextFormatterFields. Example format: [timestamp, component, message] |
hence I would store all known fields in a variable (TextFormatterFields) that can be referred to.
lib/config/formatter.go
Outdated
| } | ||
|
|
||
| // DefaultLoggerFormat is the default format in which to display logs (i.e. log.format in teleport.yaml is empty) | ||
| var DefaultLoggerFormat = []string{"level", "component", "message", "caller"} |
There was a problem hiding this comment.
nit: move known formatter field literals into corresponding consts since they're used on multiple occasions.
lib/config/formatter.go
Outdated
| // Defaults to filePathAndLine() if unspecified | ||
| FormatCaller func() (caller string) | ||
| // InitFormat is the initial format of log configuration | ||
| InitFormat []string |
There was a problem hiding this comment.
Why do you need both InitFormat and InputFormat - looks like they're the same most of the time and InitFormat is only used for validation.
lib/config/configuration_test.go
Outdated
| @@ -1363,3 +1363,37 @@ func TestDatabaseFlags(t *testing.T) { | |||
| }) | |||
| } | |||
| } | |||
|
|
|||
| func TestCheckAndSetDefaults(t *testing.T) { | |||
There was a problem hiding this comment.
The name of the test method does not hint that it has anything to do with the text formatter type.
klizhentas
approved these changes
Mar 11, 2021
a-palchikov
approved these changes
Mar 11, 2021
a-palchikov
reviewed
Mar 11, 2021
a-palchikov
reviewed
Mar 12, 2021
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
| if contains(res, timestampField) { |
There was a problem hiding this comment.
Sorry for late reply: contains is a dupe for api.types.SliceContainsStr
| @@ -56,10 +56,11 @@ teleport: | |||
|
|
|||
| # Logging configuration. Possible output values to disk via '/var/lib/teleport/teleport.log', | |||
| # 'stdout', 'stderr' and 'syslog'. Possible severity values are INFO, WARN | |||
| # and ERROR (default). | |||
| # and ERROR (default). Possible format values include: timestamp, component, message, caller, and level. | |||
| @@ -459,6 +459,8 @@ type Log struct { | |||
| Output string `yaml:"output,omitempty"` | |||
| // Severity defines how verbose the log will be. Possible valus are "error", "info", "warn" | |||
| Severity string `yaml:"severity,omitempty"` | |||
| // Format lists the output fields from KnownFormatFields. Example format: [timestamp, component, message] | |||
This was referenced Apr 12, 2021
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.
Purpose
Format logs with according to teleport config specification.
Implementation
Created a new formatter based off of logrus's text formatter, this new formatter writes to the log line in the order that the user specifies in the file.
Example log config in teleport config file:
Resolves #2676