Skip to content

Commit

Permalink
server: Use default logging configuration instead of zap production one
Browse files Browse the repository at this point in the history
This fixes problem where logs json changes format of timestamp.
  • Loading branch information
serathius committed Mar 31, 2022
1 parent e5bf230 commit f7d1de2
Show file tree
Hide file tree
Showing 20 changed files with 134 additions and 59 deletions.
1 change: 1 addition & 0 deletions CHANGELOG/CHANGELOG-3.5.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ The minimum recommended etcd versions to run in **production** are 3.3.18+, 3.4.

### etcd server
- Fix [Provide a better liveness probe for when etcd runs as a Kubernetes pod](https://github.com/etcd-io/etcd/pull/13706)
- Fix [inconsistent log format](https://github.com/etcd-io/etcd/pull/13864)

### package `client/pkg/v3`

Expand Down
1 change: 1 addition & 0 deletions CHANGELOG/CHANGELOG-3.6.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.5.0...v3.6.0).
- Fix [A client can cause a nil dereference in etcd by passing an invalid SortTarget](https://github.com/etcd-io/etcd/pull/13555)
- Fix [Grant lease with negative ID can possibly cause db out of sync](https://github.com/etcd-io/etcd/pull/13676)
- Fix [segmentation violation(SIGSEGV) error due to premature unlocking of watchableStore](https://github.com/etcd-io/etcd/pull/13505)
- Fix [inconsistent log format](https://github.com/etcd-io/etcd/pull/13864)

### tools/benchmark

Expand Down
11 changes: 11 additions & 0 deletions client/pkg/logutil/zap.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ import (
"go.uber.org/zap/zapcore"
)

// CreateDefaultZapLogger creates a logger with default zap configuration
func CreateDefaultZapLogger(level zapcore.Level) (*zap.Logger, error) {
lcfg := DefaultZapLoggerConfig
lcfg.Level = zap.NewAtomicLevelAt(level)
c, err := lcfg.Build()
if err != nil {
return nil, err
}
return c, nil
}

// DefaultZapLoggerConfig defines default zap logger configuration.
var DefaultZapLoggerConfig = zap.Config{
Level: zap.NewAtomicLevelAt(ConvertToZapLevel(DefaultLogLevel)),
Expand Down
6 changes: 5 additions & 1 deletion client/v3/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"time"

"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
"go.etcd.io/etcd/client/pkg/v3/logutil"
"go.etcd.io/etcd/client/v3/credentials"
"go.etcd.io/etcd/client/v3/internal/endpoint"
"go.etcd.io/etcd/client/v3/internal/resolver"
Expand Down Expand Up @@ -370,7 +371,10 @@ func newClient(cfg *Config) (*Client, error) {
} else if cfg.LogConfig != nil {
client.lg, err = cfg.LogConfig.Build()
} else {
client.lg, err = CreateDefaultZapLogger()
client.lg, err = logutil.CreateDefaultZapLogger(etcdClientDebugLevel())
if client.lg != nil {
client.lg = client.lg.Named("etcd-client")
}
}
if err != nil {
return nil, err
Expand Down
22 changes: 2 additions & 20 deletions client/v3/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"os"

"go.etcd.io/etcd/client/pkg/v3/logutil"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zapgrpc"
"google.golang.org/grpc/grpclog"
Expand All @@ -29,10 +28,11 @@ func init() {
// We override grpc logger only when the environment variable is set
// in order to not interfere by default with user's code or other libraries.
if os.Getenv("ETCD_CLIENT_DEBUG") != "" {
lg, err := CreateDefaultZapLogger()
lg, err := logutil.CreateDefaultZapLogger(etcdClientDebugLevel())
if err != nil {
panic(err)
}
lg = lg.Named("etcd-client")
grpclog.SetLoggerV2(zapgrpc.NewLogger(lg))
}
}
Expand All @@ -57,21 +57,3 @@ func etcdClientDebugLevel() zapcore.Level {
}
return l
}

// CreateDefaultZapLoggerConfig creates a logger config that is configurable using env variable:
// ETCD_CLIENT_DEBUG= debug|info|warn|error|dpanic|panic|fatal|true (true=info)
func CreateDefaultZapLoggerConfig() zap.Config {
lcfg := logutil.DefaultZapLoggerConfig
lcfg.Level = zap.NewAtomicLevelAt(etcdClientDebugLevel())
return lcfg
}

// CreateDefaultZapLogger creates a logger that is configurable using env variable:
// ETCD_CLIENT_DEBUG= debug|info|warn|error|dpanic|panic|fatal|true (true=info)
func CreateDefaultZapLogger() (*zap.Logger, error) {
c, err := CreateDefaultZapLoggerConfig().Build()
if err != nil {
return nil, err
}
return c.Named("etcd-client"), nil
}
2 changes: 1 addition & 1 deletion client/v3/snapshot/v3_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func hasChecksum(n int64) bool {
// Etcd <v3.6 will return "" as version.
func SaveWithVersion(ctx context.Context, lg *zap.Logger, cfg clientv3.Config, dbPath string) (version string, err error) {
if lg == nil {
lg = zap.NewExample()
lg = zap.NewNop()
}
cfg.Logger = lg.Named("client")
if len(cfg.Endpoints) != 1 {
Expand Down
5 changes: 3 additions & 2 deletions etcdctl/ctlv3/command/ep_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
"go.etcd.io/etcd/client/pkg/v3/logutil"
"go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/cobrautl"
"go.etcd.io/etcd/pkg/v3/flags"
Expand Down Expand Up @@ -88,7 +89,7 @@ type epHealth struct {

// epHealthCommandFunc executes the "endpoint-health" command.
func epHealthCommandFunc(cmd *cobra.Command, args []string) {
lg, err := zap.NewProduction()
lg, err := logutil.CreateDefaultZapLogger(zap.InfoLevel)
if err != nil {
cobrautl.ExitWithError(cobrautl.ExitError, err)
}
Expand Down Expand Up @@ -260,7 +261,7 @@ func endpointsFromCluster(cmd *cobra.Command) []string {
cobrautl.ExitWithError(cobrautl.ExitError, err)
}
// exclude auth for not asking needless password (MemberList() doesn't need authentication)
lg, _ := zap.NewProduction()
lg, _ := logutil.CreateDefaultZapLogger(zap.InfoLevel)
cfg, err := clientv3.NewClientConfig(&clientv3.ConfigSpec{
Endpoints: eps,
DialTimeout: dt,
Expand Down
7 changes: 4 additions & 3 deletions etcdctl/ctlv3/command/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"time"

"github.com/bgentry/speakeasy"
"go.etcd.io/etcd/client/pkg/v3/logutil"
"go.etcd.io/etcd/client/pkg/v3/srv"
"go.etcd.io/etcd/client/pkg/v3/transport"
"go.etcd.io/etcd/client/v3"
Expand Down Expand Up @@ -88,7 +89,7 @@ func (*discardValue) Set(string) error { return nil }
func (*discardValue) Type() string { return "" }

func clientConfigFromCmd(cmd *cobra.Command) *clientv3.ConfigSpec {
lg, err := zap.NewProduction()
lg, err := logutil.CreateDefaultZapLogger(zap.InfoLevel)
if err != nil {
cobrautl.ExitWithError(cobrautl.ExitError, err)
}
Expand Down Expand Up @@ -137,7 +138,7 @@ func clientConfigFromCmd(cmd *cobra.Command) *clientv3.ConfigSpec {

func mustClientCfgFromCmd(cmd *cobra.Command) *clientv3.Config {
cc := clientConfigFromCmd(cmd)
lg, _ := zap.NewProduction()
lg, _ := logutil.CreateDefaultZapLogger(zap.InfoLevel)
cfg, err := clientv3.NewClientConfig(cc, lg)
if err != nil {
cobrautl.ExitWithError(cobrautl.ExitBadArgs, err)
Expand All @@ -151,7 +152,7 @@ func mustClientFromCmd(cmd *cobra.Command) *clientv3.Client {
}

func mustClient(cc *clientv3.ConfigSpec) *clientv3.Client {
lg, _ := zap.NewProduction()
lg, _ := logutil.CreateDefaultZapLogger(zap.InfoLevel)
cfg, err := clientv3.NewClientConfig(cc, lg)
if err != nil {
cobrautl.ExitWithError(cobrautl.ExitBadArgs, err)
Expand Down
3 changes: 2 additions & 1 deletion etcdctl/ctlv3/command/snapshot_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"

"github.com/spf13/cobra"
"go.etcd.io/etcd/client/pkg/v3/logutil"
snapshot "go.etcd.io/etcd/client/v3/snapshot"
"go.etcd.io/etcd/pkg/v3/cobrautl"
"go.uber.org/zap"
Expand Down Expand Up @@ -48,7 +49,7 @@ func snapshotSaveCommandFunc(cmd *cobra.Command, args []string) {
cobrautl.ExitWithError(cobrautl.ExitBadArgs, err)
}

lg, err := zap.NewProduction()
lg, err := logutil.CreateDefaultZapLogger(zap.InfoLevel)
if err != nil {
cobrautl.ExitWithError(cobrautl.ExitError, err)
}
Expand Down
3 changes: 2 additions & 1 deletion etcdutl/etcdutl/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@
package etcdutl

import (
"go.etcd.io/etcd/client/pkg/v3/logutil"
"go.etcd.io/etcd/pkg/v3/cobrautl"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
)

func GetLogger() *zap.Logger {
config := zap.NewProductionConfig()
config := logutil.DefaultZapLoggerConfig
config.Encoding = "console"
config.EncoderConfig.EncodeTime = zapcore.RFC3339TimeEncoder
lg, err := config.Build()
Expand Down
2 changes: 1 addition & 1 deletion etcdutl/snapshot/v3_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type Manager interface {
// NewV3 returns a new snapshot Manager for v3.x snapshot.
func NewV3(lg *zap.Logger) Manager {
if lg == nil {
lg = zap.NewExample()
lg = zap.NewNop()
}
return &v3Manager{lg: lg}
}
Expand Down
5 changes: 1 addition & 4 deletions pkg/netutil/netutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,7 @@ func URLStringsEqual(ctx context.Context, lg *zap.Logger, a []string, b []string
return false, err
}
if lg == nil {
lg, _ = zap.NewProduction()
if lg == nil {
lg = zap.NewExample()
}
lg = zap.NewNop()
}
return urlsEqual(ctx, lg, urlsA, urlsB)
}
Expand Down
11 changes: 1 addition & 10 deletions pkg/proxy/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,8 @@ var (
defaultDialTimeout = 3 * time.Second
defaultBufferSize = 48 * 1024
defaultRetryInterval = 10 * time.Millisecond
defaultLogger *zap.Logger
)

func init() {
var err error
defaultLogger, err = zap.NewProduction()
if err != nil {
panic(err)
}
}

// Server defines proxy server layer that simulates common network faults:
// latency spikes and packet drop or corruption. The proxy overhead is very
// small overhead (<500μs per request). Please run tests to compute actual
Expand Down Expand Up @@ -241,7 +232,7 @@ func NewServer(cfg ServerConfig) Server {
s.retryInterval = defaultRetryInterval
}
if s.lg == nil {
s.lg = defaultLogger
s.lg = zap.NewNop()
}

close(s.pauseAcceptc)
Expand Down
3 changes: 1 addition & 2 deletions server/etcdmain/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,10 @@ func (cfg *config) parse(arguments []string) error {

func (cfg *config) configFromCmdLine() error {
// user-specified logger is not setup yet, use this logger during flag parsing
lg, err := zap.NewProduction()
lg, err := logutil.CreateDefaultZapLogger(zap.InfoLevel)
if err != nil {
return err
}

verKey := "ETCD_VERSION"
if verVal := os.Getenv(verKey); verVal != "" {
// unset to avoid any possible side-effect.
Expand Down
12 changes: 8 additions & 4 deletions server/etcdmain/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package etcdmain
import (
"encoding/json"
"fmt"

"net/http"
"os"
"path/filepath"
Expand All @@ -27,6 +26,7 @@ import (
"time"

"go.etcd.io/etcd/client/pkg/v3/fileutil"
"go.etcd.io/etcd/client/pkg/v3/logutil"
"go.etcd.io/etcd/client/pkg/v3/transport"
"go.etcd.io/etcd/client/pkg/v3/types"
pkgioutil "go.etcd.io/etcd/pkg/v3/ioutil"
Expand Down Expand Up @@ -64,7 +64,7 @@ func startEtcdOrProxyV2(args []string) {
if lg == nil {
var zapError error
// use this logger
lg, zapError = zap.NewProduction()
lg, zapError = logutil.CreateDefaultZapLogger(zap.InfoLevel)
if zapError != nil {
fmt.Printf("error creating zap logger %v", zapError)
os.Exit(1)
Expand Down Expand Up @@ -470,6 +470,10 @@ func identifyDataDirOrDie(lg *zap.Logger, dir string) dirType {
}

func checkSupportArch() {
lg, err := logutil.CreateDefaultZapLogger(zap.InfoLevel)
if err != nil {
panic(err)
}
// to add a new platform, check https://github.com/etcd-io/website/blob/main/content/en/docs/next/op-guide/supported-platform.md
if runtime.GOARCH == "amd64" ||
runtime.GOARCH == "arm64" ||
Expand All @@ -481,10 +485,10 @@ func checkSupportArch() {
// so unset here to not parse through flag
defer os.Unsetenv("ETCD_UNSUPPORTED_ARCH")
if env, ok := os.LookupEnv("ETCD_UNSUPPORTED_ARCH"); ok && env == runtime.GOARCH {
fmt.Printf("running etcd on unsupported architecture %q since ETCD_UNSUPPORTED_ARCH is set\n", env)
lg.Info("running etcd on unsupported architecture since ETCD_UNSUPPORTED_ARCH is set", zap.String("arch", env))
return
}

fmt.Printf("etcd on unsupported platform without ETCD_UNSUPPORTED_ARCH=%s set\n", runtime.GOARCH)
lg.Error("running etcd on unsupported architecture since ETCD_UNSUPPORTED_ARCH is set", zap.String("arch", runtime.GOARCH))
os.Exit(1)
}
4 changes: 2 additions & 2 deletions server/etcdmain/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"os"
"time"

"go.etcd.io/etcd/client/pkg/v3/logutil"
"go.etcd.io/etcd/server/v3/proxy/tcpproxy"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -92,8 +93,7 @@ func stripSchema(eps []string) []string {
}

func startGateway(cmd *cobra.Command, args []string) {
var lg *zap.Logger
lg, err := zap.NewProduction()
lg, err := logutil.CreateDefaultZapLogger(zap.InfoLevel)
if err != nil {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
Expand Down
10 changes: 4 additions & 6 deletions server/etcdmain/grpc_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,16 +164,14 @@ func newGRPCProxyStartCommand() *cobra.Command {

func startGRPCProxy(cmd *cobra.Command, args []string) {
checkArgs()

lcfg := logutil.DefaultZapLoggerConfig
lvl := zap.InfoLevel
if grpcProxyDebug {
lcfg.Level = zap.NewAtomicLevelAt(zap.DebugLevel)
lvl = zap.DebugLevel
grpc.EnableTracing = true
}

lg, err := lcfg.Build()
lg, err := logutil.CreateDefaultZapLogger(lvl)
if err != nil {
log.Fatal(err)
panic(err)
}
defer lg.Sync()

Expand Down
2 changes: 1 addition & 1 deletion server/etcdmain/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func Main(args []string) {

func notifySystemd(lg *zap.Logger) {
if lg == nil {
lg = zap.NewExample()
lg = zap.NewNop()
}
lg.Info("notifying init daemon")
_, err := daemon.SdNotify(false, daemon.SdNotifyReady)
Expand Down
Loading

0 comments on commit f7d1de2

Please sign in to comment.