Make log_level centrally configurable
#859
+140
−59
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,14 +24,26 @@ import ( | |
| "os" | ||
| "strings" | ||
| "sync" | ||
| "sync/atomic" | ||
| "time" | ||
|
|
||
| "go.elastic.co/fastjson" | ||
| ) | ||
|
|
||
| const ( | ||
| // EnvLogFile is the environment variable that controls where the default logger writes. | ||
| EnvLogFile = "ELASTIC_APM_LOG_FILE" | ||
|
|
||
| // EnvLogLevel is the environment variable that controls the default logger's level. | ||
| EnvLogLevel = "ELASTIC_APM_LOG_LEVEL" | ||
|
|
||
| // DefaultLevel holds the default log level, if EnvLogLevel is not specified. | ||
| DefaultLevel Level = ErrorLevel | ||
| ) | ||
|
|
||
| var ( | ||
| // DefaultLogger is the default Logger to use, if ELASTIC_APM_LOG_* are specified. | ||
| DefaultLogger *LevelLogger | ||
|
|
||
| fastjsonPool = &sync.Pool{ | ||
| New: func() interface{} { | ||
|
|
@@ -41,12 +53,15 @@ var ( | |
| ) | ||
|
|
||
| func init() { | ||
| InitDefaultLogger() | ||
| } | ||
|
|
||
| // InitDefaultLogger initialises DefaultLogger using the environment variables | ||
| // ELASTIC_APM_LOG_FILE and ELASTIC_APM_LOG_LEVEL. | ||
| func InitDefaultLogger() { | ||
| fileStr := strings.TrimSpace(os.Getenv(EnvLogFile)) | ||
| if fileStr == "" { | ||
| DefaultLogger = nil | ||
| return | ||
| } | ||
|
|
||
|
|
@@ -65,85 +80,98 @@ func initDefaultLogger() { | |
| logWriter = &syncFile{File: f} | ||
| } | ||
|
|
||
| logLevel := DefaultLevel | ||
| if levelStr := strings.TrimSpace(os.Getenv(EnvLogLevel)); levelStr != "" { | ||
| level, err := ParseLogLevel(levelStr) | ||
| if err != nil { | ||
| log.Printf("invalid %s %q, falling back to %q", EnvLogLevel, levelStr, logLevel) | ||
| } else { | ||
| logLevel = level | ||
| } | ||
| } | ||
| DefaultLogger = &LevelLogger{w: logWriter, level: logLevel} | ||
| } | ||
|
|
||
| // Log levels. | ||
| const ( | ||
| DebugLevel Level = iota | ||
| InfoLevel | ||
| WarnLevel | ||
| ErrorLevel | ||
| CriticalLevel | ||
| OffLevel | ||
| ) | ||
|
|
||
| // Level represents a log level. | ||
| type Level uint32 | ||
|
|
||
| func (l Level) String() string { | ||
| switch l { | ||
| case DebugLevel: | ||
| return "debug" | ||
| case InfoLevel: | ||
| return "info" | ||
| case WarnLevel: | ||
| return "warn" | ||
| case ErrorLevel: | ||
|
|
||
| return "error" | ||
| } | ||
| return "" | ||
| } | ||
|
|
||
| // ParseLogLevel parses s as a log level. | ||
| func ParseLogLevel(s string) (Level, error) { | ||
| switch strings.ToLower(s) { | ||
| case "debug": | ||
| return DebugLevel, nil | ||
| case "info": | ||
| return InfoLevel, nil | ||
| case "warn": | ||
|
There was a problem hiding this comment. (This is my first time looking at the go agent, and really reviewing much Go code at all, so feel free to ignore if I'm missing something obvious.) The spec uses "warning" rather than "warn". Is this function parsing level strings from central config? There was a problem hiding this comment. @trentm Thanks, nice pick up! You're quite right, I missed that. I'll open another PR to fix it. |
||
| return WarnLevel, nil | ||
| case "error": | ||
| return ErrorLevel, nil | ||
| case "critical": | ||
| return CriticalLevel, nil | ||
| case "off": | ||
| return OffLevel, nil | ||
| } | ||
| return OffLevel, fmt.Errorf("invalid log level string %q", s) | ||
| } | ||
|
|
||
| // LevelLogger is a level logging implementation that will log to a file, | ||
| // stdout, or stderr. The level may be updated dynamically via SetLevel. | ||
| type LevelLogger struct { | ||
| level Level // should be accessed with sync/atomic | ||
| w io.Writer | ||
| } | ||
|
|
||
| // Level returns the current logging level. | ||
| func (l *LevelLogger) Level() Level { | ||
| return Level(atomic.LoadUint32((*uint32)(&l.level))) | ||
| } | ||
|
|
||
| // SetLevel sets level as the minimum logging level. | ||
| func (l *LevelLogger) SetLevel(level Level) { | ||
| atomic.StoreUint32((*uint32)(&l.level), uint32(level)) | ||
| } | ||
|
|
||
| // Debugf logs a message with log.Printf, with a DEBUG prefix. | ||
| func (l *LevelLogger) Debugf(format string, args ...interface{}) { | ||
| l.logf(DebugLevel, format, args...) | ||
| } | ||
|
|
||
| // Errorf logs a message with log.Printf, with an ERROR prefix. | ||
| func (l *LevelLogger) Errorf(format string, args ...interface{}) { | ||
| l.logf(ErrorLevel, format, args...) | ||
| } | ||
|
|
||
| // Warningf logs a message with log.Printf, with a WARNING prefix. | ||
| func (l *LevelLogger) Warningf(format string, args ...interface{}) { | ||
| l.logf(WarnLevel, format, args...) | ||
| } | ||
|
|
||
| func (l *LevelLogger) logf(level Level, format string, args ...interface{}) { | ||
| if level < l.Level() { | ||
| return | ||
| } | ||
| jw := fastjsonPool.Get().(*fastjson.Writer) | ||
|
|
||
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
Oops, something went wrong.
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.
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.
I might be missing something - but are there any tests for actually applying the remote log level and also falling back to the local config if a remote config is no longer provided?
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.
testTracerCentralConfigUpdate(which this test calls) does that inapm-agent-go/config_test.go
Lines 142 to 165 in 879c34a
Each of these sub-tests defines a config key, value to set remotely, and a predicate that indicates whether the remote config has been applied. The code block I pointed to first checks that with the initial tracer config the predicate returns false; then sets the remote config and checks the predicate returns true; then clears out all remote config and again checks that the predicate returns false.