Skip to content

Commit

Permalink
Replace go.uber.org/atomic and github.com/pkg/errors with standar…
Browse files Browse the repository at this point in the history
…d library packages (#5678)

* Replace `go.uber.org/atomic` and `github.com/pkg/errors` with standard library packages

The packages are now forbidden by a linter rule

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Fixed E2E tests failing

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Co-authored-by: Yaron Schneider <schneider.yaron@live.com>
  • Loading branch information
ItalyPaleAle and yaron2 committed Dec 29, 2022
1 parent 366645e commit 1c95ad1
Show file tree
Hide file tree
Showing 71 changed files with 292 additions and 330 deletions.
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ linters-settings:
packages-with-error-message:
- "github.com/Sirupsen/logrus": "must use github.com/dapr/kit/logger"
- "github.com/agrea/ptr": "must use github.com/dapr/kit/ptr"
- "go.uber.org/atomic": "must use sync/atomic"
- "github.com/pkg/errors": "must use standard library (errors package and/or fmt.Errorf)"
- "github.com/cenkalti/backoff": "must use github.com/cenkalti/backoff/v4"
- "github.com/cenkalti/backoff/v2": "must use github.com/cenkalti/backoff/v4"
- "github.com/cenkalti/backoff/v3": "must use github.com/cenkalti/backoff/v4"
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ require (
github.com/minio/blake2b-simd v0.0.0-20160723061019-3f5f724cb5b1
github.com/mitchellh/mapstructure v1.5.1-0.20220423185008-bf980b35cac4
github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.14.0
github.com/prometheus/client_model v0.3.0
github.com/prometheus/common v0.37.0
Expand All @@ -51,7 +50,6 @@ require (
go.opentelemetry.io/otel/trace v1.11.1
go.temporal.io/api v1.13.0
go.temporal.io/sdk v1.19.0
go.uber.org/atomic v1.10.0
go.uber.org/automaxprocs v1.5.1
go.uber.org/ratelimit v0.2.0
golang.org/x/exp v0.0.0-20221028150844-83b7d23a625f
Expand Down Expand Up @@ -320,6 +318,7 @@ require (
github.com/pierrec/lz4 v2.6.0+incompatible // indirect
github.com/pierrec/lz4/v4 v4.1.17 // indirect
github.com/pkg/browser v0.0.0-20210115035449-ce105d075bb4 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect
github.com/pquerna/cachecontrol v0.1.0 // indirect
Expand Down Expand Up @@ -372,6 +371,7 @@ require (
go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.11.1 // indirect
go.opentelemetry.io/proto/otlp v0.19.0 // indirect
go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect
go.uber.org/atomic v1.10.0 // indirect
go.uber.org/multierr v1.8.0 // indirect
go.uber.org/zap v1.24.0 // indirect
golang.org/x/crypto v0.4.0 // indirect
Expand Down
10 changes: 4 additions & 6 deletions pkg/actors/actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@ limitations under the License.
package actors

import (
"errors"
"sync"
"sync/atomic"
"time"

"go.uber.org/atomic"

"github.com/pkg/errors"

diag "github.com/dapr/dapr/pkg/diagnostics"
)

Expand Down Expand Up @@ -91,7 +89,7 @@ func (a *actor) channel() chan struct{} {

// lock holds the lock for turn-based concurrency.
func (a *actor) lock(reentrancyID *string) error {
pending := a.pendingActorCalls.Inc()
pending := a.pendingActorCalls.Add(1)
diag.DefaultMonitoring.ReportActorPendingCalls(a.actorType, pending)

err := a.actorLock.Lock(reentrancyID)
Expand All @@ -113,7 +111,7 @@ func (a *actor) lock(reentrancyID *string) error {
// unlock releases the lock for turn-based concurrency. If disposeCh is available,
// it will close the channel to notify runtime to dispose actor.
func (a *actor) unlock() {
pending := a.pendingActorCalls.Dec()
pending := a.pendingActorCalls.Add(-1)
if pending == 0 {
func() {
a.disposeLock.Lock()
Expand Down
15 changes: 7 additions & 8 deletions pkg/actors/actor_lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@ limitations under the License.
package actors

import (
"errors"
"sync"

"github.com/pkg/errors"
"go.uber.org/atomic"
"sync/atomic"
)

var ErrMaxStackDepthExceeded = errors.New("Maximum stack depth exceeded")
var ErrMaxStackDepthExceeded = errors.New("maximum stack depth exceeded")

type ActorLock struct {
methodLock *sync.Mutex
Expand All @@ -35,7 +34,7 @@ func NewActorLock(maxStackDepth int32) ActorLock {
methodLock: &sync.Mutex{},
requestLock: &sync.Mutex{},
activeRequest: nil,
stackDepth: atomic.NewInt32(int32(0)),
stackDepth: &atomic.Int32{},
maxStackDepth: maxStackDepth,
}
}
Expand All @@ -50,16 +49,16 @@ func (a *ActorLock) Lock(requestID *string) error {
if currentRequest == nil || *currentRequest != *requestID {
a.methodLock.Lock()
a.setCurrentID(requestID)
a.stackDepth.Inc()
a.stackDepth.Add(1)
} else {
a.stackDepth.Inc()
a.stackDepth.Add(1)
}

return nil
}

func (a *ActorLock) Unlock() {
a.stackDepth.Dec()
a.stackDepth.Add(-1)
if a.stackDepth.Load() == 0 {
a.clearCurrentID()
a.methodLock.Unlock()
Expand Down
2 changes: 1 addition & 1 deletion pkg/actors/actor_lock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func TestStackDepthLimit(t *testing.T) {
err = lock.Lock(requestID)

assert.NotNil(t, err)
assert.Equal(t, "Maximum stack depth exceeded", err.Error())
assert.Equal(t, "maximum stack depth exceeded", err.Error())
}

func TestLockBlocksForNonMatchingID(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/actors/actor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ limitations under the License.
package actors

import (
"sync/atomic"
"testing"
"time"

"github.com/stretchr/testify/assert"
"go.uber.org/atomic"
)

var reentrancyStackDepth = 32
Expand Down Expand Up @@ -117,7 +117,7 @@ func TestPendingActorCalls(t *testing.T) {
testActor := newActor("testType", "testID", &reentrancyStackDepth)
testActor.lock(nil)

channelClosed := atomic.NewBool(false)
channelClosed := atomic.Bool{}
go func() {
select {
case <-time.After(200 * time.Millisecond):
Expand Down
13 changes: 8 additions & 5 deletions pkg/apphealth/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ package apphealth
import (
"context"
"strconv"
"sync/atomic"
"time"

"go.uber.org/atomic"

"github.com/dapr/kit/logger"
)

Expand Down Expand Up @@ -55,12 +54,16 @@ type ChangeCallback func(status uint8)

// NewAppHealth creates a new AppHealth object.
func NewAppHealth(config *Config, probeFn ProbeFunction) *AppHealth {
// Initial state is unhealthy until we validate it
failureCount := &atomic.Int32{}
failureCount.Store(config.Threshold)

return &AppHealth{
config: config,
probeFn: probeFn,
report: make(chan uint8, 1),
failureCount: atomic.NewInt32(config.Threshold), // Initial state is unhealthy until we validate it
lastReport: atomic.NewInt64(0),
failureCount: failureCount,
lastReport: &atomic.Int64{},
reportMinInterval: 1e6,
queue: make(chan struct{}, 1),
}
Expand Down Expand Up @@ -209,7 +212,7 @@ func (h *AppHealth) setResult(successful bool) {
}

// Count the failure
failures := h.failureCount.Inc()
failures := h.failureCount.Add(1)

// First, check if we've overflown
if failures < 0 {
Expand Down
10 changes: 5 additions & 5 deletions pkg/apphealth/health_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ package apphealth
import (
"math"
"sync"
"sync/atomic"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/atomic"
)

func TestAppHealth_setResult(t *testing.T) {
Expand All @@ -34,14 +34,14 @@ func TestAppHealth_setResult(t *testing.T) {
h.setResult(true)

statusChange := make(chan uint8, 1)
unexpectedStatusChanges := atomic.NewUint32(0)
unexpectedStatusChanges := atomic.Int32{}
h.OnHealthChange(func(status uint8) {
select {
case statusChange <- status:
// Do nothing
default:
// If the channel is full, it means we were not expecting a status change
unexpectedStatusChanges.Inc()
unexpectedStatusChanges.Add(1)
}
})

Expand Down Expand Up @@ -121,7 +121,7 @@ func TestAppHealth_ratelimitReports(t *testing.T) {
var minInterval int64 = 1e5

h := &AppHealth{
lastReport: atomic.NewInt64(0),
lastReport: &atomic.Int64{},
reportMinInterval: minInterval,
}

Expand Down Expand Up @@ -166,7 +166,7 @@ func TestAppHealth_ratelimitReports(t *testing.T) {
// Repeat, but run with 3 parallel goroutines
time.Sleep(time.Duration(minInterval+10) * time.Microsecond)
wg := sync.WaitGroup{}
totalPassed := atomic.NewInt64(0)
totalPassed := atomic.Int64{}
for i := 0; i < 3; i++ {
wg.Add(1)
go func() {
Expand Down
9 changes: 3 additions & 6 deletions pkg/components/bindings/input_pluggable.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,14 @@ package bindings

import (
"context"
"fmt"
"io"
"sync"

"github.com/dapr/components-contrib/bindings"
"github.com/dapr/dapr/pkg/components/pluggable"
proto "github.com/dapr/dapr/pkg/proto/components/v1"

"github.com/dapr/components-contrib/bindings"

"github.com/dapr/kit/logger"

"github.com/pkg/errors"
)

// grpcInputBinding is a implementation of a inputbinding over a gRPC Protocol.
Expand Down Expand Up @@ -101,7 +98,7 @@ func (b *grpcInputBinding) adaptHandler(ctx context.Context, streamingPull proto
func (b *grpcInputBinding) Read(ctx context.Context, handler bindings.Handler) error {
readStream, err := b.Client.Read(ctx)
if err != nil {
return errors.Wrapf(err, "unable to read from binding")
return fmt.Errorf("unable to read from binding: %w", err)
}

streamCtx, cancel := context.WithCancel(readStream.Context())
Expand Down
7 changes: 3 additions & 4 deletions pkg/components/bindings/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ limitations under the License.
package bindings

import (
"fmt"
"strings"

"github.com/pkg/errors"

"github.com/dapr/components-contrib/bindings"
"github.com/dapr/dapr/pkg/components"
"github.com/dapr/kit/logger"
Expand Down Expand Up @@ -60,15 +59,15 @@ func (b *Registry) CreateInputBinding(name, version string) (bindings.InputBindi
if method, ok := b.getInputBinding(name, version); ok {
return method(), nil
}
return nil, errors.Errorf("couldn't find input binding %s/%s", name, version)
return nil, fmt.Errorf("couldn't find input binding %s/%s", name, version)
}

// CreateOutputBinding Create instantiates an output binding based on `name`.
func (b *Registry) CreateOutputBinding(name, version string) (bindings.OutputBinding, error) {
if method, ok := b.getOutputBinding(name, version); ok {
return method(), nil
}
return nil, errors.Errorf("couldn't find output binding %s/%s", name, version)
return nil, fmt.Errorf("couldn't find output binding %s/%s", name, version)
}

// HasInputBinding checks if an input binding based on `name` exists in the registry.
Expand Down
9 changes: 4 additions & 5 deletions pkg/components/bindings/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,15 @@ limitations under the License.
package bindings_test

import (
"fmt"
"strings"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"

b "github.com/dapr/components-contrib/bindings"
"github.com/dapr/kit/logger"

"github.com/dapr/dapr/pkg/components/bindings"
"github.com/dapr/kit/logger"
)

type (
Expand Down Expand Up @@ -90,7 +89,7 @@ func TestRegistry(t *testing.T) {
assert.False(t, testRegistry.HasInputBinding(componentName, "v1"))
assert.False(t, testRegistry.HasInputBinding(componentName, "v2"))
p, actualError := testRegistry.CreateInputBinding(componentName, "v1")
expectedError := errors.Errorf("couldn't find input binding %s/v1", componentName)
expectedError := fmt.Errorf("couldn't find input binding %s/v1", componentName)

// assert
assert.Nil(t, p)
Expand Down Expand Up @@ -144,7 +143,7 @@ func TestRegistry(t *testing.T) {
assert.False(t, testRegistry.HasOutputBinding(componentName, "v1"))
assert.False(t, testRegistry.HasOutputBinding(componentName, "v2"))
p, actualError := testRegistry.CreateOutputBinding(componentName, "v1")
expectedError := errors.Errorf("couldn't find output binding %s/v1", componentName)
expectedError := fmt.Errorf("couldn't find output binding %s/v1", componentName)

// assert
assert.Nil(t, p)
Expand Down
5 changes: 2 additions & 3 deletions pkg/components/configuration/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ limitations under the License.
package configuration

import (
"fmt"
"strings"

"github.com/pkg/errors"

"github.com/dapr/components-contrib/configuration"
"github.com/dapr/dapr/pkg/components"
"github.com/dapr/kit/logger"
Expand Down Expand Up @@ -53,7 +52,7 @@ func (s *Registry) Create(name, version string) (configuration.Store, error) {
if method, ok := s.getConfigurationStore(name, version); ok {
return method(), nil
}
return nil, errors.Errorf("couldn't find configuration store %s/%s", name, version)
return nil, fmt.Errorf("couldn't find configuration store %s/%s", name, version)
}

func (s *Registry) getConfigurationStore(name, version string) (func() configuration.Store, bool) {
Expand Down
7 changes: 3 additions & 4 deletions pkg/components/configuration/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,15 @@ limitations under the License.
package configuration_test

import (
"fmt"
"strings"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"

c "github.com/dapr/components-contrib/configuration"
"github.com/dapr/dapr/pkg/components/configuration"
"github.com/dapr/kit/logger"

c "github.com/dapr/components-contrib/configuration"
)

type mockState struct {
Expand Down Expand Up @@ -79,7 +78,7 @@ func TestRegistry(t *testing.T) {

// act
p, actualError := testRegistry.Create(componentName, "v1")
expectedError := errors.Errorf("couldn't find configuration store %s/v1", componentName)
expectedError := fmt.Errorf("couldn't find configuration store %s/v1", componentName)

// assert
assert.Nil(t, p)
Expand Down

0 comments on commit 1c95ad1

Please sign in to comment.