From 434d206d083ee81aba57690c2df990189b7109b8 Mon Sep 17 00:00:00 2001 From: Mark Vanderwiel Date: Fri, 14 Jun 2024 16:02:12 -0500 Subject: [PATCH] Add rule unlock error metric Related https://github.ibm.com/alchemy-containers/armada-ballast/issues/2783 --- go.mod | 6 +++--- go.sum | 12 ++++++------ metrics/prometheus.go | 11 +++++++++++ rules/lock/cooloff_lock.go | 2 +- rules/lock/lock.go | 4 ++-- rules/lock/map_locker.go | 2 +- rules/lock/metrics.go | 32 +++++++++++++++++++++++--------- rules/lock/metrics_test.go | 11 ++++++++--- rules/lock/mock.go | 8 ++++---- rules/lock/nested_lock.go | 2 +- rules/lock/nested_lock_test.go | 2 +- 11 files changed, 61 insertions(+), 31 deletions(-) diff --git a/go.mod b/go.mod index 9846c72..0a287e2 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( go.etcd.io/etcd/api/v3 v3.5.14 go.etcd.io/etcd/client/v3 v3.5.14 go.uber.org/zap v1.27.0 - golang.org/x/net v0.25.0 + golang.org/x/net v0.26.0 ) require ( @@ -26,8 +26,8 @@ require ( github.com/prometheus/procfs v0.14.0 // indirect go.etcd.io/etcd/client/pkg/v3 v3.5.14 // indirect go.uber.org/multierr v1.10.0 // indirect - golang.org/x/sys v0.20.0 // indirect - golang.org/x/text v0.15.0 // indirect + golang.org/x/sys v0.21.0 // indirect + golang.org/x/text v0.16.0 // indirect google.golang.org/genproto v0.0.0-20240123012728-ef4313101c80 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240123012728-ef4313101c80 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240123012728-ef4313101c80 // indirect diff --git a/go.sum b/go.sum index e1c8e87..bf54f7e 100644 --- a/go.sum +++ b/go.sum @@ -60,20 +60,20 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac= -golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM= +golang.org/x/net v0.26.0 h1:soB7SVo0PWrY4vPW/+ay0jKDNScG2X9wFeYlXIvJsOQ= +golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= -golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws= +golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.15.0 h1:h1V/4gjBv8v9cjcR6+AR5+/cIYK5N/WAgiv4xlsEtAk= -golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= +golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= +golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= diff --git a/metrics/prometheus.go b/metrics/prometheus.go index d3f7de8..a61e972 100644 --- a/metrics/prometheus.go +++ b/metrics/prometheus.go @@ -14,6 +14,12 @@ var ( Namespace: "rules", Help: "etcd rules engine lock count", }, []string{"locker", "method", "pattern", "success"}) + rulesEngineUnlockErrorCount = prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "unlock_error_count", + Subsystem: "etcd", + Namespace: "rules", + Help: "etcd rules engine unlock error count", + }, []string{"locker", "method", "pattern"}) rulesEngineSatisfiedThenNot = prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "rule_satisfied_then_not", Subsystem: "etcd", @@ -99,6 +105,11 @@ func IncLockMetric(locker string, methodName string, pattern string, lockSucceed rulesEngineLockCount.WithLabelValues(locker, methodName, pattern, strconv.FormatBool(lockSucceeded)).Inc() } +// IncUnlockMetric increments the unlock error count. +func IncUnlockErrorMetric(locker string, methodName string, pattern string) { + rulesEngineUnlockErrorCount.WithLabelValues(locker, methodName, pattern).Inc() +} + // IncSatisfiedThenNot increments the count of a rule having initially been // satisfied and then not satisfied, either after the initial evaluation // or after the lock was obtained. diff --git a/rules/lock/cooloff_lock.go b/rules/lock/cooloff_lock.go index 5fbe2b1..5ad4c3a 100644 --- a/rules/lock/cooloff_lock.go +++ b/rules/lock/cooloff_lock.go @@ -64,6 +64,6 @@ func (col coolOffLocker) Lock(key string, options ...Option) (RuleLock, error) { type coolOffLock struct { } -func (coolOffLock) Unlock() error { +func (coolOffLock) Unlock(_ ...Option) error { return nil } diff --git a/rules/lock/lock.go b/rules/lock/lock.go index c6b4ed4..7286f05 100644 --- a/rules/lock/lock.go +++ b/rules/lock/lock.go @@ -15,7 +15,7 @@ type RuleLocker interface { } type RuleLock interface { - Unlock() error + Unlock(...Option) error } type GetSession func(context.Context) (*v3c.Session, error) @@ -89,7 +89,7 @@ type v3Lock struct { // ErrNilMutex indicates that the lock has a nil mutex var ErrNilMutex = errors.New("mutex is nil") -func (v3l *v3Lock) Unlock() error { +func (v3l *v3Lock) Unlock(_ ...Option) error { if v3l.mutex != nil { // This should be given every chance to complete, otherwise // a lock could prevent future interactions with a resource. diff --git a/rules/lock/map_locker.go b/rules/lock/map_locker.go index 4b691f0..44392e9 100644 --- a/rules/lock/map_locker.go +++ b/rules/lock/map_locker.go @@ -72,7 +72,7 @@ type toggleLock struct { key string } -func (tl toggleLock) Unlock() error { +func (tl toggleLock) Unlock(_ ...Option) error { _ = tl.toggle(tl.key, false) return nil } diff --git a/rules/lock/metrics.go b/rules/lock/metrics.go index e25d3e4..fe38be3 100644 --- a/rules/lock/metrics.go +++ b/rules/lock/metrics.go @@ -4,26 +4,40 @@ import "github.com/IBM-Cloud/go-etcd-rules/metrics" // WithMetrics decorates a locker with metrics. func WithMetrics(ruleLocker RuleLocker, name string) RuleLocker { - return withMetrics(ruleLocker, name, metrics.IncLockMetric) + return withMetrics(ruleLocker, name, metrics.IncLockMetric, metrics.IncUnlockErrorMetric) } func withMetrics(ruleLocker RuleLocker, name string, - observeLock func(locker string, methodName string, pattern string, lockSucceeded bool)) RuleLocker { + observeLock func(locker string, methodName string, pattern string, lockSucceeded bool), + observeUnlock func(locker string, methodName string, pattern string)) RuleLocker { return metricLocker{ - RuleLocker: ruleLocker, - lockerName: name, - observeLock: observeLock, + RuleLocker: ruleLocker, + lockerName: name, + observeLock: observeLock, + observeUnlockError: observeUnlock, } } type metricLocker struct { RuleLocker - lockerName string - observeLock func(locker string, methodName string, pattern string, lockSucceeded bool) + RuleLock + lockerName string + observeLock func(locker string, methodName string, pattern string, lockSucceeded bool) + observeUnlockError func(locker string, methodName string, pattern string) } func (ml metricLocker) Lock(key string, options ...Option) (RuleLock, error) { opts := buildOptions(options...) - lock, err := ml.RuleLocker.Lock(key, options...) + var err error + ml.RuleLock, err = ml.RuleLocker.Lock(key, options...) ml.observeLock(ml.lockerName, opts.method, opts.pattern, err == nil) - return lock, err + return ml.RuleLock, err +} + +func (ml metricLocker) Unlock(options ...Option) error { + opts := buildOptions(options...) + err := ml.RuleLock.Unlock(options...) + if err != nil { + ml.observeUnlockError(ml.lockerName, opts.method, opts.pattern) + } + return err } diff --git a/rules/lock/metrics_test.go b/rules/lock/metrics_test.go index 9e64e73..cc33b40 100644 --- a/rules/lock/metrics_test.go +++ b/rules/lock/metrics_test.go @@ -17,7 +17,7 @@ func Test_metricLocker_Lock(t *testing.T) { errUnlock := errors.New("unlock") errLock := errors.New("lock") mockLock := FuncMockLock{ - UnlockF: func() error { + UnlockF: func(_ ...Option) error { return errUnlock }, } @@ -77,13 +77,18 @@ func Test_metricLocker_Lock(t *testing.T) { return mockLock, tc.err }, } - observe := func(locker string, methodName string, pattern string, lockSucceeded bool) { + observeLock := func(locker string, methodName string, pattern string, lockSucceeded bool) { assert.Equal(t, tc.pattern, pattern) assert.Equal(t, tc.method, methodName) assert.Equal(t, tc.succeeded, lockSucceeded) } + observeUnlock := func(locker string, methodName string, pattern string) { + assert.Equal(t, tc.pattern, pattern) + assert.Equal(t, tc.method, methodName) + } ml := withMetrics(nested, testLockerName, - observe, + observeLock, + observeUnlock, ) lock, err := ml.Lock(testKey, tc.options...) if tc.err != nil { diff --git a/rules/lock/mock.go b/rules/lock/mock.go index 8908052..d602769 100644 --- a/rules/lock/mock.go +++ b/rules/lock/mock.go @@ -22,7 +22,7 @@ type mockLock struct { channel chan bool } -func (tl *mockLock) Unlock() error { +func (tl *mockLock) Unlock(options ...Option) error { tl.channel <- true return nil } @@ -39,10 +39,10 @@ func (ml FuncMockLocker) Lock(key string, options ...Option) (RuleLock, error) { // FuncMockLock instances are driven by functions that are provided. type FuncMockLock struct { - UnlockF func() error + UnlockF func(options ...Option) error } // Unlock is a mock implementation of RuleLock.Unlock -func (ml FuncMockLock) Unlock() error { - return ml.UnlockF() +func (ml FuncMockLock) Unlock(options ...Option) error { + return ml.UnlockF(options...) } diff --git a/rules/lock/nested_lock.go b/rules/lock/nested_lock.go index 62ffae7..65696c8 100644 --- a/rules/lock/nested_lock.go +++ b/rules/lock/nested_lock.go @@ -42,7 +42,7 @@ type nestedLock struct { nested RuleLock } -func (nl nestedLock) Unlock() error { +func (nl nestedLock) Unlock(_ ...Option) error { // Always unlock own lock, but after // nested lock. This prevents attempting // to get a new instance of the nested lock diff --git a/rules/lock/nested_lock_test.go b/rules/lock/nested_lock_test.go index b5588a5..f96cb57 100644 --- a/rules/lock/nested_lock_test.go +++ b/rules/lock/nested_lock_test.go @@ -16,7 +16,7 @@ func Test_nestedLocker_Lock(t *testing.T) { var ownUnlockCalled bool testOwnLock := testLock{ RuleLock: FuncMockLock{ - UnlockF: func() error { + UnlockF: func(_ ...Option) error { ownUnlockCalled = true return nil },