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

client: Bring back ETCD_CLIENT_DEBUG variable interpretation. #12786

Merged
merged 2 commits into from
Mar 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice :)

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
1 change: 1 addition & 0 deletions pkg/transport/sockopt_unix.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build !windows
// +build !windows

package transport
Expand Down
1 change: 1 addition & 0 deletions pkg/transport/sockopt_windows.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build windows
// +build windows

package transport
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