Skip to content

Commit

Permalink
Merge pull request #479 from xmidt-org/denopink/patch/typo-replace-Wi…
Browse files Browse the repository at this point in the history 
…thLabelValues

patch: replace prometheus `WithLabelValues` with `With`
  • Loading branch information
@denopink
denopink authored Sep 20, 2024
2 parents 8f6cebe + bbf19e8 commit f0ef6c7
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 23 deletions.
28 changes: 14 additions & 14 deletions deviceAccess.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,19 @@ type talariaDeviceAccess struct {
logger *zap.Logger
}

func (t *talariaDeviceAccess) withFailure(labelValues ...string) prometheus.Counter {
func (t *talariaDeviceAccess) withFailure(reason string) prometheus.Counter {
if !t.strict {
return t.withSuccess(labelValues...)
return t.withSuccess(reason)
}
return t.wrpMessagesCounter.WithLabelValues(append(labelValues, outcomeLabel, rejected)...)
return t.wrpMessagesCounter.With(prometheus.Labels{reasonLabel: reason, outcomeLabel: rejected})
}

func (t *talariaDeviceAccess) withFatal(labelValues ...string) prometheus.Counter {
return t.wrpMessagesCounter.WithLabelValues(append(labelValues, outcomeLabel, rejected)...)
func (t *talariaDeviceAccess) withFatal(reason string) prometheus.Counter {
return t.wrpMessagesCounter.With(prometheus.Labels{reasonLabel: reason, outcomeLabel: rejected})
}

func (t *talariaDeviceAccess) withSuccess(labelValues ...string) prometheus.Counter {
return t.wrpMessagesCounter.WithLabelValues(append(labelValues, outcomeLabel, accepted)...)
func (t *talariaDeviceAccess) withSuccess(reason string) prometheus.Counter {
return t.wrpMessagesCounter.With(prometheus.Labels{reasonLabel: reason, outcomeLabel: accepted})
}

func getRight(check *parsedCheck, wrpCredentials *gojsonq.JSONQ) interface{} {
Expand All @@ -112,13 +112,13 @@ func getRight(check *parsedCheck, wrpCredentials *gojsonq.JSONQ) interface{} {
func (t *talariaDeviceAccess) authorizeWRP(_ context.Context, message *wrp.Message) error {
ID, err := device.ParseID(message.Destination)
if err != nil {
t.withFatal(reasonLabel, invalidWRPDest).Add(1)
t.withFatal(invalidWRPDest).Add(1)
return errInvalidWRPDestination
}

d, ok := t.deviceRegistry.Get(ID)
if !ok {
t.withFatal(reasonLabel, deviceNotFound).Add(1)
t.withFatal(deviceNotFound).Add(1)
return errDeviceNotFound
}
deviceCredentials := gojsonq.New(gojsonq.WithSeparator(t.sep)).FromInterface(d.Metadata().Claims())
Expand All @@ -128,7 +128,7 @@ func (t *talariaDeviceAccess) authorizeWRP(_ context.Context, message *wrp.Messa
left := deviceCredentials.Reset().Find(c.deviceCredentialPath)

if left == nil {
t.withFailure(reasonLabel, missingDeviceCredential).Add(1)
t.withFailure(missingDeviceCredential).Add(1)
if t.strict {
return errDeviceCredentialMissing
}
Expand All @@ -137,7 +137,7 @@ func (t *talariaDeviceAccess) authorizeWRP(_ context.Context, message *wrp.Messa

right := getRight(c, wrpCredentials)
if right == nil {
t.withFailure(reasonLabel, missingWRPCredential).Add(1)
t.withFailure(missingWRPCredential).Add(1)
if t.strict {
return errWRPCredentialsMissing
}
Expand All @@ -153,7 +153,7 @@ func (t *talariaDeviceAccess) authorizeWRP(_ context.Context, message *wrp.Messa
ok, err := c.assertion.evaluate(left, right)
if err != nil {
t.logger.Debug("Check failed to complete", zap.String("check", c.name), zap.Error(err))
t.withFailure(reasonLabel, incompleteCheck).Add(1)
t.withFailure(incompleteCheck).Add(1)

if t.strict {
return errIncompleteCheck
Expand All @@ -163,7 +163,7 @@ func (t *talariaDeviceAccess) authorizeWRP(_ context.Context, message *wrp.Messa

if !ok {
t.logger.Debug("WRP is unauthorized to reach device", zap.String("check", c.name))
t.withFailure(reasonLabel, denied).Add(1)
t.withFailure(denied).Add(1)

if t.strict {
return errDeniedDeviceAccess
Expand All @@ -173,6 +173,6 @@ func (t *talariaDeviceAccess) authorizeWRP(_ context.Context, message *wrp.Messa
}
}

t.withSuccess(reasonLabel, authorized).Add(1)
t.withSuccess(authorized).Add(1)
return nil
}
7 changes: 4 additions & 3 deletions deviceAccess_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"testing"

"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/xmidt-org/webpa-common/v2/device"
Expand Down Expand Up @@ -40,7 +41,7 @@ func testAuthorizeWRP(t *testing.T, testCases []deviceAccessTestCase, strict boo
mockDevice = new(device.MockDevice)
mockBinOp = new(mockBinOp)
testLogger = zaptest.NewLogger(t)
counter = mockCounter{labelPairs: make(map[string]string)}
counter = mockCounter{labelPairs: make(map[string]string), labels: []string{reasonLabel, outcomeLabel}}
expectedLabels = getLabelMaps(testCase.ExpectedError, testCase.IsFatal, strict, testCase.BaseLabelPairs)

wrpMsg = &wrp.Message{
Expand All @@ -66,7 +67,7 @@ func testAuthorizeWRP(t *testing.T, testCases []deviceAccessTestCase, strict boo
checks = getChecks(t, mockBinOp, testCase.IncompleteCheck, testCase.Authorized)
}

counter.On("WithLabelValues", []string{reasonLabel, invalidWRPDest, outcomeLabel, rejected}).Return().Once()
counter.On("With", expectedLabels).Return().Once()
counter.On("Add", 1.).Return().Once()
deviceAccessAuthority := &talariaDeviceAccess{
strict: strict,
Expand Down Expand Up @@ -166,7 +167,7 @@ func TestAuthorizeWRP(t *testing.T) {
})
}

func getLabelMaps(err error, isFatal, strict bool, baseLabelPairs map[string]string) map[string]string {
func getLabelMaps(err error, isFatal, strict bool, baseLabelPairs map[string]string) prometheus.Labels {
out := make(map[string]string)

for k, v := range baseLabelPairs {
Expand Down
19 changes: 13 additions & 6 deletions mocks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ type mockCounter struct {

// port over testCounter functionality
count float64
labelPairs map[string]string
labels []string
labelPairs prometheus.Labels
}

func (m *mockCounter) Add(delta float64) {
Expand All @@ -149,16 +150,18 @@ func (m *mockCounter) Add(delta float64) {

func (m *mockCounter) Inc() {}

func (m *mockCounter) With(labelValues prometheus.Labels) prometheus.Counter {
for k, v := range labelValues {
func (m *mockCounter) With(labelPairs prometheus.Labels) prometheus.Counter {
for k, v := range labelPairs {
if !utf8.ValidString(k) {
panic(fmt.Sprintf("key `%s`, value `%s`: key is not UTF-8", k, v))
} else if !utf8.ValidString(v) {
panic(fmt.Sprintf("key `%s`, value `%s`: value is not UTF-8", k, v))
}
}

m.Called(labelValues)
m.labelPairs = labelPairs
m.Called(labelPairs)

return m
}

Expand All @@ -180,8 +183,12 @@ func (m *mockCounter) MustCurryWith(labels prometheus.Labels) (c *prometheus.Cou

func (m *mockCounter) WithLabelValues(lvs ...string) (c prometheus.Counter) {
// port over testCounter functionality
for i := 0; i < len(lvs)-1; i += 2 {
m.labelPairs[lvs[i]] = lvs[i+1]
if len(lvs) != len(m.labels) {
panic(fmt.Sprintf("expected %d label values but got %d", len(m.labels), len(lvs)))
}

for i, l := range m.labels {
m.labelPairs[l] = lvs[i]
}

return m
Expand Down

0 comments on commit f0ef6c7

Please sign in to comment.