-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
*: make log level configurable #10947
Changes from all commits
348b0d4
e911f90
b9de4bd
a37f344
abba542
c6e3401
4e43a08
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 |
---|---|---|
|
@@ -69,11 +69,25 @@ func (cfg *Config) setupLogging() error { | |
return fmt.Errorf("'--log-output=%q' and '--log-outputs=%q' are incompatible; only set --log-outputs", cfg.DeprecatedLogOutput, cfg.LogOutputs) | ||
} | ||
if !reflect.DeepEqual(cfg.DeprecatedLogOutput, []string{DefaultLogOutput}) { | ||
fmt.Fprintf(os.Stderr, "Deprecated '--log-output' flag is set to %q\n", cfg.DeprecatedLogOutput) | ||
fmt.Fprintf(os.Stderr, "[WARNING] Deprecated '--log-output' flag is set to %q\n", cfg.DeprecatedLogOutput) | ||
fmt.Fprintln(os.Stderr, "Please use '--log-outputs' flag") | ||
} | ||
} | ||
|
||
// TODO: remove after deprecating log related flags in v3.5 | ||
if cfg.Debug { | ||
fmt.Fprintf(os.Stderr, "[WARNING] Deprecated '--debug' flag is set to %v (use '--log-level=debug' instead\n", cfg.Debug) | ||
} | ||
if cfg.Debug && cfg.LogLevel != "debug" { | ||
fmt.Fprintf(os.Stderr, "[WARNING] Deprecated '--debug' flag is set to %v with inconsistent '--log-level=%s' flag\n", cfg.Debug, cfg.LogLevel) | ||
} | ||
if cfg.Logger == "capnslog" { | ||
fmt.Fprintf(os.Stderr, "[WARNING] Deprecated '--logger=%s' flag is set; use '--logger=zap' flag instead\n", cfg.Logger) | ||
} | ||
if cfg.LogPkgLevels != "" { | ||
fmt.Fprintf(os.Stderr, "[WARNING] Deprecated '--log-package-levels=%s' flag is set; use '--logger=zap' flag instead\n", cfg.LogPkgLevels) | ||
} | ||
|
||
switch cfg.Logger { | ||
case "capnslog": // TODO: deprecate this in v3.5 | ||
cfg.ClientTLSInfo.HandshakeFailure = logTLSHandshakeFailure | ||
|
@@ -85,7 +99,7 @@ func (cfg *Config) setupLogging() error { | |
// enable info, warning, error | ||
grpclog.SetLoggerV2(grpclog.NewLoggerV2(os.Stderr, os.Stderr, os.Stderr)) | ||
} else { | ||
capnslog.SetGlobalLogLevel(capnslog.INFO) | ||
capnslog.SetGlobalLogLevel(logutil.ConvertToCapnslogLogLevel(cfg.LogLevel)) | ||
// only discard info | ||
grpclog.SetLoggerV2(grpclog.NewLoggerV2(ioutil.Discard, os.Stderr, os.Stderr)) | ||
} | ||
|
@@ -157,13 +171,15 @@ func (cfg *Config) setupLogging() error { | |
|
||
if !isJournal { | ||
copied := logutil.AddOutputPaths(logutil.DefaultZapLoggerConfig, outputPaths, errOutputPaths) | ||
|
||
if cfg.Debug { | ||
copied.Level = zap.NewAtomicLevelAt(zap.DebugLevel) | ||
copied.Level = zap.NewAtomicLevelAt(logutil.ConvertToZapLevel(cfg.LogLevel)) | ||
if cfg.Debug || cfg.LogLevel == "debug" { | ||
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. If 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.
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. So under configuration 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.
Yes, because 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. Sorry I think I am still confused. As an example, 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.
I clarified with comments in the code. Basically, we want to keep the backward compatibility since Let me know. 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. Oh I misread earlier. Thanks! |
||
// enable tracing even when "--debug --log-level info" | ||
// in order to keep backward compatibility with <= v3.3 | ||
// TODO: remove "Debug" check in v3.5 | ||
grpc.EnableTracing = true | ||
} | ||
if cfg.ZapLoggerBuilder == nil { | ||
cfg.ZapLoggerBuilder = func(c *Config) error { | ||
if cfg.zapLoggerBuilder == nil { | ||
cfg.zapLoggerBuilder = func(c *Config) error { | ||
var err error | ||
c.logger, err = copied.Build() | ||
if err != nil { | ||
|
@@ -201,9 +217,11 @@ func (cfg *Config) setupLogging() error { | |
return lerr | ||
} | ||
|
||
lvl := zap.NewAtomicLevelAt(zap.InfoLevel) | ||
if cfg.Debug { | ||
lvl = zap.NewAtomicLevelAt(zap.DebugLevel) | ||
lvl := zap.NewAtomicLevelAt(logutil.ConvertToZapLevel(cfg.LogLevel)) | ||
if cfg.Debug || cfg.LogLevel == "debug" { | ||
// enable tracing even when "--debug --log-level info" | ||
// in order to keep backward compatibility with <= v3.3 | ||
// TODO: remove "Debug" check in v3.5 | ||
grpc.EnableTracing = true | ||
} | ||
|
||
|
@@ -214,8 +232,8 @@ func (cfg *Config) setupLogging() error { | |
syncer, | ||
lvl, | ||
) | ||
if cfg.ZapLoggerBuilder == nil { | ||
cfg.ZapLoggerBuilder = func(c *Config) error { | ||
if cfg.zapLoggerBuilder == nil { | ||
cfg.zapLoggerBuilder = func(c *Config) error { | ||
c.logger = zap.New(cr, zap.AddCaller(), zap.ErrorOutput(syncer)) | ||
c.loggerMu.Lock() | ||
defer c.loggerMu.Unlock() | ||
|
@@ -231,7 +249,7 @@ func (cfg *Config) setupLogging() error { | |
} | ||
} | ||
|
||
err := cfg.ZapLoggerBuilder(cfg) | ||
err := cfg.zapLoggerBuilder(cfg) | ||
if err != nil { | ||
return err | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -173,15 +173,11 @@ Profiling and Monitoring: | |
|
||
Logging: | ||
--logger 'capnslog' | ||
Specify 'zap' for structured logging or 'capnslog'. | ||
Specify 'zap' for structured logging or 'capnslog'. [WARN] 'capnslog' will be deprecated in v3.5. | ||
--log-outputs 'default' | ||
Specify 'stdout' or 'stderr' to skip journald logging even when running under systemd, or list of comma separated output targets. | ||
--debug 'false' | ||
Enable debug-level logging for etcd. | ||
|
||
Logging (to be deprecated in v3.5): | ||
--log-package-levels '' | ||
Specify a particular log level for each etcd package (eg: 'etcdmain=CRITICAL,etcdserver=DEBUG'). | ||
--log-level 'info' | ||
Configures log level. Only supports debug, info, warn, error, panic, or fatal. | ||
|
||
v2 Proxy (to be deprecated in v4): | ||
--proxy 'off' | ||
|
@@ -214,5 +210,12 @@ Unsafe feature: | |
Force to create a new one-member cluster. | ||
|
||
CAUTIOUS with unsafe flag! It may break the guarantees given by the consensus protocol! | ||
|
||
TO BE DEPRECATED: | ||
|
||
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: remove the first empty line? 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. Fixed. |
||
--debug 'false' | ||
Enable debug-level logging for etcd. [WARN] Will be deprecated in v3.5. Use '--log-level=debug' instead. | ||
--log-package-levels '' | ||
Specify a particular log level for each etcd package (eg: 'etcdmain=CRITICAL,etcdserver=DEBUG'). | ||
` | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
// Copyright 2019 The etcd Authors | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package logutil | ||
|
||
import ( | ||
"fmt" | ||
|
||
"github.com/coreos/pkg/capnslog" | ||
"go.uber.org/zap" | ||
"go.uber.org/zap/zapcore" | ||
) | ||
|
||
var DefaultLogLevel = "info" | ||
|
||
// ConvertToZapLevel converts log level string to zapcore.Level. | ||
func ConvertToZapLevel(lvl string) zapcore.Level { | ||
switch lvl { | ||
case "debug": | ||
return zap.DebugLevel | ||
case "info": | ||
return zap.InfoLevel | ||
case "warn": | ||
return zap.WarnLevel | ||
case "error": | ||
return zap.ErrorLevel | ||
case "dpanic": | ||
return zap.DPanicLevel | ||
case "panic": | ||
return zap.PanicLevel | ||
case "fatal": | ||
return zap.FatalLevel | ||
default: | ||
panic(fmt.Sprintf("unknown level %q", lvl)) | ||
} | ||
} | ||
|
||
// ConvertToCapnslogLogLevel convert log level string to capnslog.LogLevel. | ||
// TODO: deprecate this in 3.5 | ||
func ConvertToCapnslogLogLevel(lvl string) capnslog.LogLevel { | ||
switch lvl { | ||
case "debug": | ||
return capnslog.DEBUG | ||
case "info": | ||
return capnslog.INFO | ||
case "warn": | ||
return capnslog.WARNING | ||
case "error": | ||
return capnslog.ERROR | ||
case "dpanic": | ||
return capnslog.CRITICAL | ||
case "panic": | ||
return capnslog.CRITICAL | ||
case "fatal": | ||
return capnslog.CRITICAL | ||
default: | ||
panic(fmt.Sprintf("unknown level %q", lvl)) | ||
} | ||
} |
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.
Should this just be a warning? This setting is actually conflicting with itself.
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.
Could you clarify? This is a warning when something like
etcd --debug --log-level warning
is configured. With this change, the log level will be overwritten by--log-level warning
.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 see. So
--log-level
, when provided, will overwrite--debug
.