Skip to content

Commit

Permalink
pkg: remove capnslog
Browse files Browse the repository at this point in the history
  • Loading branch information
jingyih committed Feb 12, 2020
1 parent 61f2794 commit 84b85b4
Show file tree
Hide file tree
Showing 15 changed files with 66 additions and 54 deletions.
7 changes: 6 additions & 1 deletion etcdctl/ctlv3/command/ep_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"go.etcd.io/etcd/pkg/flags"

"github.com/spf13/cobra"
"go.uber.org/zap"
)

var epClusterEndpoints bool
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion etcdctl/ctlv3/command/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,18 @@ 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
// silence "pkg/flags: unrecognized environment variable ETCDCTL_WATCH_RANGE_END=bar" warnings
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 {
Expand Down
2 changes: 1 addition & 1 deletion etcdmain/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ func (cfg *config) configFromCmdLine() error {
}
}

err := flags.SetFlagsFromEnv("ETCD", cfg.cf.flagSet)
err := flags.SetFlagsFromEnv(cfg.ec.GetLogger(), "ETCD", cfg.cf.flagSet)
if err != nil {
return err
}
Expand Down
4 changes: 0 additions & 4 deletions pkg/fileutil/fileutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import (
"io/ioutil"
"os"
"path/filepath"

"github.com/coreos/pkg/capnslog"
)

const (
Expand All @@ -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 {
Expand Down
15 changes: 5 additions & 10 deletions pkg/fileutil/purge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
43 changes: 27 additions & 16 deletions pkg/flags/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,48 +21,46 @@ 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) {
alreadySet[FlagToEnv(prefix, f.Name)] = true
})
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)
fs.VisitAll(func(f *pflag.Flag) {
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
}

Expand All @@ -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))
}
}
}
}
Expand All @@ -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)
Expand All @@ -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),
)
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions pkg/flags/flag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"os"
"strings"
"testing"

"go.uber.org/zap"
)

func TestSetFlagsFromEnv(t *testing.T) {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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")
}
}
Expand All @@ -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
Expand Down
7 changes: 6 additions & 1 deletion pkg/flags/ignored.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/flags/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package flags

import (
"flag"
"fmt"
"sort"
"strings"
)
Expand All @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/flags/unique_strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package flags

import (
"flag"
"fmt"
"sort"
"strings"
)
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/flags/unique_urls.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package flags

import (
"flag"
"fmt"
"net/url"
"sort"
"strings"
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/flags/urls.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package flags

import (
"flag"
"fmt"
"net/url"
"strings"

Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/osutil/interrupt_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 0 additions & 4 deletions pkg/osutil/osutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
10 changes: 3 additions & 7 deletions pkg/pbutil/pbutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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))
}
}

Expand Down

0 comments on commit 84b85b4

Please sign in to comment.