pkg: remove capnslog (#11616)

* pkg: remove capnslog

* CHANGELOG: function signature change
This commit is contained in:
Jingyi Hu 2020-02-12 11:15:41 -08:00 committed by GitHub
parent bb29615359
commit ae983523db
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 81 additions and 61 deletions

View File

@ -53,6 +53,9 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.4.0...v3.5.0) and
- Previously, `NewClusterProxy(c *clientv3.Client, advaddr string, prefix string) (pb.ClusterServer, <-chan struct{})`, now `NewClusterProxy(lg *zap.Logger, c *clientv3.Client, advaddr string, prefix string) (pb.ClusterServer, <-chan struct{})`.
- Previously, `Register(c *clientv3.Client, prefix string, addr string, ttl int)`, now `Register(lg *zap.Logger, c *clientv3.Client, prefix string, addr string, ttl int) <-chan struct{}`.
- Previously, `NewHandler(t *http.Transport, urlsFunc GetProxyURLs, failureWait time.Duration, refreshInterval time.Duration) http.Handler`, now `NewHandler(lg *zap.Logger, t *http.Transport, urlsFunc GetProxyURLs, failureWait time.Duration, refreshInterval time.Duration) http.Handler`.
- Changed `pkg/flags` function signature to [support structured logger](https://github.com/etcd-io/etcd/pull/11616).
- Previously, `SetFlagsFromEnv(prefix string, fs *flag.FlagSet) error`, now `SetFlagsFromEnv(lg *zap.Logger, prefix string, fs *flag.FlagSet) error`.
- Previously, `SetPflagsFromEnv(prefix string, fs *pflag.FlagSet) error`, now `SetPflagsFromEnv(lg *zap.Logger, prefix string, fs *pflag.FlagSet) error`.
### Metrics, Monitoring

View File

@ -25,6 +25,7 @@ import (
"go.etcd.io/etcd/pkg/flags"
"github.com/spf13/cobra"
"go.uber.org/zap"
)
var epClusterEndpoints bool
@ -85,7 +86,11 @@ type epHealth struct {
// epHealthCommandFunc executes the "endpoint-health" command.
func epHealthCommandFunc(cmd *cobra.Command, args []string) {
flags.SetPflagsFromEnv("ETCDCTL", cmd.InheritedFlags())
lg, err := zap.NewProduction()
if err != nil {
ExitWithError(ExitError, err)
}
flags.SetPflagsFromEnv(lg, "ETCDCTL", cmd.InheritedFlags())
initDisplayFromCmd(cmd)
sec := secureCfgFromCmd(cmd)

View File

@ -113,6 +113,10 @@ func (*discardValue) Set(string) error { return nil }
func (*discardValue) Type() string { return "" }
func clientConfigFromCmd(cmd *cobra.Command) *clientConfig {
lg, err := zap.NewProduction()
if err != nil {
ExitWithError(ExitError, err)
}
fs := cmd.InheritedFlags()
if strings.HasPrefix(cmd.Use, "watch") {
// silence "pkg/flags: unrecognized environment variable ETCDCTL_WATCH_KEY=foo" warnings
@ -120,7 +124,7 @@ func clientConfigFromCmd(cmd *cobra.Command) *clientConfig {
fs.AddFlag(&pflag.Flag{Name: "watch-key", Value: &discardValue{}})
fs.AddFlag(&pflag.Flag{Name: "watch-range-end", Value: &discardValue{}})
}
flags.SetPflagsFromEnv("ETCDCTL", fs)
flags.SetPflagsFromEnv(lg, "ETCDCTL", fs)
debug, err := cmd.Flags().GetBool("debug")
if err != nil {

View File

@ -308,20 +308,25 @@ 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()
if err != nil {
return err
}
verKey := "ETCD_VERSION"
if verVal := os.Getenv(verKey); verVal != "" {
// unset to avoid any possible side-effect.
os.Unsetenv(verKey)
if lg := cfg.ec.GetLogger(); lg != nil {
lg.Warn(
"cannot set special environment variable",
zap.String("key", verKey),
zap.String("value", verVal),
)
}
lg.Warn(
"cannot set special environment variable",
zap.String("key", verKey),
zap.String("value", verVal),
)
}
err := flags.SetFlagsFromEnv("ETCD", cfg.cf.flagSet)
err = flags.SetFlagsFromEnv(lg, "ETCD", cfg.cf.flagSet)
if err != nil {
return err
}

View File

@ -20,8 +20,6 @@ import (
"io/ioutil"
"os"
"path/filepath"
"github.com/coreos/pkg/capnslog"
)
const (
@ -31,8 +29,6 @@ const (
PrivateDirMode = 0700
)
var plog = capnslog.NewPackageLogger("go.etcd.io/etcd", "pkg/fileutil")
// IsDirWriteable checks if dir is writable by writing and removing a file
// to dir. It returns nil if dir is writable.
func IsDirWriteable(dir string) error {

View File

@ -37,6 +37,9 @@ func PurgeFileWithDoneNotify(lg *zap.Logger, dirname string, suffix string, max
// purgeFile is the internal implementation for PurgeFile which can post purged files to purgec if non-nil.
// if donec is non-nil, the function closes it to notify its exit.
func purgeFile(lg *zap.Logger, dirname string, suffix string, max uint, interval time.Duration, stop <-chan struct{}, purgec chan<- string, donec chan<- struct{}) <-chan error {
if lg == nil {
lg = zap.NewNop()
}
errC := make(chan error, 1)
go func() {
if donec != nil {
@ -67,19 +70,11 @@ func purgeFile(lg *zap.Logger, dirname string, suffix string, max uint, interval
return
}
if err = l.Close(); err != nil {
if lg != nil {
lg.Warn("failed to unlock/close", zap.String("path", l.Name()), zap.Error(err))
} else {
plog.Errorf("error unlocking %s when purging file (%v)", l.Name(), err)
}
lg.Warn("failed to unlock/close", zap.String("path", l.Name()), zap.Error(err))
errC <- err
return
}
if lg != nil {
lg.Info("purged", zap.String("path", f))
} else {
plog.Infof("purged file %s successfully", f)
}
lg.Info("purged", zap.String("path", f))
newfnames = newfnames[1:]
}
if purgec != nil {

View File

@ -21,18 +21,16 @@ import (
"os"
"strings"
"github.com/coreos/pkg/capnslog"
"github.com/spf13/pflag"
"go.uber.org/zap"
)
var plog = capnslog.NewPackageLogger("go.etcd.io/etcd", "pkg/flags")
// SetFlagsFromEnv parses all registered flags in the given flagset,
// and if they are not already set it attempts to set their values from
// environment variables. Environment variables take the name of the flag but
// are UPPERCASE, have the given prefix and any dashes are replaced by
// underscores - for example: some-flag => ETCD_SOME_FLAG
func SetFlagsFromEnv(prefix string, fs *flag.FlagSet) error {
func SetFlagsFromEnv(lg *zap.Logger, prefix string, fs *flag.FlagSet) error {
var err error
alreadySet := make(map[string]bool)
fs.Visit(func(f *flag.Flag) {
@ -40,17 +38,17 @@ func SetFlagsFromEnv(prefix string, fs *flag.FlagSet) error {
})
usedEnvKey := make(map[string]bool)
fs.VisitAll(func(f *flag.Flag) {
if serr := setFlagFromEnv(fs, prefix, f.Name, usedEnvKey, alreadySet, true); serr != nil {
if serr := setFlagFromEnv(lg, fs, prefix, f.Name, usedEnvKey, alreadySet, true); serr != nil {
err = serr
}
})
verifyEnv(prefix, usedEnvKey, alreadySet)
verifyEnv(lg, prefix, usedEnvKey, alreadySet)
return err
}
// SetPflagsFromEnv is similar to SetFlagsFromEnv. However, the accepted flagset type is pflag.FlagSet
// and it does not do any logging.
func SetPflagsFromEnv(prefix string, fs *pflag.FlagSet) error {
func SetPflagsFromEnv(lg *zap.Logger, prefix string, fs *pflag.FlagSet) error {
var err error
alreadySet := make(map[string]bool)
usedEnvKey := make(map[string]bool)
@ -58,11 +56,11 @@ func SetPflagsFromEnv(prefix string, fs *pflag.FlagSet) error {
if f.Changed {
alreadySet[FlagToEnv(prefix, f.Name)] = true
}
if serr := setFlagFromEnv(fs, prefix, f.Name, usedEnvKey, alreadySet, false); serr != nil {
if serr := setFlagFromEnv(lg, fs, prefix, f.Name, usedEnvKey, alreadySet, false); serr != nil {
err = serr
}
})
verifyEnv(prefix, usedEnvKey, alreadySet)
verifyEnv(lg, prefix, usedEnvKey, alreadySet)
return err
}
@ -71,20 +69,29 @@ func FlagToEnv(prefix, name string) string {
return prefix + "_" + strings.ToUpper(strings.Replace(name, "-", "_", -1))
}
func verifyEnv(prefix string, usedEnvKey, alreadySet map[string]bool) {
func verifyEnv(lg *zap.Logger, prefix string, usedEnvKey, alreadySet map[string]bool) {
for _, env := range os.Environ() {
kv := strings.SplitN(env, "=", 2)
if len(kv) != 2 {
plog.Warningf("found invalid env %s", env)
if lg != nil {
lg.Warn("found invalid environment variable", zap.String("environment-variable", env))
}
}
if usedEnvKey[kv[0]] {
continue
}
if alreadySet[kv[0]] {
plog.Fatalf("conflicting environment variable %q is shadowed by corresponding command-line flag (either unset environment variable or disable flag)", kv[0])
if lg != nil {
lg.Fatal(
"conflicting environment variable is shadowed by corresponding command-line flag (either unset environment variable or disable flag))",
zap.String("environment-variable", kv[0]),
)
}
}
if strings.HasPrefix(env, prefix+"_") {
plog.Warningf("unrecognized environment variable %s", env)
if lg != nil {
lg.Warn("unrecognized environment variable", zap.String("environment-variable", env))
}
}
}
}
@ -93,7 +100,7 @@ type flagSetter interface {
Set(fk string, fv string) error
}
func setFlagFromEnv(fs flagSetter, prefix, fname string, usedEnvKey, alreadySet map[string]bool, log bool) error {
func setFlagFromEnv(lg *zap.Logger, fs flagSetter, prefix, fname string, usedEnvKey, alreadySet map[string]bool, log bool) error {
key := FlagToEnv(prefix, fname)
if !alreadySet[key] {
val := os.Getenv(key)
@ -102,8 +109,12 @@ func setFlagFromEnv(fs flagSetter, prefix, fname string, usedEnvKey, alreadySet
if serr := fs.Set(fname, val); serr != nil {
return fmt.Errorf("invalid value %q for %s: %v", val, key, serr)
}
if log {
plog.Infof("recognized and used environment variable %s=%s", key, val)
if log && lg != nil {
lg.Info(
"recognized and used environment variable",
zap.String("variable-name", key),
zap.String("variable-value", val),
)
}
}
}

View File

@ -19,6 +19,8 @@ import (
"os"
"strings"
"testing"
"go.uber.org/zap"
)
func TestSetFlagsFromEnv(t *testing.T) {
@ -47,7 +49,7 @@ func TestSetFlagsFromEnv(t *testing.T) {
}
// now read the env and verify flags were updated as expected
err := SetFlagsFromEnv("ETCD", fs)
err := SetFlagsFromEnv(zap.NewExample(), "ETCD", fs)
if err != nil {
t.Errorf("err=%v, want nil", err)
}
@ -66,7 +68,7 @@ func TestSetFlagsFromEnvBad(t *testing.T) {
fs := flag.NewFlagSet("testing", flag.ExitOnError)
fs.Int("x", 0, "")
os.Setenv("ETCD_X", "not_a_number")
if err := SetFlagsFromEnv("ETCD", fs); err == nil {
if err := SetFlagsFromEnv(zap.NewExample(), "ETCD", fs); err == nil {
t.Errorf("err=nil, want != nil")
}
}
@ -81,7 +83,7 @@ func TestSetFlagsFromEnvParsingError(t *testing.T) {
}
defer os.Unsetenv("ETCD_HEARTBEAT_INTERVAL")
err := SetFlagsFromEnv("ETCD", fs)
err := SetFlagsFromEnv(zap.NewExample(), "ETCD", fs)
for _, v := range []string{"invalid syntax", "parse error"} {
if strings.Contains(err.Error(), v) {
err = nil

View File

@ -14,10 +14,13 @@
package flags
import "go.uber.org/zap"
// IgnoredFlag encapsulates a flag that may have been previously valid but is
// now ignored. If an IgnoredFlag is set, a warning is printed and
// operation continues.
type IgnoredFlag struct {
lg *zap.Logger
Name string
}
@ -27,7 +30,9 @@ func (f *IgnoredFlag) IsBoolFlag() bool {
}
func (f *IgnoredFlag) Set(s string) error {
plog.Warningf(`flag "-%s" is no longer supported - ignoring.`, f.Name)
if f.lg != nil {
f.lg.Warn("flag is no longer supported - ignoring", zap.String("flag-name", f.Name))
}
return nil
}

View File

@ -16,6 +16,7 @@ package flags
import (
"flag"
"fmt"
"sort"
"strings"
)
@ -41,7 +42,7 @@ func NewStringsValue(s string) (ss *StringsValue) {
}
ss = new(StringsValue)
if err := ss.Set(s); err != nil {
plog.Panicf("new StringsValue should never fail: %v", err)
panic(fmt.Sprintf("new StringsValue should never fail: %v", err))
}
return ss
}

View File

@ -16,6 +16,7 @@ package flags
import (
"flag"
"fmt"
"sort"
"strings"
)
@ -60,7 +61,7 @@ func NewUniqueStringsValue(s string) (us *UniqueStringsValue) {
return us
}
if err := us.Set(s); err != nil {
plog.Panicf("new UniqueStringsValue should never fail: %v", err)
panic(fmt.Sprintf("new UniqueStringsValue should never fail: %v", err))
}
return us
}

View File

@ -16,6 +16,7 @@ package flags
import (
"flag"
"fmt"
"net/url"
"sort"
"strings"
@ -76,7 +77,7 @@ func NewUniqueURLsWithExceptions(s string, exceptions ...string) *UniqueURLs {
return us
}
if err := us.Set(s); err != nil {
plog.Panicf("new UniqueURLs should never fail: %v", err)
panic(fmt.Sprintf("new UniqueURLs should never fail: %v", err))
}
return us
}

View File

@ -16,6 +16,7 @@ package flags
import (
"flag"
"fmt"
"net/url"
"strings"
@ -54,7 +55,7 @@ func NewURLsValue(s string) *URLsValue {
}
v := &URLsValue{}
if err := v.Set(s); err != nil {
plog.Panicf("new URLsValue should never fail: %v", err)
panic(fmt.Sprintf("new URLsValue should never fail: %v", err))
}
return v
}

View File

@ -61,8 +61,6 @@ func HandleInterrupts(lg *zap.Logger) {
if lg != nil {
lg.Info("received signal; shutting down", zap.String("signal", sig.String()))
} else {
plog.Noticef("received %v signal, shutting down...", sig)
}
for _, h := range ihs {

View File

@ -18,13 +18,9 @@ package osutil
import (
"os"
"strings"
"github.com/coreos/pkg/capnslog"
)
var (
plog = capnslog.NewPackageLogger("go.etcd.io/etcd", "pkg/osutil")
// support to override setting SIG_DFL so tests don't terminate early
setDflSignal = dflSignal
)

View File

@ -15,11 +15,7 @@
// Package pbutil defines interfaces for handling Protocol Buffer objects.
package pbutil
import "github.com/coreos/pkg/capnslog"
var (
plog = capnslog.NewPackageLogger("go.etcd.io/etcd", "pkg/pbutil")
)
import "fmt"
type Marshaler interface {
Marshal() (data []byte, err error)
@ -32,14 +28,14 @@ type Unmarshaler interface {
func MustMarshal(m Marshaler) []byte {
d, err := m.Marshal()
if err != nil {
plog.Panicf("marshal should never fail (%v)", err)
panic(fmt.Sprintf("marshal should never fail (%v)", err))
}
return d
}
func MustUnmarshal(um Unmarshaler, data []byte) {
if err := um.Unmarshal(data); err != nil {
plog.Panicf("unmarshal should never fail (%v)", err)
panic(fmt.Sprintf("unmarshal should never fail (%v)", err))
}
}