From e647995a38edc34cd2b34ab23942b5419a0cca2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Fri, 10 Sep 2021 11:47:13 +0300 Subject: [PATCH 1/3] Add zap encoding configurable Json encoding is the default zap encoding value and can not be changeable. This PR enables configuring zap encoding to console via new flag `log-format`. --- CHANGELOG-3.6.md | 4 +++ client/pkg/logutil/log_format.go | 31 ++++++++++++++++++++++++ client/pkg/logutil/log_format_test.go | 35 +++++++++++++++++++++++++++ client/pkg/logutil/zap.go | 2 +- server/embed/config.go | 2 ++ server/embed/config_logging.go | 10 +++++++- server/etcdmain/config.go | 1 + server/etcdmain/help.go | 2 ++ 8 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 client/pkg/logutil/log_format.go create mode 100644 client/pkg/logutil/log_format_test.go diff --git a/CHANGELOG-3.6.md b/CHANGELOG-3.6.md index 360f25f4978..99c6b1f9713 100644 --- a/CHANGELOG-3.6.md +++ b/CHANGELOG-3.6.md @@ -28,3 +28,7 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.5.0...v3.6.0). - Package `mvcc/buckets` was moved to `storage/schema` - Package `wal` was moved to `storage/wal` - Package `datadir` was moved to `storage/datadir` + +### etcd server + +- Add [`etcd --log-format`](https://github.com/etcd-io/etcd/pull/13339) flag to support log format. \ No newline at end of file diff --git a/client/pkg/logutil/log_format.go b/client/pkg/logutil/log_format.go new file mode 100644 index 00000000000..a1366763e8e --- /dev/null +++ b/client/pkg/logutil/log_format.go @@ -0,0 +1,31 @@ +// 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 + +const ( + JsonLogFormat = "json" + ConsoleLogFormat = "console" +) + +var DefaultLogFormat = JsonLogFormat + +// ConvertToZapFormat converts log level string to zapcore.Level. +func ConvertToZapFormat(format string) string { + if format == ConsoleLogFormat { + return format + } + + return DefaultLogFormat +} diff --git a/client/pkg/logutil/log_format_test.go b/client/pkg/logutil/log_format_test.go new file mode 100644 index 00000000000..7b477770a82 --- /dev/null +++ b/client/pkg/logutil/log_format_test.go @@ -0,0 +1,35 @@ +// 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 "testing" + +func TestLogFormat(t *testing.T) { + tests := []struct { + given string + want string + }{ + {"json", JsonLogFormat}, + {"console", ConsoleLogFormat}, + {"", JsonLogFormat}, + } + + for i, tt := range tests { + got := ConvertToZapFormat(tt.given) + if got != tt.want { + t.Errorf("#%d: ConvertToZapFormat failure: want=%v, got=%v", i, tt.want, got) + } + } +} diff --git a/client/pkg/logutil/zap.go b/client/pkg/logutil/zap.go index 8fc6e03b77b..4f78756e39b 100644 --- a/client/pkg/logutil/zap.go +++ b/client/pkg/logutil/zap.go @@ -31,7 +31,7 @@ var DefaultZapLoggerConfig = zap.Config{ Thereafter: 100, }, - Encoding: "json", + Encoding: ConvertToZapFormat(DefaultLogFormat), // copied from "zap.NewProductionEncoderConfig" with some updates EncoderConfig: zapcore.EncoderConfig{ diff --git a/server/embed/config.go b/server/embed/config.go index 70bc26d8141..11ccbe9379e 100644 --- a/server/embed/config.go +++ b/server/embed/config.go @@ -361,6 +361,8 @@ type Config struct { Logger string `json:"logger"` // LogLevel configures log level. Only supports debug, info, warn, error, panic, or fatal. Default 'info'. LogLevel string `json:"log-level"` + // LogFormat set log encoding. Only supports json, console. Default is 'json'. + LogFormat string `json:"log-format"` // LogOutputs is either: // - "default" as os.Stderr, // - "stderr" as os.Stderr, diff --git a/server/embed/config_logging.go b/server/embed/config_logging.go index 9cb6e577765..05bf070bbaf 100644 --- a/server/embed/config_logging.go +++ b/server/embed/config_logging.go @@ -106,6 +106,7 @@ func (cfg *Config) setupLogging() error { copied.ErrorOutputPaths = errOutputPaths copied = logutil.MergeOutputPaths(copied) copied.Level = zap.NewAtomicLevelAt(logutil.ConvertToZapLevel(cfg.LogLevel)) + copied.Encoding = logutil.ConvertToZapFormat(cfg.LogFormat) if cfg.ZapLoggerBuilder == nil { lg, err := copied.Build() if err != nil { @@ -130,10 +131,17 @@ func (cfg *Config) setupLogging() error { lvl := zap.NewAtomicLevelAt(logutil.ConvertToZapLevel(cfg.LogLevel)) + var encoder zapcore.Encoder + if logutil.ConvertToZapFormat(cfg.LogFormat) == logutil.ConsoleLogFormat { + encoder = zapcore.NewConsoleEncoder(logutil.DefaultZapLoggerConfig.EncoderConfig) + } else { + encoder = zapcore.NewJSONEncoder(logutil.DefaultZapLoggerConfig.EncoderConfig) + } + // WARN: do not change field names in encoder config // journald logging writer assumes field names of "level" and "caller" cr := zapcore.NewCore( - zapcore.NewJSONEncoder(logutil.DefaultZapLoggerConfig.EncoderConfig), + encoder, syncer, lvl, ) diff --git a/server/etcdmain/config.go b/server/etcdmain/config.go index 116c6102237..0b9b28cc1dc 100644 --- a/server/etcdmain/config.go +++ b/server/etcdmain/config.go @@ -247,6 +247,7 @@ func newConfig() *config { fs.StringVar(&cfg.ec.Logger, "logger", "zap", "Currently only supports 'zap' for structured logging.") fs.Var(flags.NewUniqueStringsValue(embed.DefaultLogOutput), "log-outputs", "Specify 'stdout' or 'stderr' to skip journald logging even when running under systemd, or list of comma separated output targets.") fs.StringVar(&cfg.ec.LogLevel, "log-level", logutil.DefaultLogLevel, "Configures log level. Only supports debug, info, warn, error, panic, or fatal. Default 'info'.") + fs.StringVar(&cfg.ec.LogFormat, "log-format", logutil.DefaultLogFormat, "Configures log format. Only supports json, console. Default is 'json'.") fs.BoolVar(&cfg.ec.EnableLogRotation, "enable-log-rotation", false, "Enable log rotation of a single log-outputs file target.") fs.StringVar(&cfg.ec.LogRotationConfigJSON, "log-rotation-config-json", embed.DefaultLogRotationConfig, "Configures log rotation if enabled with a JSON logger config. Default: MaxSize=100(MB), MaxAge=0(days,no limit), MaxBackups=0(no limit), LocalTime=false(UTC), Compress=false(gzip)") diff --git a/server/etcdmain/help.go b/server/etcdmain/help.go index b0cf5d54448..1af8293340c 100644 --- a/server/etcdmain/help.go +++ b/server/etcdmain/help.go @@ -196,6 +196,8 @@ Logging: Specify 'stdout' or 'stderr' to skip journald logging even when running under systemd, or list of comma separated output targets. --log-level 'info' Configures log level. Only supports debug, info, warn, error, panic, or fatal. + --log-format 'json' + Configures log format. Only supports json, console. --enable-log-rotation 'false' Enable log rotation of a single log-outputs file target. --log-rotation-config-json '{"maxsize": 100, "maxage": 0, "maxbackups": 0, "localtime": false, "compress": false}' From ec252d06c92466210178dfa85c9126391a69ea2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 22 Sep 2021 19:45:08 +0300 Subject: [PATCH 2/3] Return error when log-format is invalid --- client/pkg/logutil/log_format.go | 17 +++++++++++------ client/pkg/logutil/log_format_test.go | 23 ++++++++++++++++------- client/pkg/logutil/zap.go | 2 +- server/embed/config_logging.go | 13 +++++++++++-- 4 files changed, 39 insertions(+), 16 deletions(-) diff --git a/client/pkg/logutil/log_format.go b/client/pkg/logutil/log_format.go index a1366763e8e..818bd2fbdfc 100644 --- a/client/pkg/logutil/log_format.go +++ b/client/pkg/logutil/log_format.go @@ -14,6 +14,8 @@ package logutil +import "fmt" + const ( JsonLogFormat = "json" ConsoleLogFormat = "console" @@ -21,11 +23,14 @@ const ( var DefaultLogFormat = JsonLogFormat -// ConvertToZapFormat converts log level string to zapcore.Level. -func ConvertToZapFormat(format string) string { - if format == ConsoleLogFormat { - return format +// ConvertToZapFormat converts and validated log format string. +func ConvertToZapFormat(format string) (string, error) { + switch format { + case ConsoleLogFormat: + return ConsoleLogFormat, nil + case JsonLogFormat: + return DefaultLogFormat, nil + default: + return "", fmt.Errorf("unknown log format: %s, supported values json, console", format) } - - return DefaultLogFormat } diff --git a/client/pkg/logutil/log_format_test.go b/client/pkg/logutil/log_format_test.go index 7b477770a82..23a0a1bfa1f 100644 --- a/client/pkg/logutil/log_format_test.go +++ b/client/pkg/logutil/log_format_test.go @@ -14,22 +14,31 @@ package logutil -import "testing" +import ( + "testing" +) func TestLogFormat(t *testing.T) { tests := []struct { - given string - want string + given string + want string + errExpected bool }{ - {"json", JsonLogFormat}, - {"console", ConsoleLogFormat}, - {"", JsonLogFormat}, + {"json", JsonLogFormat, false}, + {"console", ConsoleLogFormat, false}, + {"", "", true}, } for i, tt := range tests { - got := ConvertToZapFormat(tt.given) + got, err := ConvertToZapFormat(tt.given) if got != tt.want { t.Errorf("#%d: ConvertToZapFormat failure: want=%v, got=%v", i, tt.want, got) } + + if err != nil { + if !tt.errExpected { + t.Errorf("#%d: ConvertToZapFormat unexpected error: %v", i, err) + } + } } } diff --git a/client/pkg/logutil/zap.go b/client/pkg/logutil/zap.go index 4f78756e39b..33d95e9cb17 100644 --- a/client/pkg/logutil/zap.go +++ b/client/pkg/logutil/zap.go @@ -31,7 +31,7 @@ var DefaultZapLoggerConfig = zap.Config{ Thereafter: 100, }, - Encoding: ConvertToZapFormat(DefaultLogFormat), + Encoding: DefaultLogFormat, // copied from "zap.NewProductionEncoderConfig" with some updates EncoderConfig: zapcore.EncoderConfig{ diff --git a/server/embed/config_logging.go b/server/embed/config_logging.go index 05bf070bbaf..516ec73ef34 100644 --- a/server/embed/config_logging.go +++ b/server/embed/config_logging.go @@ -106,7 +106,11 @@ func (cfg *Config) setupLogging() error { copied.ErrorOutputPaths = errOutputPaths copied = logutil.MergeOutputPaths(copied) copied.Level = zap.NewAtomicLevelAt(logutil.ConvertToZapLevel(cfg.LogLevel)) - copied.Encoding = logutil.ConvertToZapFormat(cfg.LogFormat) + encoding, err := logutil.ConvertToZapFormat(cfg.LogFormat) + if err != nil { + return err + } + copied.Encoding = encoding if cfg.ZapLoggerBuilder == nil { lg, err := copied.Build() if err != nil { @@ -132,7 +136,12 @@ func (cfg *Config) setupLogging() error { lvl := zap.NewAtomicLevelAt(logutil.ConvertToZapLevel(cfg.LogLevel)) var encoder zapcore.Encoder - if logutil.ConvertToZapFormat(cfg.LogFormat) == logutil.ConsoleLogFormat { + encoding, err := logutil.ConvertToZapFormat(cfg.LogFormat) + if err != nil { + return err + } + + if encoding == logutil.ConsoleLogFormat { encoder = zapcore.NewConsoleEncoder(logutil.DefaultZapLoggerConfig.EncoderConfig) } else { encoder = zapcore.NewJSONEncoder(logutil.DefaultZapLoggerConfig.EncoderConfig) From 3c6e09f93253372d637af154cddf037146137f8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Thu, 23 Sep 2021 08:17:29 +0300 Subject: [PATCH 3/3] Handle empty log-format gracefully --- client/pkg/logutil/log_format.go | 2 ++ client/pkg/logutil/log_format_test.go | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/client/pkg/logutil/log_format.go b/client/pkg/logutil/log_format.go index 818bd2fbdfc..494ab33fb97 100644 --- a/client/pkg/logutil/log_format.go +++ b/client/pkg/logutil/log_format.go @@ -29,6 +29,8 @@ func ConvertToZapFormat(format string) (string, error) { case ConsoleLogFormat: return ConsoleLogFormat, nil case JsonLogFormat: + return JsonLogFormat, nil + case "": return DefaultLogFormat, nil default: return "", fmt.Errorf("unknown log format: %s, supported values json, console", format) diff --git a/client/pkg/logutil/log_format_test.go b/client/pkg/logutil/log_format_test.go index 23a0a1bfa1f..3c17061db7e 100644 --- a/client/pkg/logutil/log_format_test.go +++ b/client/pkg/logutil/log_format_test.go @@ -26,7 +26,8 @@ func TestLogFormat(t *testing.T) { }{ {"json", JsonLogFormat, false}, {"console", ConsoleLogFormat, false}, - {"", "", true}, + {"", JsonLogFormat, false}, + {"konsole", "", true}, } for i, tt := range tests {