Skip to content
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

Add zap encoding configurable #13339

Merged
merged 4 commits into from
Oct 20, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG-3.6.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
31 changes: 31 additions & 0 deletions client/pkg/logutil/log_format.go
Original file line number Diff line number Diff line change
@@ -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
}
35 changes: 35 additions & 0 deletions client/pkg/logutil/log_format_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
2 changes: 1 addition & 1 deletion client/pkg/logutil/zap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 2 additions & 0 deletions server/embed/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 9 additions & 1 deletion server/embed/config_logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might have missed this but do we validate the logFormat? IE --log-format=konsole

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It returns json other than anything console including the empty string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should consider returning a clear error to admin that the config was not valid vs defaulting silently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, you are right. I'll add validation and warning user if log-format value is incorrect.

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,
)
Expand Down
1 change: 1 addition & 0 deletions server/etcdmain/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)")

Expand Down
2 changes: 2 additions & 0 deletions server/etcdmain/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}'
Expand Down