diff --git a/.circleci/config.yml b/.circleci/config.yml index 49228ae..6707ae9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,7 +3,8 @@ jobs: build: working_directory: ~/repo docker: - - image: circleci/golang:1.17.2 + - image: circleci/golang:1.17.3 + resource_class: xlarge # to avoid sfxclient timeouts lets use a bigger box steps: - checkout - restore_cache: @@ -20,7 +21,7 @@ jobs: name: Run tests command: | set -x - go test -covermode atomic -race -timeout 120s -parallel 4 -coverprofile ./coverage.out $(go list ./...) + go test -covermode atomic -race -timeout 120s -coverprofile ./coverage.out $(go list ./...) cov="$(go tool cover -func coverage.out | grep -v 100.0% || true)" echo $cov test -z "$cov" diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 2e49400..8075054 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -16,7 +16,7 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v2 with: - version: v1.41.1 + version: v1.42.1 # Optional: golangci-lint command line arguments. args: '--config=.golangci.yml -v' diff --git a/.golangci.yml b/.golangci.yml index 9fe96e7..0809a56 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,9 +1,12 @@ run: - timeout: 2m + timeout: 5m tests: true skip-dirs: + - src/external_libs + - gocat - genfiles$ - vendor$ + - ketama # which files to skip: they will be analyzed, but issues from them # won't be reported. Default value is empty list, but there is # no need to include all autogenerated files, we confidently recognize @@ -91,33 +94,131 @@ linters-settings: funlen: lines: 500 # TODO: need to set this to 150 statements and work on it statements: 150 + ifshort: + # Maximum length of variable declaration measured in number of lines, after which linter won't suggest using short syntax. + # Has higher priority than max-decl-chars. + max-decl-lines: 1 + # Maximum length of variable declaration measured in number of characters, after which linter won't suggest using short syntax. + max-decl-chars: 30 + cyclop: + # the maximal code complexity to report + max-complexity: 15 + # should ignore tests (default false) + skip-tests: false + gosec: + # To select a subset of rules to run. + # Available rules: https://github.com/securego/gosec#available-rules + includes: + - G101 # Look for hard coded credentials + - G102 # Bind to all interfaces + - G106 # Audit the use of ssh.InsecureIgnoreHostKey + - G107 # Url provided to HTTP request as taint input + - G108 # Profiling endpoint automatically exposed on /debug/pprof + - G109 # Potential Integer overflow made by strconv.Atoi result conversion to int16/32 + - G110 # Potential DoS vulnerability via decompression bomb + - G306 # Poor file permissions used when writing to a new file + - G401 # Detect the usage of DES, RC4, MD5 or SHA1 + - G501 # Import blocklist: crypto/md5 + - G502 # Import blocklist: crypto/des + - G503 # Import blocklist: crypto/rc4 + - G504 # Import blocklist: net/http/cgi + - G505 # Import blocklist: crypto/sha1 + - G601 # Implicit memory aliasing of items from a range statement + # To specify a set of rules to explicitly exclude. + # Available rules: https://github.com/securego/gosec#available-rules + excludes: + - G204 # Audit use of command execution + # To specify the configuration of rules. + # The configuration of rules is not fully documented by gosec: + # https://github.com/securego/gosec#configuration + # https://github.com/securego/gosec/blob/569328eade2ccbad4ce2d0f21ee158ab5356a5cf/rules/rulelist.go#L60-L102 + config: + G306: "0600" + G101: + pattern: "(?i)pwd|password|private_key|secret" + ignore_entropy: false + entropy_threshold: "80.0" + per_char_threshold: "3.0" + truncate: "32" + staticcheck: + # Select the Go version to target. The default is '1.13'. + go: "1.17" + # https://staticcheck.io/docs/options#checks + checks: [ "all" ] + dupl: + # tokens count to trigger issue, 150 by default + threshold: 100 linters: + enable: # please use alphabetical order when enabling any linters + - asciicheck + - bodyclose + - cyclop + - deadcode + - depguard + - dupl + - durationcheck + - errorlint + - errname + - exportloopref + - forbidigo + - forcetypeassert + - gci + - gocognit + - goconst + - gocyclo + - godox + - gofmt + - gofumpt + - goheader + - goimports + - gomodguard + - goprintffuncname + - gosec + - gosimple + - govet + - ifshort + - ineffassign + - makezero + - megacheck + - misspell + - nakedret + - nilerr + - noctx + - prealloc + - predeclared + - revive + - staticcheck + - structcheck + - stylecheck + - testpackage + - thelper + - tparallel + - typecheck + - unconvert + - unparam + - unused + - wastedassign + - whitespace enable-all: true disable: - - ifshort - - forcetypeassert - # need to enable linters above this line - lll - - gochecknoinits - paralleltest # this has lots of test cases that needs to be added as parallel and some of them have race condition so disabling after some effort - gochecknoglobals - - errcheck - - unparam - - interfacer - - dupl - - exhaustive # not urgent this currently as its complaining mainly about switch statements in sharecache + - exhaustive # TODO: need to enable this two low priority for better coding guidelines in terms of space between condition - - whitespace - wsl - godot - - gomnd - - nestif + - gomnd # we cannot enable this as there are lots of magic numbers + - nestif # cannot enable this given the way the code structure is organized for unit testing - nlreturn - nolintlint - tagliatelle # cannot be enabled as it's asking to modify `json` which is not possible - promlinter # we do not care above prometheus linter for metrics - gomoddirectives # needs to be disabled as we need replacement for jaeger & redis + - scopelint # deprecated and hence disabled it + - interfacer # deprecated and hence disabled + - wrapcheck # Checks that errors returned from external packages are wrapped fast: true # output configuration options diff --git a/datapoint/cast_test.go b/datapoint/cast_test.go index 4819c4c..81a684f 100644 --- a/datapoint/cast_test.go +++ b/datapoint/cast_test.go @@ -159,6 +159,7 @@ func TestCastMetricValueWithBool(t *testing.T) { } } +//nolint:dupl func TestCastFloatValue(t *testing.T) { type args struct { value interface{} @@ -192,6 +193,7 @@ func TestCastFloatValue(t *testing.T) { } } +//nolint:dupl func TestCastUIntegerValue(t *testing.T) { type args struct { value interface{} @@ -225,6 +227,7 @@ func TestCastUIntegerValue(t *testing.T) { } } +//nolint:dupl func TestCastIntegerValue(t *testing.T) { type args struct { value interface{} diff --git a/datapoint/dpsink/counter.go b/datapoint/dpsink/counter.go index 39d863e..b8ad6b4 100644 --- a/datapoint/dpsink/counter.go +++ b/datapoint/dpsink/counter.go @@ -2,6 +2,7 @@ package dpsink import ( "context" + goerrors "errors" "sync/atomic" "time" @@ -67,6 +68,7 @@ func (c *Counter) logErrMsg(ctx context.Context, err error, msg string) { } // AddDatapoints will send points to the next sink and track points send to the next sink +//nolint:dupl func (c *Counter) AddDatapoints(ctx context.Context, points []*datapoint.Datapoint, next Sink) error { atomic.AddInt64(&c.TotalDatapoints, int64(len(points))) atomic.AddInt64(&c.TotalProcessCalls, 1) @@ -91,6 +93,7 @@ func (c *Counter) logger() log.Logger { } // AddEvents will send events to the next sink and track events sent to the next sink +//nolint:dupl func (c *Counter) AddEvents(ctx context.Context, events []*event.Event, next Sink) error { atomic.AddInt64(&c.TotalEvents, int64(len(events))) atomic.AddInt64(&c.TotalProcessCalls, 1) @@ -108,6 +111,7 @@ func (c *Counter) AddEvents(ctx context.Context, events []*event.Event, next Sin } // AddSpans will send spans to the next sink and track spans sent to the next sink +//nolint:dupl func (c *Counter) AddSpans(ctx context.Context, spans []*trace.Span, next trace.Sink) error { atomic.AddInt64(&c.TotalSpans, int64(len(spans))) atomic.AddInt64(&c.TotalProcessCalls, 1) @@ -118,7 +122,8 @@ func (c *Counter) AddSpans(ctx context.Context, spans []*trace.Span, next trace. atomic.AddInt64(&c.CallsInFlight, -1) if err != nil && spanfilter.IsInvalid(err) { atomic.AddInt64(&c.TotalProcessErrors, 1) - if m, ok := err.(*spanfilter.Map); ok { + var m *spanfilter.Map + if goerrors.As(err, &m) { atomic.AddInt64(&c.ProcessErrorSpans, int64(len(m.Invalid))) } else { atomic.AddInt64(&c.ProcessErrorSpans, int64(len(spans))) diff --git a/datapoint/dptest/basicsink_test.go b/datapoint/dptest/basicsink_test.go index aedd8a9..19730a7 100644 --- a/datapoint/dptest/basicsink_test.go +++ b/datapoint/dptest/basicsink_test.go @@ -87,6 +87,7 @@ func TestContextErrorTrace(t *testing.T) { <-progressChan } +//nolint:dupl func TestNext(t *testing.T) { ctx := context.Background() b := NewBasicSink() @@ -105,6 +106,7 @@ func TestNext(t *testing.T) { }) } +//nolint:dupl func TestNextEvent(t *testing.T) { ctx := context.Background() b := NewBasicSink() @@ -123,6 +125,7 @@ func TestNextEvent(t *testing.T) { }) } +//nolint:dupl func TestNextSpan(t *testing.T) { ctx := context.Background() b := NewBasicSink() diff --git a/datapoint/dptest/generator.go b/datapoint/dptest/generator.go index c03ce83..8a2f004 100644 --- a/datapoint/dptest/generator.go +++ b/datapoint/dptest/generator.go @@ -25,6 +25,7 @@ type DatapointSource struct { var globalSource DatapointSource +//nolint:gochecknoinits func init() { globalSource.Metric = "random" globalSource.Dims = map[string]string{"source": "randtest"} diff --git a/disco/disco.go b/disco/disco.go index 7614a3d..83439cb 100644 --- a/disco/disco.go +++ b/disco/disco.go @@ -6,6 +6,7 @@ import ( "crypto/rand" "encoding/binary" "encoding/json" + goerrors "errors" "expvar" "fmt" "io" @@ -130,32 +131,34 @@ var DefaultConfig = &Config{ } // New creates a disco discovery/publishing service -func New(zkConnCreator ZkConnCreator, publishAddress string, config *Config) (*Disco, error) { - conf := pointer.FillDefaultFrom(config, DefaultConfig).(*Config) - var GUID [16]byte - _, err := io.ReadFull(conf.RandomSource, GUID[:16]) - if err != nil { - return nil, errors.Annotate(err, "cannot create random GUID") - } - - d := &Disco{ - zkConnCreator: zkConnCreator, - myAdvertisedServices: make(map[string]ServiceInstance), - myEphemeralNodes: make(map[string][]byte), - GUIDbytes: GUID, - jsonMarshal: json.Marshal, - publishAddress: publishAddress, - shouldQuit: make(chan struct{}), - eventLoopDone: make(chan struct{}), - watchedServices: make(map[string]*Service), - manualEvents: make(chan zk.Event), - } - d.stateLog = log.NewContext(conf.Logger).With(logkey.GUID, d.GUID()) - d.zkConn, d.eventChan, err = zkConnCreator.Connect() - if err != nil { - return nil, errors.Annotate(err, "cannot create first zk connection") +func New(zkConnCreator ZkConnCreator, publishAddress string, config *Config) (d *Disco, err error) { + conf, ok := pointer.FillDefaultFrom(config, DefaultConfig).(*Config) + if ok { + var GUID [16]byte + _, err = io.ReadFull(conf.RandomSource, GUID[:16]) + if err != nil { + return nil, errors.Annotate(err, "cannot create random GUID") + } + + d = &Disco{ + zkConnCreator: zkConnCreator, + myAdvertisedServices: make(map[string]ServiceInstance), + myEphemeralNodes: make(map[string][]byte), + GUIDbytes: GUID, + jsonMarshal: json.Marshal, + publishAddress: publishAddress, + shouldQuit: make(chan struct{}), + eventLoopDone: make(chan struct{}), + watchedServices: make(map[string]*Service), + manualEvents: make(chan zk.Event), + } + d.stateLog = log.NewContext(conf.Logger).With(logkey.GUID, d.GUID()) + d.zkConn, d.eventChan, err = zkConnCreator.Connect() + if err != nil { + return nil, errors.Annotate(err, "cannot create first zk connection") + } + go d.eventLoop() } - go d.eventLoop() return d, nil } @@ -339,7 +342,7 @@ func (d *Disco) createInZk(deleteIfExists bool, pathDirectory string, instanceBy func (d *Disco) advertiseInZK(deleteIfExists bool, serviceName string, instanceData ServiceInstance) error { // Need to create service root node _, err := d.zkConn.Create(fmt.Sprintf("/%s", serviceName), []byte{}, 0, zk.WorldACL(zk.PermAll)) - if err != nil && errors.Tail(err) != zk.ErrNodeExists { + if err != nil && !goerrors.Is(errors.Tail(err), zk.ErrNodeExists) { return errors.Annotatef(err, "unhandled ZK error on Create of %s", serviceName) } instanceBytes, err := d.jsonMarshal(instanceData) @@ -409,7 +412,7 @@ func (d *Disco) CreatePersistentEphemeralNode(path string, payload []byte) (err d.watchedMutex.Lock() defer d.watchedMutex.Unlock() _, err = d.zkConn.Create(fmt.Sprintf("/%s", path), []byte{}, 0, zk.WorldACL(zk.PermAll)) - if err != nil && errors.Tail(err) != zk.ErrNodeExists { + if err != nil && !goerrors.Is(errors.Tail(err), zk.ErrNodeExists) { return errors.Annotatef(err, "unhandled ZK error on Create of %s", path) } if err = d.createInZk(true, path, payload); err != nil { @@ -469,11 +472,13 @@ func (x serviceInstanceList) Swap(i, j int) { } // ServiceInstances that represent instances of this service in your system -func (s *Service) ServiceInstances() []ServiceInstance { - svcs := s.services.Load().([]ServiceInstance) - ret := make([]ServiceInstance, len(svcs)) - copy(ret, svcs) - sort.Sort(serviceInstanceList(ret)) +func (s *Service) ServiceInstances() (ret []ServiceInstance) { + svcs, ok := s.services.Load().([]ServiceInstance) + if ok { + ret = make([]ServiceInstance, len(svcs)) + copy(ret, svcs) + sort.Sort(serviceInstanceList(ret)) + } return ret } @@ -549,6 +554,7 @@ func childrenServices(logger log.Logger, serviceName string, children []string, return ret, errors.NewMultiErr(allErrors) } +//nolint:ifshort func (s *Service) refresh(zkConn ZkConn) error { if atomic.LoadInt64(&s.ignoreUpdates) != 0 { s.stateLog.Log("refresh flag set. Ignoring refresh") @@ -557,12 +563,12 @@ func (s *Service) refresh(zkConn ZkConn) error { s.stateLog.Log(log.Msg, fmt.Sprintf("refresh called for %s service", s.name)) oldHash := s.byteHashes() children, _, _, err := zkConn.ChildrenW(fmt.Sprintf("/%s", s.name)) - if err != nil && errors.Tail(err) != zk.ErrNoNode { + if err != nil && !goerrors.Is(errors.Tail(err), zk.ErrNoNode) { s.stateLog.Log(log.Err, err, "Error getting children") return errors.Annotatef(err, "unexpected zk error on childrenw") } - if errors.Tail(err) == zk.ErrNoNode { + if goerrors.Is(errors.Tail(err), zk.ErrNoNode) { exists, _, _, err := zkConn.ExistsW(fmt.Sprintf("/%s", s.name)) if exists || err != nil { s.stateLog.Log(log.Err, err, "Unable to register exists watch!") diff --git a/disco/disco_test.go b/disco/disco_test.go index f64bf7a..19f138e 100644 --- a/disco/disco_test.go +++ b/disco/disco_test.go @@ -2,6 +2,7 @@ package disco import ( "bytes" + errors2 "errors" "fmt" "runtime" "sort" @@ -65,6 +66,7 @@ func TestDupAdvertise(t *testing.T) { } func testDupAdvertise(t *testing.T, z zktest.ZkConnSupported, ch <-chan zk.Event) { + t.Helper() _, err := z.Create("/test", []byte(""), 0, zk.WorldACL(zk.PermAll)) log.IfErr(log.Panic, err) @@ -168,8 +170,7 @@ func TestBadRefresh(t *testing.T) { } for { runtime.Gosched() - err1 := errors.Tail(s.refresh(z)) - if err1 == badForce { + if err1 := errors.Tail(s.refresh(z)); errors2.Is(err1, badForce) { break } } @@ -279,6 +280,7 @@ func TestAdvertise(t *testing.T) { } func testAdvertise(t *testing.T, zkConnFunc ZkConnCreatorFunc, zkConnFunc2 ZkConnCreatorFunc) { + t.Helper() d1, err := New(zkConnFunc, "TestAdvertise1", nil) require.NoError(t, err) @@ -328,6 +330,7 @@ func TestServices(t *testing.T) { } func testServices(t *testing.T, z1 zktest.ZkConnSupported, ch <-chan zk.Event, z2 zktest.ZkConnSupported) { + t.Helper() zkConnFunc := ZkConnCreatorFunc(func() (ZkConn, <-chan zk.Event, error) { return z1, ch, nil }) diff --git a/distconf/bool.go b/distconf/bool.go index 7bc2c97..8d89c66 100644 --- a/distconf/bool.go +++ b/distconf/bool.go @@ -25,6 +25,7 @@ type Bool struct { } // Update the contents of Bool to the new value +//nolint:ifshort func (s *boolConf) Update(newValue []byte) error { s.mutex.Lock() defer s.mutex.Unlock() diff --git a/distconf/distconf.go b/distconf/distconf.go index 6d02ccf..5acf594 100644 --- a/distconf/distconf.go +++ b/distconf/distconf.go @@ -265,8 +265,7 @@ func (c *Distconf) refresh(key string, configVar configVariable) bool { } } - e := configVar.Update(nil) - if e != nil { + if e := configVar.Update(nil); e != nil { c.Logger.Log(log.Err, e, "Unable to set bytes to nil/clear") } diff --git a/distconf/distconf_test.go b/distconf/distconf_test.go index c12fc19..66b9606 100644 --- a/distconf/distconf_test.go +++ b/distconf/distconf_test.go @@ -52,6 +52,7 @@ func makeConf() (ReaderWriter, *Distconf) { return memConf, conf } +//nolint:dupl func TestDistconfInt(t *testing.T) { memConf, conf := makeConf() defer conf.Close() @@ -87,6 +88,7 @@ func TestDistconfInt(t *testing.T) { assert.Contains(t, conf.Var().String(), "testval") } +//nolint:dupl func TestDistconfFloat(t *testing.T) { memConf, conf := makeConf() defer conf.Close() @@ -235,6 +237,7 @@ func TestDistconfErrorBackings(t *testing.T) { } func testInfo(t *testing.T, dat map[string]DistInfo, key string, val interface{}, dtype DistType) { + t.Helper() v, ok := dat[key] assert.True(t, ok) assert.NotEqual(t, v.Line, 0) diff --git a/distconf/duration.go b/distconf/duration.go index 8635eff..4a7e23b 100644 --- a/distconf/duration.go +++ b/distconf/duration.go @@ -33,6 +33,7 @@ func (s *Duration) Get() time.Duration { } // Update the contents of Duration to the new value +//nolint:ifshort func (s *durationConf) Update(newValue []byte) error { s.mutex.Lock() defer s.mutex.Unlock() diff --git a/distconf/float.go b/distconf/float.go index b925ab5..264fc83 100644 --- a/distconf/float.go +++ b/distconf/float.go @@ -32,6 +32,7 @@ func (c *Float) Get() float64 { } // Update the content of this config variable to newValue. +//nolint:ifshort func (c *floatConf) Update(newValue []byte) error { c.mutex.Lock() defer c.mutex.Unlock() diff --git a/distconf/int.go b/distconf/int.go index 520e170..790f0ee 100644 --- a/distconf/int.go +++ b/distconf/int.go @@ -30,6 +30,7 @@ func (c *Int) Get() int64 { } // Update the content of this config variable to newValue. +//nolint:ifshort func (c *intConf) Update(newValue []byte) error { c.mutex.Lock() defer c.mutex.Unlock() diff --git a/distconf/str.go b/distconf/str.go index fb74c08..a72fabb 100644 --- a/distconf/str.go +++ b/distconf/str.go @@ -26,7 +26,7 @@ type Str struct { func (s *strConf) Update(newValue []byte) error { s.mutex.Lock() defer s.mutex.Unlock() - oldValue := s.currentVal.Load().(string) + oldValue, _ := s.currentVal.Load().(string) if newValue == nil { s.currentVal.Store(s.defaultVal) } else { diff --git a/distconf/zk.go b/distconf/zk.go index 8a66ed7..1dde71a 100644 --- a/distconf/zk.go +++ b/distconf/zk.go @@ -2,6 +2,7 @@ package distconf import ( + errors2 "errors" "fmt" "sync" "time" @@ -113,7 +114,7 @@ func (back *zkConfig) Get(key string) ([]byte, error) { pathToFetch := back.configPath(key) bytes, _, _, err := back.conn.GetW(pathToFetch) if err != nil { - if err == zk.ErrNoNode { + if errors2.Is(err, zk.ErrNoNode) { return nil, nil } return nil, errors.Annotatef(err, "cannot load zk node %s", pathToFetch) @@ -147,8 +148,7 @@ func (back *zkConfig) Write(key string, value []byte) error { func (back *zkConfig) Watch(key string, callback backingCallbackFunction) error { back.rootLogger.Log(logkey.ZkMethod, "watch", logkey.ZkPath, key) path := back.configPath(key) - _, _, _, err := back.conn.ExistsW(path) - if err != nil { + if _, _, _, err := back.conn.ExistsW(path); err != nil { return errors.Annotatef(err, "cannot watch path %s", path) } back.callbacks.add(path, callback) @@ -240,8 +240,7 @@ func (back *zkConfig) setRefreshDelay(refreshDelay time.Duration) { func (back *zkConfig) reregisterWatch(path string, logger log.Logger) error { logger = log.NewContext(logger).With(logkey.ZkPath, path) logger.Log("Reregistering watch") - _, _, _, err := back.conn.ExistsW(path) - if err != nil { + if _, _, _, err := back.conn.ExistsW(path); err != nil { logger.Log(log.Err, err, "Unable to reregistering watch") return errors.Annotatef(err, "unable to reregister watch for node %s", path) } @@ -260,22 +259,25 @@ var DefaultZkConfig = &ZkConfig{ // Zk creates a zookeeper readable backing func Zk(zkConnector ZkConnector, conf *ZkConfig) (ReaderWriter, error) { - conf = pointer.FillDefaultFrom(conf, DefaultZkConfig).(*ZkConfig) - ret := &zkConfig{ - rootLogger: log.NewContext(conf.Logger).With(logkey.DistconfBacking, "zk"), - shouldQuit: make(chan struct{}), - callbacks: callbackMap{ - callbacks: make(map[string][]backingCallbackFunction), - }, - refreshDelay: atomicDuration{ - refreshRetryDelay: time.Millisecond * 500, - }, - } - var err error - ret.conn, ret.eventChan, err = zkConnector.Connect() - if err != nil { - return nil, errors.Annotate(err, "cannot create zk connection") + zkCfg, ok := pointer.FillDefaultFrom(conf, DefaultZkConfig).(*ZkConfig) + var ret *zkConfig + if ok { + ret = &zkConfig{ + rootLogger: log.NewContext(zkCfg.Logger).With(logkey.DistconfBacking, "zk"), + shouldQuit: make(chan struct{}), + callbacks: callbackMap{ + callbacks: make(map[string][]backingCallbackFunction), + }, + refreshDelay: atomicDuration{ + refreshRetryDelay: time.Millisecond * 500, + }, + } + var err error + ret.conn, ret.eventChan, err = zkConnector.Connect() + if err != nil { + return nil, errors.Annotate(err, "cannot create zk connection") + } + go ret.drainEventChan(zkCfg.Logger) } - go ret.drainEventChan(conf.Logger) return ret, nil } diff --git a/env/env_test.go b/env/env_test.go index 74a52b2..16f67ef 100644 --- a/env/env_test.go +++ b/env/env_test.go @@ -60,6 +60,7 @@ func TestGetDurationEnvVar(t *testing.T) { }) } +//nolint:dupl func TestGetUint64EnvVar(t *testing.T) { convey.Convey("getUint64EnvVar", t, func() { convey.Convey("should return the value if the environment variable is set", func() { @@ -84,6 +85,7 @@ func TestGetUint64EnvVar(t *testing.T) { }) } +//nolint:dupl func TestGetUintEnvVar(t *testing.T) { convey.Convey("getUintEnvVar", t, func() { convey.Convey("should return the value if the environment variable is set", func() { diff --git a/errors/analyze.go b/errors/analyze.go index 23e987d..a8fff56 100644 --- a/errors/analyze.go +++ b/errors/analyze.go @@ -2,19 +2,25 @@ package errors import ( "bytes" + "errors" ) // Tail of the error chain: at the end func Tail(err error) error { - if tail, ok := err.(errLinkedList); ok { + var ( + tail errLinkedList + causable causableError + hasTail hasInner + ) + if errors.As(err, &tail) { return tail.Tail() } - if causable, ok := err.(causableError); ok { + if errors.As(err, &causable) { return causable.Cause() } - if hasTail, ok := err.(hasInner); ok { + if errors.As(err, &hasTail) { i := hasTail.GetInner() - if i == err || i == nil { + if errors.Is(err, i) || i == nil { return err } return Tail(i) @@ -25,13 +31,18 @@ func Tail(err error) error { // Next error just below this one, or nil if there is no next error. Note this may be an error created // for you if you used annotations. As a user, you probably don't want to use this. func Next(err error) error { - if tail, ok := err.(errLinkedList); ok { + var ( + tail errLinkedList + u hasUnderline + i hasInner + ) + if errors.As(err, &tail) { return tail.Next() } - if u, ok := err.(hasUnderline); ok { + if errors.As(err, &u) { return u.Underlying() } - if i, ok := err.(hasInner); ok { + if errors.As(err, &i) { return i.GetInner() } return nil @@ -39,10 +50,14 @@ func Next(err error) error { // Message is the error string at the Head of the linked list func Message(err error) string { - if tail, ok := err.(errLinkedList); ok { + var ( + tail errLinkedList + hasMsg hasMessage + ) + if errors.As(err, &tail) { return tail.Head().Error() } - if hasMsg, ok := err.(hasMessage); ok { + if errors.As(err, &hasMsg) { return hasMsg.Message() } return err.Error() diff --git a/errors/chain.go b/errors/chain.go index 6c0b1b7..0a9e62d 100644 --- a/errors/chain.go +++ b/errors/chain.go @@ -1,7 +1,11 @@ package errors -// ErrorChain is a linked list of error pointers that point to a parent above and a child below. -type ErrorChain struct { +// ErrorChain is type cast to ChainError for backwards compatibility +// This will be deprecated in future major release +type ErrorChain = ChainError + +// ChainError is a linked list of error pointers that point to a parent above and a child below. +type ChainError struct { // Head of the linked list head error // Next node in the linked list @@ -11,22 +15,22 @@ type ErrorChain struct { } // Tail is the end of the linked list -func (e *ErrorChain) Tail() error { +func (e *ChainError) Tail() error { return e.tail } // Head is the start of the linked list -func (e *ErrorChain) Head() error { +func (e *ChainError) Head() error { return e.head } // Next is the next node in the linked list -func (e *ErrorChain) Next() error { +func (e *ChainError) Next() error { return e.next } // Error returns the error string of the tail of the linked list -func (e *ErrorChain) Error() string { +func (e *ChainError) Error() string { return Cause(e).Error() } @@ -36,4 +40,4 @@ type errLinkedList interface { Tail() error } -var _ errLinkedList = &ErrorChain{} +var _ errLinkedList = &ChainError{} diff --git a/errors/compatibility.go b/errors/compatibility.go index be07162..00c52f9 100644 --- a/errors/compatibility.go +++ b/errors/compatibility.go @@ -18,33 +18,33 @@ type hasMessage interface { } var ( - _ causableError = &ErrorChain{} - _ hasUnderline = &ErrorChain{} - _ hasMessage = &ErrorChain{} + _ causableError = &ChainError{} + _ hasUnderline = &ChainError{} + _ hasMessage = &ChainError{} ) // Cause lets me simulate errgo -func (e *ErrorChain) Cause() error { +func (e *ChainError) Cause() error { return e.Tail() } // Message lets me simulate errgo -func (e *ErrorChain) Message() string { +func (e *ChainError) Message() string { return e.Head().Error() } // GetMessage is used by dropbox -func (e *ErrorChain) GetMessage() string { +func (e *ChainError) GetMessage() string { return e.Head().Error() } // GetInner is used by dropbox -func (e *ErrorChain) GetInner() error { +func (e *ChainError) GetInner() error { return e.Head() } // Underlying lets me simulate errgo/facebook -func (e *ErrorChain) Underlying() error { +func (e *ChainError) Underlying() error { return e.Next() } diff --git a/errors/compatibility_test.go b/errors/compatibility_test.go index c056950..1e66a08 100644 --- a/errors/compatibility_test.go +++ b/errors/compatibility_test.go @@ -1,6 +1,7 @@ package errors import ( + "errors" "testing" dropboxerrors "github.com/dropbox/godropbox/errors" @@ -15,7 +16,8 @@ func TestGoDropbox(t *testing.T) { So(Tail(root), ShouldEqual, root) dropboxWrap := dropboxerrors.Wrap(root, "Wrapped error") So(Tail(dropboxWrap), ShouldEqual, root) - myAnnotation := Annotate(dropboxWrap, "I have annotated dropbox error").(*ErrorChain) + var myAnnotation *ChainError + So(errors.As(Annotate(dropboxWrap, "I have annotated dropbox error"), &myAnnotation), ShouldBeTrue) So(Tail(myAnnotation), ShouldEqual, root) So(Cause(myAnnotation), ShouldEqual, root) So(Details(myAnnotation), ShouldContainSubstring, "dropbox root error") diff --git a/errors/constructors.go b/errors/constructors.go index e00b3c3..5f0fa41 100644 --- a/errors/constructors.go +++ b/errors/constructors.go @@ -5,12 +5,12 @@ import ( "fmt" ) -// New error. Note returns error rather than *ErrorChain so that it matches errors.New signature +// New error. Note returns error rather than *ChainError so that it matches errors.New signature func New(msg string) error { return errors.New(msg) } -// Errorf is fmt.Errorf. Note returns error rather than *ErrorChain so that it matches fmt.Errorf signature +// Errorf is fmt.Errorf. Note returns error rather than *ChainError so that it matches fmt.Errorf signature func Errorf(msg string, args ...interface{}) error { return fmt.Errorf(msg, args...) } @@ -42,7 +42,7 @@ func Wrap(head error, next error) error { return head } tail := Tail(next) - return &ErrorChain{ + return &ChainError{ head: head, next: next, tail: tail, diff --git a/errors/constructors_test.go b/errors/constructors_test.go index 869cb67..a5fc267 100644 --- a/errors/constructors_test.go +++ b/errors/constructors_test.go @@ -30,7 +30,8 @@ func TestBaseError(t *testing.T) { Err: os.ErrNotExist, } So(Wrap(baseErr, nil), ShouldEqual, baseErr) - wrappedErr := Wrap(baseErr, pathErr).(*ErrorChain) + var wrappedErr *ChainError + So(errors.As(Wrap(baseErr, pathErr), &wrappedErr), ShouldBeTrue) So(Tail(wrappedErr), ShouldEqual, pathErr) So(Next(wrappedErr), ShouldEqual, pathErr) So(wrappedErr.Error(), ShouldContainSubstring, "file does not exist") diff --git a/errors/log.go b/errors/log.go index e0a53ac..c524458 100644 --- a/errors/log.go +++ b/errors/log.go @@ -24,8 +24,7 @@ func LogIfErr(err error, l Loggable, msg string, args ...interface{}) { // // Do something with f // } func DeferLogIfErr(errCallback func() error, l Loggable, msg string, args ...interface{}) { - err := errCallback() - if err != nil { + if err := errCallback(); err != nil { l.Printf("%s: %s", err.Error(), fmt.Sprintf(msg, args...)) } } diff --git a/errors/multi.go b/errors/multi.go index 7a27356..8b28133 100644 --- a/errors/multi.go +++ b/errors/multi.go @@ -2,12 +2,16 @@ package errors import "strings" -// MultiErr wraps multiple errors into one error string -type MultiErr struct { +// MultiErr type cast MultiError for backward compatibility +// This will be deprecated in future major release +type MultiErr = MultiError + +// MultiError wraps multiple errors into one error string +type MultiError struct { errs []error } -func (e *MultiErr) Error() string { +func (e *MultiError) Error() string { r := make([]string, 0, len(e.errs)) for _, err := range e.errs { r = append(r, err.Error()) @@ -15,10 +19,10 @@ func (e *MultiErr) Error() string { return strings.Join(r, " | ") } -var _ error = &MultiErr{} +var _ error = &MultiError{} // NewMultiErr will return nil if there are no valid errors in errs, will return the exact, single error -// if errs only contains a single error, and will otherwise return an instance of MultiErr that wraps all +// if errs only contains a single error, and will otherwise return an instance of MultiError that wraps all // the errors at once. func NewMultiErr(errs []error) error { retErrs := make([]error, 0, len(errs)) @@ -33,7 +37,7 @@ func NewMultiErr(errs []error) error { if len(retErrs) == 1 { return retErrs[0] } - return &MultiErr{ + return &MultiError{ errs: retErrs, } } diff --git a/explorable/explorable_test.go b/explorable/explorable_test.go index 2ac7512..e7801ab 100644 --- a/explorable/explorable_test.go +++ b/explorable/explorable_test.go @@ -158,6 +158,7 @@ func TestMap(t *testing.T) { } func verify(t *testing.T, v reflect.Value, desc string, vals []string) { + t.Helper() for i := 0; i < len(vals)-1; i++ { o := ExploreObject(v, vals[0:i]) assert.Contains(t, o.Children, vals[i]) diff --git a/expvar2/handler_test.go b/expvar2/handler_test.go index bfe0acf..6c5d8f7 100644 --- a/expvar2/handler_test.go +++ b/expvar2/handler_test.go @@ -15,6 +15,7 @@ import ( var toFind = "asdfasdfdsa" +//nolint:gochecknoinits func init() { expvar.NewString("expvar2.TestExpvarHandler.a").Set("abc") expvar.NewString("expvar2.TestExpvarHandler.b").Set(toFind) diff --git a/httpdebug/pprof15.go b/httpdebug/pprof15.go index cac7b10..bcb17d9 100644 --- a/httpdebug/pprof15.go +++ b/httpdebug/pprof15.go @@ -1,3 +1,4 @@ +//go:build go1.5 // +build go1.5 package httpdebug diff --git a/httpdebug/server.go b/httpdebug/server.go index c2a67d2..cef6f64 100644 --- a/httpdebug/server.go +++ b/httpdebug/server.go @@ -38,7 +38,7 @@ var LogKeyHTTPClass = log.Key("http_class") // New creates a new debug server func New(conf *Config) *Server { - conf = pointer.FillDefaultFrom(conf, DefaultConfig).(*Config) + conf, _ = pointer.FillDefaultFrom(conf, DefaultConfig).(*Config) m := http.NewServeMux() s := &Server{ Server: http.Server{ diff --git a/log/dynamic_test.go b/log/dynamic_test.go index b698470..13bc08a 100644 --- a/log/dynamic_test.go +++ b/log/dynamic_test.go @@ -17,6 +17,7 @@ func BenchmarkValueBindingTimestampCount(b *testing.B) { } func benchmarkValueBindingTimestamp(b *testing.B, logger Logger) { + b.Helper() lc := NewContext(logger).With("ts", DefaultTimestamp) b.ReportAllocs() b.ResetTimer() @@ -34,6 +35,7 @@ func BenchmarkValueBindingCallerCount(b *testing.B) { } func benchmarkValueBindingCaller(b *testing.B, logger Logger) { + b.Helper() lc := NewContext(logger).With("caller", DefaultCaller) b.ReportAllocs() b.ResetTimer() diff --git a/log/hierarchy.go b/log/hierarchy.go index 31e9605..b2bc7fb 100644 --- a/log/hierarchy.go +++ b/log/hierarchy.go @@ -11,6 +11,7 @@ var DefaultLogger = NewHierarchy(NewLogfmtLogger(os.Stderr, Discard)) // LoggingEnv is the env variable that if exported to /dev/null can disable the default logger const LoggingEnv = "GOLIB_LOG" +//nolint:gochecknoinits func init() { DefaultLogger.setupFromEnv(os.Getenv) } diff --git a/log/json.go b/log/json.go index 89064a5..9280d7e 100644 --- a/log/json.go +++ b/log/json.go @@ -58,7 +58,7 @@ func mapFromKeyvals(missingValueKey Key, keyvals ...interface{}) map[string]inte func mapKey(k interface{}) (s string) { defer func() { if panicVal := recover(); panicVal != nil { - s = nilCheck(k, panicVal, "NULL").(string) + s, _ = nilCheck(k, panicVal, "NULL").(string) } }() switch x := k.(type) { diff --git a/log/kitbenchmark_test.go b/log/kitbenchmark_test.go index 0467104..91f7bdc 100644 --- a/log/kitbenchmark_test.go +++ b/log/kitbenchmark_test.go @@ -7,6 +7,7 @@ import ( ) func benchmarkRunner(b *testing.B, logger log.Logger, f func(log.Logger)) { + b.Helper() lc := log.NewContext(logger).With("common_key", "common_value") b.ReportAllocs() b.ResetTimer() diff --git a/log/kitlog_test.go b/log/kitlog_test.go index 9ff8b57..0824656 100644 --- a/log/kitlog_test.go +++ b/log/kitlog_test.go @@ -8,6 +8,7 @@ import ( "github.com/go-stack/stack" "github.com/signalfx/golib/v3/log" + "github.com/stretchr/testify/assert" ) type panicLogger struct{} @@ -101,8 +102,8 @@ func TestContextStackDepth(t *testing.T) { // Call through interface to get baseline. iface.Log("k", "v") - want := output[1].(int) - + want, ok := output[1].(int) + assert.True(t, ok) for len(output) < 10 { concrete.Log("k", "v") if have := output[1]; have != want { @@ -138,8 +139,10 @@ func TestWithConcurrent(t *testing.T) { // This logger extracts a goroutine id from the last value field and // increments the referenced bucket. logger := log.LoggerFunc(func(kv ...interface{}) { - goroutine := kv[len(kv)-1].(int) - counts[goroutine]++ + goroutine, ok := kv[len(kv)-1].(int) + if ok { + counts[goroutine]++ + } }) // With must be careful about handling slices that can grow without diff --git a/log/log.go b/log/log.go index 2322662..520ec86 100644 --- a/log/log.go +++ b/log/log.go @@ -113,7 +113,7 @@ func (l *Context) With(keyvals ...interface{}) *Context { } } -// WithPrefix is like With but adds keyvalus to the beginning of the eventual log statement +// WithPrefix is like With but adds key/values to the beginning of the eventual log statement func (l *Context) WithPrefix(keyvals ...interface{}) *Context { if len(keyvals)%2 != 0 { panic("Programmer error. Please call log.Context.WithPrefix() only with an even number of arguments.") diff --git a/log/log_test.go b/log/log_test.go index d9e0d06..10177a8 100644 --- a/log/log_test.go +++ b/log/log_test.go @@ -22,8 +22,9 @@ func TestWithConcurrentInternal(t *testing.T) { // This logger extracts a goroutine id from the last value field and // increments the referenced bucket. logger := LoggerFunc(func(kv ...interface{}) { - goroutine := kv[len(kv)-1].(int) - counts[goroutine]++ + if goroutine, ok := kv[len(kv)-1].(int); ok { + counts[goroutine]++ + } if len(kv) != 10 { panic(kv) } @@ -88,6 +89,7 @@ func toStr(in []interface{}) []string { return ret } +//nolint:staticcheck func TestLoggingBasics(t *testing.T) { Convey("A nil logger should work as expected", t, func() { var l *Context @@ -379,6 +381,8 @@ func TestIfErr(t *testing.T) { } } +var errTest = errors.New("nope") + func TestIfErrAndReturn(t *testing.T) { b := &bytes.Buffer{} l := NewLogfmtLogger(b, Panic) @@ -388,8 +392,8 @@ func TestIfErrAndReturn(t *testing.T) { if b.String() != "" { t.Error("Expected empty string") } - testErr := errors.New("nope") - if err := IfErrAndReturn(l, testErr); err != testErr { + + if err := IfErrAndReturn(l, errTest); !errors.Is(err, errTest) { t.Error("Expected error to equal ") } if b.String() == "" { @@ -406,8 +410,8 @@ func TestIfErrWithKeysAndReturn(t *testing.T) { if b.String() != "" { t.Error("Expected empty string") } - testErr := errors.New("nope") - if err := IfErrWithKeysAndReturn(l, testErr, Err, "test message"); err != testErr { + + if err := IfErrWithKeysAndReturn(l, errTest, Err, "test message"); !errors.Is(err, errTest) { t.Error("Expected error to equal ") } if b.String() == "" { @@ -436,6 +440,7 @@ func TestChannelLoggerRace(t *testing.T) { } func fullyVerifyLogger(t *testing.T, factory func() (Logger, *bytes.Buffer)) { + t.Helper() Convey("When verifying a logger", t, func() { Convey("Racing should be ok", func() { l, _ := factory() diff --git a/log/logfmt_test.go b/log/logfmt_test.go index d1197fb..d6085aa 100644 --- a/log/logfmt_test.go +++ b/log/logfmt_test.go @@ -9,13 +9,13 @@ import ( . "github.com/smartystreets/goconvey/convey" ) -type errMarshall struct{} +type marshallError struct{} -func (e *errMarshall) MarshalText() (text []byte, err error) { - return nil, &errMarshall{} +func (e *marshallError) MarshalText() (text []byte, err error) { + return nil, &marshallError{} } -func (e *errMarshall) Error() string { +func (e *marshallError) Error() string { return "I am an error" } @@ -41,9 +41,9 @@ func TestNewLogfmtLogger(t *testing.T) { l.Log("name", "john doe") So(strings.TrimSpace(buf.String()), ShouldResemble, `name="john doe"`) }) - Convey("should forward marshall errors", func() { + Convey("should forward marshallError errors", func() { So(func() { - l.Log(&errMarshall{}, &errMarshall{}) + l.Log(&marshallError{}, &marshallError{}) }, ShouldPanic) }) Convey("should forward writer errors", func() { diff --git a/maestro/maestro_test.go b/maestro/maestro_test.go index 7c7234f..0c11bd3 100644 --- a/maestro/maestro_test.go +++ b/maestro/maestro_test.go @@ -18,11 +18,13 @@ func (e envMap) get(key string) string { } func asserErrorEq(t *testing.T, e error, f func() (string, error)) { + t.Helper() _, err := f() assert.Equal(t, e, err) } func assertStrEq(t *testing.T, s string, f func() (string, error)) { + t.Helper() r, err := f() assert.NoError(t, err) assert.Equal(t, s, r) @@ -31,6 +33,7 @@ func assertStrEq(t *testing.T, s string, f func() (string, error)) { var i = 0 func testPath(t *testing.T, e envMap, err error, key string, f func() (string, error)) { + t.Helper() asserErrorEq(t, err, f) v := fmt.Sprintf("%dhello", i) e[key] = v diff --git a/metadata/hostmetadata/host-linux_test.go b/metadata/hostmetadata/host-linux_test.go index be6ae28..d8348e0 100644 --- a/metadata/hostmetadata/host-linux_test.go +++ b/metadata/hostmetadata/host-linux_test.go @@ -1,3 +1,4 @@ +//go:build linux // +build linux package hostmetadata diff --git a/metadata/hostmetadata/host-not-linux.go b/metadata/hostmetadata/host-not-linux.go index 9643d2a..ca97f65 100644 --- a/metadata/hostmetadata/host-not-linux.go +++ b/metadata/hostmetadata/host-not-linux.go @@ -1,3 +1,4 @@ +//go:build !linux // +build !linux package hostmetadata diff --git a/metadata/hostmetadata/host_linux.go b/metadata/hostmetadata/host_linux.go index 5944ef7..cec9f50 100644 --- a/metadata/hostmetadata/host_linux.go +++ b/metadata/hostmetadata/host_linux.go @@ -1,3 +1,4 @@ +//go:build linux // +build linux package hostmetadata diff --git a/nettest/trackingdialer.go b/nettest/trackingdialer.go index 67422c2..65e5c62 100644 --- a/nettest/trackingdialer.go +++ b/nettest/trackingdialer.go @@ -15,7 +15,7 @@ type TrackingDialer struct { // Close all stored connections. Returns an error on the first close that fails func (t *TrackingDialer) Close() error { - errs := make([]error, len(t.Conns)) + errs := make([]error, 0, len(t.Conns)) for len(t.Conns) != 0 { c := t.Conns[0] err := c.Close() diff --git a/pointer/pointer_test.go b/pointer/pointer_test.go index c50c7ca..8050813 100644 --- a/pointer/pointer_test.go +++ b/pointer/pointer_test.go @@ -80,7 +80,8 @@ func TestFillDefaultFrom(t *testing.T) { }, } Convey("should fill", func() { - p := FillDefaultFrom(&defaultPerson).(*Person) + p, ok := FillDefaultFrom(&defaultPerson).(*Person) + So(ok, ShouldBeTrue) So(*p.Age, ShouldEqual, 21) So(*p.Name, ShouldEqual, "john") So(*p.SinceWakeup, ShouldEqual, time.Second) @@ -100,7 +101,8 @@ func TestFillDefaultFrom(t *testing.T) { }) Convey("should be able to override defaults", func() { tkStub := timekeepertest.NewStubClock(time.Now()) - p := FillDefaultFrom(&Person{Age: Int32(22), Tk: tkStub}, &defaultPerson).(*Person) + p, ok := FillDefaultFrom(&Person{Age: Int32(22), Tk: tkStub}, &defaultPerson).(*Person) + So(ok, ShouldBeTrue) So(*p.Age, ShouldEqual, 22) So(*p.Name, ShouldEqual, "john") So(p.Job.Pay(), ShouldEqual, 100) @@ -108,25 +110,29 @@ func TestFillDefaultFrom(t *testing.T) { }) Convey("should allow nil defaults", func() { defaultPerson.Age = nil - p := FillDefaultFrom(&defaultPerson).(*Person) + p, ok := FillDefaultFrom(&defaultPerson).(*Person) + So(ok, ShouldBeTrue) So(p.Age, ShouldBeNil) So(*p.Name, ShouldEqual, "john") }) Convey("should allow nil elements", func() { var nilPerson *Person - p := FillDefaultFrom(nilPerson, &defaultPerson).(*Person) + p, ok := FillDefaultFrom(nilPerson, &defaultPerson).(*Person) + So(ok, ShouldBeTrue) So(*p.Age, ShouldEqual, 21) }) Convey("should allow nil middle elements", func() { var nilPerson *Person - p := FillDefaultFrom(nilPerson, nil, &defaultPerson).(*Person) + p, ok := FillDefaultFrom(nilPerson, nil, &defaultPerson).(*Person) + So(ok, ShouldBeTrue) So(*p.Age, ShouldEqual, 21) }) Convey("should allow chaining", func() { firstDefault := Person{ Name: String("jackie"), } - p := FillDefaultFrom(&firstDefault, &defaultPerson).(*Person) + p, ok := FillDefaultFrom(&firstDefault, &defaultPerson).(*Person) + So(ok, ShouldBeTrue) So(*p.Age, ShouldEqual, 21) So(*p.Name, ShouldEqual, "jackie") Convey("and not modify structs", func() { diff --git a/sfxclient/cumulativebucket_test.go b/sfxclient/cumulativebucket_test.go index 2ea645e..617cd9b 100644 --- a/sfxclient/cumulativebucket_test.go +++ b/sfxclient/cumulativebucket_test.go @@ -99,6 +99,7 @@ func BenchmarkCumulativeBucket10(b *testing.B) { } func benchCB(b *testing.B, numGoroutine int) { + b.Helper() cb := &CumulativeBucket{} w := sync.WaitGroup{} w.Add(numGoroutine) diff --git a/sfxclient/httpsink.go b/sfxclient/httpsink.go index 85d8ed3..2061724 100644 --- a/sfxclient/httpsink.go +++ b/sfxclient/httpsink.go @@ -4,7 +4,7 @@ import ( "bytes" "compress/gzip" "context" - "crypto/sha1" + "crypto/sha256" "encoding/hex" "encoding/json" "fmt" @@ -157,7 +157,7 @@ var _ Sink = &HTTPSink{} const TokenHeaderName = "X-Sf-Token" func getShaValue(values []string) string { - h := sha1.New() + h := sha256.New() for _, v := range values { h.Write([]byte(v)) } @@ -382,7 +382,10 @@ func (h *HTTPSink) getReader(b []byte) (io.Reader, bool, error) { var err error if !h.DisableCompression && len(b) > 1500 { buf := new(bytes.Buffer) // TODO use a pool for this too? - w := h.zippers.Get().(*gzip.Writer) + w, ok := h.zippers.Get().(*gzip.Writer) + if !ok { + return nil, false, errors.New("invalid gzip writer") + } defer h.zippers.Put(w) w.Reset(buf) _, err = w.Write(b) diff --git a/sfxclient/httpsink_test.go b/sfxclient/httpsink_test.go index 1fe7bbe..a35cb8e 100644 --- a/sfxclient/httpsink_test.go +++ b/sfxclient/httpsink_test.go @@ -11,6 +11,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" "testing" "time" @@ -31,13 +32,13 @@ type errReader struct { shouldBlock chan struct{} } -var errReadErr = errors.New("read bad") +var errRead = errors.New("read bad") func (e *errReader) Read(_ []byte) (n int, err error) { if e.shouldBlock != nil { <-e.shouldBlock } - return 0, errReadErr + return 0, errRead } func TestHelperFunctions(t *testing.T) { @@ -108,6 +109,20 @@ func TestGoEventSource(t *testing.T) { }) } +func TestHTTPSinkZipper(t *testing.T) { + t.Parallel() + Convey("http sink zippers should be setup properly", t, func() { + sink := NewHTTPSink() + reader, _, err := sink.getReader([]byte(longTraceExample)) + So(reader, ShouldNotBeNil) + So(err, ShouldBeNil) + sink.zippers = sync.Pool{New: func() interface{} { return bytes.NewBuffer([]byte{}) }} + reader, _, err = sink.getReader([]byte(longTraceExample)) + So(err.Error(), ShouldContainSubstring, "invalid gzip writer") + So(reader, ShouldBeNil) + }) +} + func ExampleHTTPSink() { sink := NewHTTPSink() sink.AuthToken = "ABCDEFG" @@ -268,17 +283,17 @@ func TestHTTPDatapointSink(t *testing.T) { } Convey("retry after seconds", func() { retHeaders["Retry-After"] = "1" - err, ok := s.AddDatapoints(ctx, dps).(*TooManyRequestError) - So(ok, ShouldBeTrue) - So(err.RetryAfter, ShouldEqual, time.Second) - So(err.Error(), ShouldContainSubstring, "[x] too many requests, retry after") + var tooManyRequestError *TooManyRequestError + So(goerrors.As(s.AddDatapoints(ctx, dps), &tooManyRequestError), ShouldBeTrue) + So(tooManyRequestError.RetryAfter, ShouldEqual, time.Second) + So(tooManyRequestError.Error(), ShouldContainSubstring, "[x] too many requests, retry after") }) Convey("retry until date", func() { retHeaders["Retry-After"] = time.Now().Add(time.Second).Format(time.RFC850) - err, ok := s.AddDatapoints(ctx, dps).(*TooManyRequestError) - So(ok, ShouldBeTrue) - So(err.RetryAfter, ShouldBeLessThanOrEqualTo, time.Second) - So(err.Error(), ShouldContainSubstring, "[x] too many requests, retry after") + var tooManyRequestError *TooManyRequestError + So(goerrors.As(s.AddDatapoints(ctx, dps), &tooManyRequestError), ShouldBeTrue) + So(tooManyRequestError.RetryAfter, ShouldBeLessThanOrEqualTo, time.Second) + So(tooManyRequestError.Error(), ShouldContainSubstring, "[x] too many requests, retry after") }) Convey("error fallback", func() { retHeaders["Retry-After"] = "" diff --git a/sfxclient/multitokensink.go b/sfxclient/multitokensink.go index 66ec26f..35b22da 100644 --- a/sfxclient/multitokensink.go +++ b/sfxclient/multitokensink.go @@ -2,6 +2,7 @@ package sfxclient import ( "context" + "errors" "fmt" "hash" "hash/fnv" @@ -53,12 +54,15 @@ func getHTTPStatusCode(status *tokenStatus, err error) *tokenStatus { if err == nil { status.status = http.StatusOK } else { - // refactor: use `errors.As` to simplify type assertion on error chain once we get rid of go 1.12 build. - if obj, ok := err.(*TooManyRequestError); ok { - err = obj.Err + var ( + tooManyRequestErr *TooManyRequestError + sfxAPIErr *SFXAPIError + ) + if errors.As(err, &tooManyRequestErr) { + err = tooManyRequestErr.Err } - if obj, ok := err.(*SFXAPIError); ok { - status.status = obj.StatusCode + if errors.As(err, &sfxAPIErr) { + status.status = sfxAPIErr.StatusCode } } return status @@ -206,6 +210,7 @@ func (w *datapointWorker) emit(token string) { w.buffer = w.buffer[:0] } +//nolint:dupl func (w *datapointWorker) handleError(err error, token string, datapoints []*datapoint.Datapoint, addDatapoints func(context.Context, []*datapoint.Datapoint) error) { errr := err status := &tokenStatus{ @@ -321,6 +326,7 @@ func (w *eventWorker) emit(token string) { w.buffer = w.buffer[:0] } +//nolint:dupl func (w *eventWorker) handleError(err error, token string, events []*event.Event, addEvents func(context.Context, []*event.Event) error) { errr := err status := &tokenStatus{ @@ -437,6 +443,7 @@ func (w *spanWorker) emit(token string) { w.buffer = w.buffer[:0] } +//nolint:dupl func (w *spanWorker) handleError(err error, token string, traces []*trace.Span, addSpans func(context.Context, []*trace.Span) error) { errr := err status := &tokenStatus{ @@ -637,6 +644,7 @@ func (a *AsyncMultiTokenSink) getChannel(input string, size int) (workerID int64 } // AddDatapointsWithToken emits a list of datapoints using a supplied token +//nolint:dupl func (a *AsyncMultiTokenSink) AddDatapointsWithToken(token string, datapoints []*datapoint.Datapoint) (err error) { var channelID int64 if channelID, err = a.getChannel(token, len(a.dpChannels)); err == nil { @@ -660,7 +668,7 @@ func (a *AsyncMultiTokenSink) AddDatapointsWithToken(token string, datapoints [] } } } else { - err = fmt.Errorf("unable to add datapoints: there was an error while hashing the token to a worker. %v", err) + err = fmt.Errorf("unable to add datapoints: there was an error while hashing the token to a worker. %w", err) } return @@ -677,6 +685,7 @@ func (a *AsyncMultiTokenSink) AddDatapoints(ctx context.Context, datapoints []*d } // AddEventsWithToken emits a list of events using a supplied token +//nolint:dupl func (a *AsyncMultiTokenSink) AddEventsWithToken(token string, events []*event.Event) (err error) { var channelID int64 if channelID, err = a.getChannel(token, len(a.evChannels)); err == nil { @@ -699,11 +708,9 @@ func (a *AsyncMultiTokenSink) AddEventsWithToken(token string, events []*event.E err = fmt.Errorf("unable to add events: the input buffer is full") } } - } else { - err = fmt.Errorf("unable to add events: there was an error while hashing the token to a worker. %v", err) + err = fmt.Errorf("unable to add events: there was an error while hashing the token to a worker. %w", err) } - return } @@ -718,6 +725,7 @@ func (a *AsyncMultiTokenSink) AddEvents(ctx context.Context, events []*event.Eve } // AddSpansWithToken emits a list of events using a supplied token +//nolint:dupl func (a *AsyncMultiTokenSink) AddSpansWithToken(token string, spans []*trace.Span) (err error) { var channelID int64 if channelID, err = a.getChannel(token, len(a.evChannels)); err == nil { @@ -740,11 +748,9 @@ func (a *AsyncMultiTokenSink) AddSpansWithToken(token string, spans []*trace.Spa err = fmt.Errorf("unable to add spans: the input buffer is full") } } - } else { - err = fmt.Errorf("unable to add spans: there was an error while hashing the token to a worker. %v", err) + err = fmt.Errorf("unable to add spans: there was an error while hashing the token to a worker. %w", err) } - return } @@ -830,6 +836,7 @@ type spanChannel struct { workers []*spanWorker } +//nolint:dupl func newDPChannel(numDrainingThreads int64, buffer int, batchSize int, datapointEndpoint string, userAgent string, httpClient func() *http.Client, errorHandler func(error) error, stats *asyncMultiTokenSinkStats, closing chan bool, done chan bool, maxRetry int) (dpc *dpChannel) { dpc = &dpChannel{ input: make(chan *dpMsg, int64(buffer)), @@ -851,6 +858,7 @@ func newDPChannel(numDrainingThreads int64, buffer int, batchSize int, datapoint return } +//nolint:dupl func newEVChannel(numDrainingThreads int64, buffer int, batchSize int, eventEndpoint string, userAgent string, httpClient func() *http.Client, errorHandler func(error) error, stats *asyncMultiTokenSinkStats, closing chan bool, done chan bool, maxRetry int) (evc *evChannel) { evc = &evChannel{ input: make(chan *evMsg, int64(buffer)), @@ -872,6 +880,7 @@ func newEVChannel(numDrainingThreads int64, buffer int, batchSize int, eventEndp return } +//nolint:dupl func newSpanChannel(numDrainingThreads int64, buffer int, batchSize int, traceEndpoint string, userAgent string, httpClient func() *http.Client, errorHandler func(error) error, stats *asyncMultiTokenSinkStats, closing chan bool, done chan bool, maxRetry int) (spc *spanChannel) { spc = &spanChannel{ input: make(chan *spanMsg, int64(buffer)), diff --git a/sfxclient/multitokensink_test.go b/sfxclient/multitokensink_test.go index d7d075c..5e1a30f 100644 --- a/sfxclient/multitokensink_test.go +++ b/sfxclient/multitokensink_test.go @@ -13,6 +13,7 @@ import ( "github.com/juju/errors" "github.com/signalfx/golib/v3/datapoint" "github.com/signalfx/golib/v3/event" + "github.com/signalfx/golib/v3/log" "github.com/signalfx/golib/v3/trace" . "github.com/smartystreets/goconvey/convey" ) @@ -349,46 +350,56 @@ func TestAsyncMultiTokenSinkShutdownDroppedSpans(t *testing.T) { } func TestAsyncTokenStatusCounter(t *testing.T) { - s := NewAsyncTokenStatusCounter("testCounter", 5000, 1, map[string]string{"testdim1": "testdimval"}) - wg := sync.WaitGroup{} - wg.Add(5) - for i := 0; i < 5; i++ { - go func() { - d := &tokenStatus{ - status: http.StatusOK, - token: "HELLOOO", - val: 5, - } - for i := 0; i < 5; i++ { - s.Increment(d) - } - wg.Done() - }() - } - wg.Wait() - dps := s.Datapoints() - Convey("An AsyncTokenStatusMap should be able to accept simultaneous calls to Increment", t, func() { - So(dps, ShouldNotBeNil) - dp := dps[0] - for dp.Value.(datapoint.IntValue).Int() != 125 { - runtime.Gosched() - dp = s.Datapoints()[0] + t.Parallel() + Convey("we should be able to properly increment the counts async", t, func() { + s := NewAsyncTokenStatusCounter("testCounter", 5000, 1, map[string]string{"testdim1": "testdimval"}) + wg := sync.WaitGroup{} + wg.Add(5) + for i := 0; i < 5; i++ { + go func() { + d := &tokenStatus{ + status: http.StatusOK, + token: "HELLOOO", + val: 5, + } + for i := 0; i < 5; i++ { + s.Increment(d) + } + wg.Done() + }() } - So(dp.Value.(datapoint.IntValue).Int(), ShouldEqual, 125) + wg.Wait() + dps := s.Datapoints() + Convey("An AsyncTokenStatusMap should be able to accept simultaneous calls to Increment", func() { + So(dps, ShouldNotBeNil) + dp := dps[0] + for dp.Value.(datapoint.IntValue).Int() != 125 { + runtime.Gosched() + dp = s.Datapoints()[0] + } + So(dp.Value.(datapoint.IntValue).Int(), ShouldEqual, 125) + }) }) } func TestAsyncMultiTokenSinkCleanCloseDatapointsEventsAndSpans(t *testing.T) { + t.Parallel() Convey("An AsyncMultiTokenSink", t, func() { Convey("should gracefully shutdown after some data is added to it", func() { - s := NewAsyncMultiTokenSink(int64(2), int64(2), 5, 2500, "", "", "", "", newDefaultHTTPClient, nil, 0) + s := NewAsyncMultiTokenSink(int64(2), int64(2), 5, 2500, "", "", "", "", newDefaultHTTPClient, func(err error) error { + log.Discard.Log(log.Err, err, "Unable to handle error") + return nil + }, 0) dps := GoMetricsSource.Datapoints() evs := GoEventSource.Events() sps := GoSpanSource.Spans() s.ShutdownTimeout = time.Second * 5 iterations := 10000 + wg := sync.WaitGroup{} for i := 0; i < iterations; i++ { + wg.Add(1) go func() { + defer wg.Done() _ = s.AddDatapointsWithToken("HELLOOOOOO", dps) _ = s.AddEventsWithToken("HELLOOOOOO", evs) _ = s.AddSpansWithToken("HELLOOOOOO", sps) @@ -399,6 +410,7 @@ func TestAsyncMultiTokenSinkCleanCloseDatapointsEventsAndSpans(t *testing.T) { }() runtime.Gosched() } + wg.Wait() for atomic.LoadInt64(&s.dpBuffered) <= int64(iterations) { runtime.Gosched() } @@ -459,6 +471,7 @@ func ProcessDatapoints(data []*datapoint.Datapoint) (dpDropped, evDropped, spans } func TestAsyncMultiTokenSinkDatapoints(t *testing.T) { + t.Parallel() verifyDrop := func(s *AsyncMultiTokenSink, expDpDropped, expEvDropped, expSpanDropped, expDpEm, expEvEm, expSpanEm int64) { data := s.Datapoints() So(data, ShouldNotBeEmpty) diff --git a/sfxclient/rollbucket_test.go b/sfxclient/rollbucket_test.go index 86b5e76..a3f9b62 100644 --- a/sfxclient/rollbucket_test.go +++ b/sfxclient/rollbucket_test.go @@ -103,6 +103,7 @@ func BenchmarkRollingBucket10(b *testing.B) { } func benchRB(b *testing.B, numGoroutine int) { + b.Helper() cb := NewRollingBucket("", nil) w := sync.WaitGroup{} w.Add(numGoroutine) diff --git a/sfxclient/sfxclient_test.go b/sfxclient/sfxclient_test.go index 81e3a47..d82b361 100644 --- a/sfxclient/sfxclient_test.go +++ b/sfxclient/sfxclient_test.go @@ -124,29 +124,30 @@ func TestScheduler_ReportingTimeout(t *testing.T) { } func TestCollectDatapointDebug(t *testing.T) { - var handleErrors []error - var handleErrRet error - s := &Scheduler{ - ErrorHandler: func(e error) error { - handleErrors = append(handleErrors, e) - return errors.Wrap(handleErrRet, e) - }, - ReportingDelayNs: time.Second.Nanoseconds(), - callbackMap: make(map[string]*callbackPair), - } - sink := &testSink{ - lastDatapoints: make(chan []*datapoint.Datapoint, 1), - } - s.Sink = sink - tk := timekeepertest.NewStubClock(time.Now()) - s.Timer = tk - - ctx, cancel := context.WithCancel(context.Background()) Convey("testing collect datapoints with debug mode enabled", t, func() { + var handleErrors []error + var handleErrRet error + s := &Scheduler{ + ErrorHandler: func(e error) error { + handleErrors = append(handleErrors, e) + return errors.Wrap(handleErrRet, e) + }, + ReportingDelayNs: time.Second.Nanoseconds(), + callbackMap: make(map[string]*callbackPair), + } + sink := &testSink{ + lastDatapoints: make(chan []*datapoint.Datapoint, 1), + } + s.Sink = sink + tk := timekeepertest.NewStubClock(time.Now()) + s.Timer = tk + + ctx, cancel := context.WithCancel(context.Background()) s.Debug(true) s.AddCallback(GoMetricsSource) s.AddGroupedCallback("goMetrics", GoMetricsSource) go s.Schedule(ctx) + runtime.Gosched() go func() { for { runtime.Gosched() diff --git a/sfxclient/spanfilter/spanfilter.go b/sfxclient/spanfilter/spanfilter.go index e237cf9..94e571f 100644 --- a/sfxclient/spanfilter/spanfilter.go +++ b/sfxclient/spanfilter/spanfilter.go @@ -3,6 +3,7 @@ package spanfilter import ( "context" "encoding/json" + goerrors "errors" "strings" "github.com/signalfx/golib/v3/errors" @@ -10,6 +11,7 @@ import ( // Map is the response we return from ingest wrt our span endpoint // It contains the number of spans that were valid, and a map of string reason to spanIds for each invalid span +//nolint:errname type Map struct { Valid int `json:"valid"` Invalid map[string][]string `json:"invalid,omitempty"` @@ -36,7 +38,7 @@ const ( RequiredTagMissing = "requiredTagMissing" ) -var emptySpanFilter = &Map{} +var errEmptySpanFilter = &Map{} const ( // OK valid spans @@ -76,6 +78,7 @@ func (s *Map) AddValid(i int) { } // FromBytes returns a Map or an error +//nolint:nilerr func FromBytes(body []byte) *Map { var spanFilter Map if err := json.Unmarshal(body, &spanFilter); err != nil { @@ -126,21 +129,20 @@ func GetSpanFilterMapFromContext(ctx context.Context) error { return failMap } } - return emptySpanFilter + return errEmptySpanFilter } // IsMap returns whether an error is an instance of Map func IsMap(err error) bool { - if _, ok := err.(*Map); ok { - return true - } - return false + var errMap *Map + return goerrors.As(err, &errMap) } // IsInvalid returns false if it's a Map with no invalid entries or nil, else true func IsInvalid(err error) bool { - if m, ok := err.(*Map); ok { - return m.CheckInvalid() + var errMap *Map + if goerrors.As(err, &errMap) { + return errMap.CheckInvalid() } return err != nil } diff --git a/sfxclient/timerange_test.go b/sfxclient/timerange_test.go index 37e3cba..cdf9b0d 100644 --- a/sfxclient/timerange_test.go +++ b/sfxclient/timerange_test.go @@ -7,13 +7,8 @@ import ( . "github.com/smartystreets/goconvey/convey" ) -type basicDuration time.Duration - -func (b basicDuration) Get() time.Duration { - return time.Duration(b) -} - func TestTimeCounter(t *testing.T) { + t.Parallel() Convey("Default error handler should not panic", t, func() { tc := TimeCounter{ NsBarrier: time.Second.Nanoseconds(), diff --git a/timekeeper/timekeepertest/timekeepertest.go b/timekeeper/timekeepertest/timekeepertest.go index acf2e17..55f852d 100644 --- a/timekeeper/timekeepertest/timekeepertest.go +++ b/timekeeper/timekeepertest/timekeepertest.go @@ -94,7 +94,7 @@ func (s *StubClock) triggerChans() { for len(s.waitingChans) > 0 { firstItem := s.waitingChans[0] if !firstItem.triggerTime.After(s.currentTime) { - i := heap.Pop(&s.waitingChans).(*chanTime) + i, _ := heap.Pop(&s.waitingChans).(*chanTime) i.ch <- i.triggerTime runtime.Gosched() } else { diff --git a/trace/translator/sfx_test.go b/trace/translator/sfx_test.go index 82a97c5..abda8f3 100644 --- a/trace/translator/sfx_test.go +++ b/trace/translator/sfx_test.go @@ -15,7 +15,9 @@ package translator import ( + "net/http" "sort" + "strings" "testing" "time" @@ -139,18 +141,21 @@ func TestBadSpans(t *testing.T) { } func assertBatchesAreEqual(t *testing.T, got, want *jaegerpb.Batch) { + t.Helper() require.Equal(t, len(got.Spans), len(want.Spans)) assertProcessesAreEqual(t, got.Process, want.Process) assertSpansAreEqual(t, got.Spans, want.Spans) } func assertProcessesAreEqual(t *testing.T, got, want *jaegerpb.Process) { + t.Helper() sortTags(want.Tags) sortTags(got.Tags) assert.Equal(t, got, want) } func assertSpansAreEqual(t *testing.T, got, want []*jaegerpb.Span) { + t.Helper() sortSpans(got) sortSpans(want) for i := 0; i < len(got); i++ { @@ -229,7 +234,7 @@ var wantPostRequest = sapmpb.PostSpansRequest{ SpanID: jaegerpb.SpanID(0x147d98), TraceID: jaegerpb.TraceID{Low: 11715721395283892799}, OperationName: "get", - StartTime: time.Date(2017, 01, 26, 21, 46, 31, 639875000, time.UTC), + StartTime: time.Date(2017, 1, 26, 21, 46, 31, 639875000, time.UTC), Duration: time.Duration(22938000), Flags: 0, Process: nil, @@ -280,7 +285,7 @@ var wantPostRequest = sapmpb.PostSpansRequest{ }, Logs: []jaegerpb.Log{ { - Timestamp: time.Date(2017, 01, 26, 21, 46, 31, 639875000, time.UTC), + Timestamp: time.Date(2017, 1, 26, 21, 46, 31, 639875000, time.UTC), Fields: []jaegerpb.KeyValue{ { Key: "key1", @@ -295,7 +300,7 @@ var wantPostRequest = sapmpb.PostSpansRequest{ }, }, { - Timestamp: time.Date(2017, 01, 26, 21, 46, 31, 639875000, time.UTC), + Timestamp: time.Date(2017, 1, 26, 21, 46, 31, 639875000, time.UTC), Fields: []jaegerpb.KeyValue{ { Key: "annotation", @@ -311,7 +316,7 @@ var wantPostRequest = sapmpb.PostSpansRequest{ TraceID: jaegerpb.TraceID{Low: 12868642899890739775, High: 1}, SpanID: jaegerpb.SpanID(0x21d092272e), OperationName: "post", - StartTime: time.Date(2017, 01, 26, 21, 46, 31, 639875000, time.UTC), + StartTime: time.Date(2017, 1, 26, 21, 46, 31, 639875000, time.UTC), Duration: time.Microsecond * 22938, References: []jaegerpb.SpanRef{{ TraceID: jaegerpb.TraceID{Low: 12868642899890739775, High: 1}, @@ -341,7 +346,7 @@ var wantPostRequest = sapmpb.PostSpansRequest{ TraceID: jaegerpb.TraceID{Low: 14021564404497586751}, SpanID: jaegerpb.SpanID(213952636718), OperationName: "post", - StartTime: time.Date(2017, 01, 26, 21, 46, 31, 639875000, time.UTC), + StartTime: time.Date(2017, 1, 26, 21, 46, 31, 639875000, time.UTC), Duration: time.Microsecond * 22938, References: []jaegerpb.SpanRef{{ TraceID: jaegerpb.TraceID{Low: 14021564404497586751}, @@ -361,8 +366,8 @@ var wantPostRequest = sapmpb.PostSpansRequest{ TraceID: jaegerpb.TraceID{Low: 15174485909104433727}, SpanID: jaegerpb.SpanID(47532398882098234), - OperationName: "post", - StartTime: time.Date(2017, 01, 26, 21, 46, 31, 639875000, time.UTC), + OperationName: strings.ToLower(http.MethodPost), + StartTime: time.Date(2017, 1, 26, 21, 46, 31, 639875000, time.UTC), Duration: time.Microsecond * 22938, Flags: 2, References: []jaegerpb.SpanRef{ @@ -391,7 +396,7 @@ var wantPostRequest = sapmpb.PostSpansRequest{ TraceID: jaegerpb.TraceID{Low: 16327407413711280703}, SpanID: jaegerpb.SpanID(52035998509468730), OperationName: "post", - StartTime: time.Date(2017, 01, 26, 21, 46, 31, 639875000, time.UTC), + StartTime: time.Date(2017, 1, 26, 21, 46, 31, 639875000, time.UTC), Duration: time.Microsecond * 22938, Flags: 2, References: []jaegerpb.SpanRef{ @@ -447,7 +452,7 @@ var wantPostRequest = sapmpb.PostSpansRequest{ TraceID: jaegerpb.TraceID{Low: 17480328918319176255}, SpanID: jaegerpb.SpanID(58525199627357242), OperationName: "post", - StartTime: time.Date(2017, 01, 26, 21, 46, 31, 639875000, time.UTC), + StartTime: time.Date(2017, 1, 26, 21, 46, 31, 639875000, time.UTC), Duration: time.Microsecond * 22938, Flags: 2, References: []jaegerpb.SpanRef{ @@ -498,7 +503,7 @@ var wantPostRequest = sapmpb.PostSpansRequest{ TraceID: jaegerpb.TraceID{Low: 18025686685695023674}, SpanID: jaegerpb.SpanID(63028799254727738), OperationName: "get", - StartTime: time.Date(2017, 01, 26, 21, 46, 31, 639875000, time.UTC), + StartTime: time.Date(2017, 1, 26, 21, 46, 31, 639875000, time.UTC), Duration: time.Microsecond * 22938, Tags: []jaegerpb.KeyValue{ { @@ -541,7 +546,7 @@ var wantPostRequest = sapmpb.PostSpansRequest{ TraceID: jaegerpb.TraceID{Low: 18025686685695023675}, SpanID: jaegerpb.SpanID(63028799254727739), OperationName: "get", - StartTime: time.Date(2017, 01, 26, 21, 46, 31, 639875000, time.UTC), + StartTime: time.Date(2017, 1, 26, 21, 46, 31, 639875000, time.UTC), Duration: time.Microsecond * 22938, Tags: []jaegerpb.KeyValue{ { diff --git a/web/web.go b/web/web.go index 95f056b..01700e5 100644 --- a/web/web.go +++ b/web/web.go @@ -132,7 +132,7 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, r *http.Request) { // ServeHTTPC will pass the request between each middleware layer func (h *Handler) ServeHTTPC(ctx context.Context, rw http.ResponseWriter, r *http.Request) { - stack := h.Pool.Get().(ContextHandler) + stack, _ := h.Pool.Get().(ContextHandler) stack.ServeHTTPC(ctx, rw, r) h.Pool.Put(stack) } diff --git a/zkplus/zkplus.go b/zkplus/zkplus.go index aec7688..1b7f90c 100644 --- a/zkplus/zkplus.go +++ b/zkplus/zkplus.go @@ -1,6 +1,7 @@ package zkplus import ( + goerrors "errors" "fmt" "strings" "time" @@ -110,7 +111,7 @@ func (z *ZkPlus) ensureRootPath(conn zktest.ZkConnSupported) error { _, err := conn.Create(totalPath, []byte(""), 0, zk.WorldACL(zk.PermAll)) // There could be a race where the root is created in the // meantime since the Exists check above, so ignore ErrNodeExists. - if err != nil && err != zk.ErrNodeExists { + if err != nil && !goerrors.Is(err, zk.ErrNodeExists) { return errors.Annotatef(err, "cannot create path %s", totalPath) } } else { diff --git a/zkplus/zkplus_test.go b/zkplus/zkplus_test.go index 52aadc1..9e0234d 100644 --- a/zkplus/zkplus_test.go +++ b/zkplus/zkplus_test.go @@ -22,6 +22,7 @@ func TestPrefix(t *testing.T) { } func testPrefix(t *testing.T, zkp zktest.ZkConnSupported) { + t.Helper() defer func() { log.IfErr(log.Panic, zktest.EnsureDelete(zkp, "modifyNode")) }() @@ -132,6 +133,7 @@ func TestWatches(t *testing.T) { } func testWatches(t *testing.T, zkp *ZkPlus) { + t.Helper() ch := zkp.EventChan() defer func() { log.IfErr(log.Panic, zktest.EnsureDelete(zkp, "testWatches")) @@ -192,6 +194,7 @@ func testWatches(t *testing.T, zkp *ZkPlus) { } func create(t *testing.T, zkp *ZkPlus) { + t.Helper() ch := zkp.EventChan() exists, _, ch2, err := zkp.ExistsW("testWatches") assert.NoError(t, err) diff --git a/zkplus/zktest/zktest.go b/zkplus/zktest/zktest.go index 0e2e4cb..cce3e09 100644 --- a/zkplus/zktest/zktest.go +++ b/zkplus/zktest/zktest.go @@ -50,7 +50,7 @@ func EnsureDelete(z ZkConnSupported, path string) error { } for i := 0; i < 3; i++ { err = z.Delete(path, -1) - if err == nil || err == zk.ErrNoNode { + if err == nil || errors.Is(err, zk.ErrNoNode) { return nil } } diff --git a/zkplus/zktest/zktest_test.go b/zkplus/zktest/zktest_test.go index f0cb8f6..5d0d1d7 100644 --- a/zkplus/zktest/zktest_test.go +++ b/zkplus/zktest/zktest_test.go @@ -118,6 +118,7 @@ func TestForcedErrorCheck(t *testing.T) { } func EnsureDeleteTesting(t *testing.T, z ZkConnSupported, path string) { + t.Helper() assert.NoError(t, EnsureDelete(z, path)) } @@ -129,6 +130,7 @@ func TestBasics(t *testing.T) { } func testBasics(t *testing.T, z ZkConnSupported) { + t.Helper() rand.Seed(time.Now().UnixNano()) ensureCreate(z.Create("/test", []byte(""), 0, zk.WorldACL(zk.PermAll))) ensureCreate(z.Create("/test/testBasics", []byte(""), 0, zk.WorldACL(zk.PermAll))) @@ -206,6 +208,7 @@ func TestSet(t *testing.T) { } func testSet(t *testing.T, z ZkConnSupported) { + t.Helper() rand.Seed(time.Now().UnixNano()) ensureCreate(z.Create("/test", []byte(""), 0, zk.WorldACL(zk.PermAll))) ensureCreate(z.Create("/test/testSet", []byte(""), 0, zk.WorldACL(zk.PermAll))) @@ -234,6 +237,7 @@ func TestExistsW(t *testing.T) { } func testExistsW(t *testing.T, z ZkConnSupported, e <-chan zk.Event) { + t.Helper() rand.Seed(time.Now().UnixNano()) ensureCreate(z.Create("/test", []byte(""), 0, zk.WorldACL(zk.PermAll))) ensureCreate(z.Create("/test/testEvents", []byte(""), 0, zk.WorldACL(zk.PermAll))) @@ -286,6 +290,7 @@ func TestGetW(t *testing.T) { } func testGetW(t *testing.T, z ZkConnSupported, e <-chan zk.Event) { + t.Helper() rand.Seed(time.Now().UnixNano()) ensureCreate(z.Create("/test", []byte(""), 0, zk.WorldACL(zk.PermAll))) ensureCreate(z.Create("/test/testGetW", []byte(""), 0, zk.WorldACL(zk.PermAll))) @@ -337,6 +342,7 @@ func TestChildrenW(t *testing.T) { } func testChildrenW(t *testing.T, z ZkConnSupported, z2 ZkConnSupported, e <-chan zk.Event) { + t.Helper() rand.Seed(time.Now().UnixNano()) ensureCreate(z.Create("/test", []byte(""), 0, zk.WorldACL(zk.PermAll))) ensureCreate(z.Create("/test/testChildrenW", []byte(""), 0, zk.WorldACL(zk.PermAll))) @@ -398,6 +404,7 @@ func TestChildrenWNotHere(t *testing.T) { } func testChildrenWNotHere(t *testing.T, z ZkConnSupported, z2 ZkConnSupported, _ <-chan zk.Event) { + t.Helper() rand.Seed(time.Now().UnixNano()) ensureCreate(z.Create("/test", []byte(""), 0, zk.WorldACL(zk.PermAll))) ensureCreate(z.Create("/test/testChildrenWNotHere", []byte(""), 0, zk.WorldACL(zk.PermAll)))