Skip to content

Commit

Permalink
clientv3: Bring back ETCD_CLIENT_DEBUG variable interpretation.
Browse files Browse the repository at this point in the history
env ETCD_CLIENT_DEBUG supports log levels (debug, info, warn, error, dpanic, panic, fatal).
Only when set, overrides application-wide grpc logging settings.
  • Loading branch information
ptabor committed Mar 18, 2021
1 parent 2932969 commit 9312d1b
Show file tree
Hide file tree
Showing 12 changed files with 72 additions and 127 deletions.
8 changes: 4 additions & 4 deletions CHANGELOG-3.5.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.4.0...v3.5.0) and
- ClientV3 supports [grpc resolver API](https://github.com/etcd-io/etcd/blob/master/client/v3/naming/resolver/resolver.go).
- Endpoints can be managed using [endpoints.Manager](https://github.com/etcd-io/etcd/blob/master/client/v3/naming/endpoints/endpoints.go)
- Previously supported [GRPCResolver was decomissioned](https://github.com/etcd-io/etcd/pull/12675). Use [resolver](https://github.com/etcd-io/etcd/blob/master/client/v3/naming/resolver/resolver.go) instead.
- Turned on [--pre-vote by default](https://github.com/etcd-io/etcd/pull/). Should prevent disrupting RAFT leader by an individual member.

### `etcdctl`
- Turned on [--pre-vote by default](https://github.com/etcd-io/etcd/pull/12770). Should prevent disrupting RAFT leader by an individual member.
- [ETCD_CLIENT_DEBUG env](https://github.com/etcd-io/etcd/pull/12786): Now supports log levels (debug, info, warn, error, dpanic, panic, fatal). Only when set, overrides application-wide grpc logging settings.
###

- Make sure [save snapshot downloads checksum for integrity checks](https://github.com/etcd-io/etcd/pull/11896).

Expand All @@ -77,7 +77,7 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.4.0...v3.5.0) and
- Add [`TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256` and `TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256` to `etcd --cipher-suites`](https://github.com/etcd-io/etcd/pull/11864).
- Changed [the format of WAL entries related to auth for not keeping password as a plain text](https://github.com/etcd-io/etcd/pull/11943).
- Add third party [Security Audit Report](https://github.com/etcd-io/etcd/pull/12201).
- A [log warning](https://github.com/etcd-io/etcd/pull/12242) is added when etcd use any existing directory that has a permission different than 700 on Linux and 777 on Windows.
- A [log warning](https://github.com/etcd-io/etcd/pull/12242) is added when etcd uses any existing directory that has a permission different than 700 on Linux and 777 on Windows.

### Metrics, Monitoring

Expand Down
13 changes: 7 additions & 6 deletions client/v3/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"go.etcd.io/etcd/client/v3/credentials"
"go.etcd.io/etcd/client/v3/internal/endpoint"
"go.etcd.io/etcd/client/v3/internal/resolver"
"go.etcd.io/etcd/pkg/v3/logutil"
"go.uber.org/zap"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -99,7 +98,9 @@ func NewFromURLs(urls []string) (*Client, error) {
return New(Config{Endpoints: urls})
}

// WithLogger sets a logger
// WithLogger overrides the logger.
// Does not changes grpcLogger, that can be explicitly configured
// using grpc_zap.ReplaceGrpcLoggerV2(..) method.
func (c *Client) WithLogger(lg *zap.Logger) *Client {
c.lgMu.Lock()
c.lg = lg
Expand Down Expand Up @@ -329,12 +330,12 @@ func newClient(cfg *Config) (*Client, error) {
lgMu: new(sync.RWMutex),
}

lcfg := logutil.DefaultZapLoggerConfig
var err error
if cfg.LogConfig != nil {
lcfg = *cfg.LogConfig
client.lg, err = cfg.LogConfig.Build()
} else {
client.lg, err = createDefaultZapLogger()
}
var err error
client.lg, err = lcfg.Build()
if err != nil {
return nil, err
}
Expand Down
51 changes: 48 additions & 3 deletions client/v3/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,61 @@
package clientv3

import (
"io/ioutil"
"log"
"os"

"go.etcd.io/etcd/pkg/v3/logutil"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"google.golang.org/grpc/grpclog"
)

func init() {
SetLogger(grpclog.NewLoggerV2(ioutil.Discard, ioutil.Discard, ioutil.Discard))
// 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") != "" {
// We don't use grpc_zap.ReplaceGrpcLoggerV2(lg) to not push (wide) set
// of grpc-ecosystem/go-grpc-middleware dependencies on etcd-client users.
lg, err := logutil.NewGRPCLoggerV2(createDefaultZapLoggerConfig())
if err != nil {
panic(err)
}

grpclog.SetLoggerV2(lg)
}
}

// SetLogger sets client-side Logger.
// SetLogger sets grpc logger.
//
// Deprecated: use grpclog.SetLoggerV2 directly or grpc_zap.ReplaceGrpcLoggerV2.
func SetLogger(l grpclog.LoggerV2) {
grpclog.SetLoggerV2(l)
}

// etcdClientDebugLevel translates ETCD_CLIENT_DEBUG into zap log level.
func etcdClientDebugLevel() zapcore.Level {
envLevel := os.Getenv("ETCD_CLIENT_DEBUG")
if envLevel == "" || envLevel == "true" {
return zapcore.InfoLevel
}
var l zapcore.Level
if err := l.Set(envLevel); err == nil {
log.Printf("Deprecated env ETCD_CLIENT_DEBUG value. Using default level: 'info'")
return zapcore.InfoLevel
}
return l
}

func createDefaultZapLoggerConfig() zap.Config {
lcfg := logutil.DefaultZapLoggerConfig
lcfg.Level = zap.NewAtomicLevelAt(etcdClientDebugLevel())
return lcfg
}

func createDefaultZapLogger() (*zap.Logger, error) {
c, err := createDefaultZapLoggerConfig().Build()
if err != nil {
return nil, err
}
return c.Named("etcd-client"), nil
}
27 changes: 0 additions & 27 deletions client/v3/ordering/logger_test.go

This file was deleted.

4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func clientConfigFromCmd(cmd *cobra.Command) *clientConfig {
ExitWithError(ExitError, err)
}
if debug {
clientv3.SetLogger(grpclog.NewLoggerV2WithVerbosity(os.Stderr, os.Stderr, os.Stderr, 4))
grpclog.SetLoggerV2(grpclog.NewLoggerV2WithVerbosity(os.Stderr, os.Stderr, os.Stderr, 4))
fs.VisitAll(func(f *pflag.Flag) {
fmt.Fprintf(os.Stderr, "%s=%v\n", flags.FlagToEnv("ETCDCTL", f.Name), f.Value)
})
Expand All @@ -140,7 +140,7 @@ func clientConfigFromCmd(cmd *cobra.Command) *clientConfig {
// too many routine connection disconnects to turn on by default.
//
// See https://github.com/etcd-io/etcd/pull/9623 for background
clientv3.SetLogger(grpclog.NewLoggerV2(ioutil.Discard, ioutil.Discard, os.Stderr))
grpclog.SetLoggerV2(grpclog.NewLoggerV2(ioutil.Discard, ioutil.Discard, os.Stderr))
}

cfg := &clientConfig{}
Expand Down
24 changes: 4 additions & 20 deletions pkg/logutil/log_level.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,16 @@
package logutil

import (
"fmt"

"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))
var level zapcore.Level
if err := level.Set(lvl); err != nil {
panic(err)
}
return level
}
2 changes: 1 addition & 1 deletion pkg/logutil/zap_grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func NewGRPCLoggerV2(lcfg zap.Config) (grpclog.LoggerV2, error) {
// if debug level is not enabled in "*zap.Logger".
func NewGRPCLoggerV2FromZapCore(cr zapcore.Core, syncer zapcore.WriteSyncer) grpclog.LoggerV2 {
// "AddCallerSkip" to annotate caller outside of "logutil"
lg := zap.New(cr, zap.AddCaller(), zap.AddCallerSkip(1), zap.ErrorOutput(syncer))
lg := zap.New(cr, zap.AddCaller(), zap.AddCallerSkip(1), zap.ErrorOutput(syncer)).Named("grpc")
return &zapGRPCLogger{lg: lg, sugar: lg.Sugar()}
}

Expand Down
27 changes: 0 additions & 27 deletions tests/integration/clientv3/logger_test.go

This file was deleted.

6 changes: 5 additions & 1 deletion tests/integration/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,11 @@ func NewClientV3(m *member) (*clientv3.Client, error) {
if m.DialOptions != nil {
cfg.DialOptions = append(cfg.DialOptions, m.DialOptions...)
}
return newClientV3(cfg)
c, err := newClientV3(cfg)
if err != nil {
return nil, err
}
return c.WithLogger(m.Logger.Named("client")), nil
}

// Clone returns a member with the same server configuration. The returned
Expand Down
27 changes: 0 additions & 27 deletions tests/integration/logger_test.go

This file was deleted.

8 changes: 0 additions & 8 deletions tests/integration/v2store/store_tag_v2v3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,13 @@
package v2store_test

import (
"io/ioutil"
"testing"

"go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/server/v3/etcdserver/api/v2store"
"go.etcd.io/etcd/server/v3/etcdserver/api/v2v3"
"go.etcd.io/etcd/tests/v3/integration"

"google.golang.org/grpc/grpclog"
)

func init() {
clientv3.SetLogger(grpclog.NewLoggerV2(ioutil.Discard, ioutil.Discard, ioutil.Discard))
}

type v2v3TestStore struct {
v2store.Store
clus *integration.ClusterV3
Expand Down
2 changes: 1 addition & 1 deletion tools/benchmark/cmd/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func mustCreateConn() *clientv3.Client {
return mustCreateConn()
}

clientv3.SetLogger(grpclog.NewLoggerV2(os.Stderr, os.Stderr, os.Stderr))
grpclog.SetLoggerV2(grpclog.NewLoggerV2(os.Stderr, os.Stderr, os.Stderr))

if err != nil {
fmt.Fprintf(os.Stderr, "dial error: %v\n", err)
Expand Down

0 comments on commit 9312d1b

Please sign in to comment.