Skip to content

Commit

Permalink
Merge pull request #234 from cben/fix-inconsistent-prom-label
Browse files Browse the repository at this point in the history 
Fix panic from unexpected prometheus label
  • Loading branch information
Irit Goihman authored Aug 18, 2020
2 parents 3be7830 + d89101b commit b9d2a1d
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 1 deletion.
3 changes: 2 additions & 1 deletion CHANGES.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
This document describes the relevant changes between releases of the OCM API
SDK.

== 0.1.121 Aug 18 2020
== 0.1.121 Aug 18 2020 BROKEN

- Better logging and metrics when retrying SSO
BROKEN, DO NOT USE THIS VERSION: causes panic "inconsistent label cardinality".

== 0.1.120 Aug 13 2020

Expand Down
1 change: 1 addition & 0 deletions metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ const (

// Array of labels added to token metrics:
var tokenMetricsLabels = []string{
metricsAttemptLabel,
metricsCodeLabel,
}

Expand Down
43 changes: 43 additions & 0 deletions token_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (
. "github.com/onsi/gomega" // nolint

"github.com/onsi/gomega/ghttp"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
)

var _ = Describe("Tokens", func() {
Expand All @@ -38,6 +40,11 @@ var _ = Describe("Tokens", func() {
// Logger used during the tests:
var logger Logger

// Metrics subsystem - value doesn't matter but configuring it enables
// prometheus exporting, exercising the counter increment functionality
// (e.g. will catch inconsistent labels).
metrics := "test_subsystem"

BeforeEach(func() {
var err error

Expand Down Expand Up @@ -76,6 +83,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(refreshToken).
Expand Down Expand Up @@ -106,6 +114,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(refreshToken).
Expand Down Expand Up @@ -141,6 +150,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(expiredAccess, refreshToken).
Expand Down Expand Up @@ -171,6 +181,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(firstAccess, refreshToken).
Expand All @@ -191,6 +202,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(accessToken).
Expand All @@ -210,6 +222,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(accessToken).
Expand All @@ -229,6 +242,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(refreshToken).
Expand Down Expand Up @@ -264,6 +278,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(refreshToken).
Expand Down Expand Up @@ -302,6 +317,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(refreshToken).
Expand Down Expand Up @@ -340,6 +356,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(expiredAccess, refreshToken).
Expand Down Expand Up @@ -376,6 +393,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
User("myuser", "mypassword").
Expand Down Expand Up @@ -411,6 +429,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
User("myuser", "mypassword").
Expand Down Expand Up @@ -451,6 +470,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
User("myuser", "mypassword").
Expand Down Expand Up @@ -491,6 +511,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
User("myuser", "mypassword").
Expand Down Expand Up @@ -521,6 +542,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
User("baduser", "mypassword").
Expand All @@ -545,6 +567,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
User("myuser", "badpassword").
Expand Down Expand Up @@ -578,6 +601,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
User("myuser", "mypassword").
Expand Down Expand Up @@ -605,6 +629,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(accessToken).
Expand All @@ -626,6 +651,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(accessToken).
Expand Down Expand Up @@ -658,6 +684,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Client("myclient", "mysecret").
Expand Down Expand Up @@ -693,6 +720,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Client("myclient", "mysecret").
Expand Down Expand Up @@ -733,6 +761,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Client("myclient", "mysecret").
Expand Down Expand Up @@ -773,6 +802,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Client("myclient", "mysecret").
Expand Down Expand Up @@ -803,6 +833,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Client("badclient", "mysecret").
Expand All @@ -827,6 +858,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Client("myclient", "badsecret").
Expand Down Expand Up @@ -861,6 +893,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Client("myclient", "mysecret").
Expand Down Expand Up @@ -897,6 +930,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Client("myclient", "mysecret").
Expand Down Expand Up @@ -950,6 +984,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(refreshToken).
Expand All @@ -962,6 +997,13 @@ var _ = Describe("Tokens", func() {
Expect(err).ToNot(HaveOccurred())
Expect(returnedAccess).ToNot(BeEmpty())
Expect(returnedRefresh).ToNot(BeEmpty())

expectedLabels := prometheus.Labels{
"attempt": "2",
"code": "502",
}
counter := connection.tokenCountMetric.With(expectedLabels)
Expect(testutil.ToFloat64(counter)).To(Equal(1.0))
})
It("Test no retry when status is not http 5xx", func() {
// Generate tokens:
Expand Down Expand Up @@ -996,6 +1038,7 @@ var _ = Describe("Tokens", func() {
// Create the connection:
connection, err := NewConnectionBuilder().
Logger(logger).
Metrics(metrics).
TokenURL(oidServer.URL()).
URL(apiServer.URL()).
Tokens(refreshToken).
Expand Down

0 comments on commit b9d2a1d

Please sign in to comment.