Skip to content

Commit

Permalink
Standardize on AssertMetricWithLabelsEquals (letsencrypt#5371)
Browse files Browse the repository at this point in the history
Update all of our tests to use `AssertMetricWithLabelsEquals`
instead of combinations of the older `CountFoo` helpers with
simple asserts. This coalesces all of our prometheus inspection
logic into a single function, allowing the deletion of four separate
helper functions.
  • Loading branch information
aarongable authored Apr 1, 2021
1 parent b4da43e commit ef1d3c4
Show file tree
Hide file tree
Showing 16 changed files with 100 additions and 177 deletions.
46 changes: 20 additions & 26 deletions bdns/dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ func TestRetry(t *testing.T) {
te *testExchanger
expected error
expectedCount int
metricsAllRetries int
metricsAllRetries float64
}
tests := []*testCase{
// The success on first try case
Expand Down Expand Up @@ -614,14 +614,12 @@ func TestRetry(t *testing.T) {
t.Errorf("#%d, error, expectedCount %v, got %v", i, tc.expectedCount, tc.te.count)
}
if tc.metricsAllRetries > 0 {
count := test.CountCounter(dr.timeoutCounter.With(prometheus.Labels{
"qtype": "TXT",
"type": "out of retries",
"resolver": dnsLoopbackHost,
}))
if count != tc.metricsAllRetries {
t.Errorf("wrong count for timeoutCounter: got %d, expected %d", count, tc.metricsAllRetries)
}
test.AssertMetricWithLabelsEquals(
t, dr.timeoutCounter, prometheus.Labels{
"qtype": "TXT",
"type": "out of retries",
"resolver": dnsLoopbackHost,
}, tc.metricsAllRetries)
}
}

Expand Down Expand Up @@ -654,23 +652,19 @@ func TestRetry(t *testing.T) {
t.Errorf("expected %s, got %s", context.DeadlineExceeded, err)
}

count := test.CountCounter(dr.timeoutCounter.With(prometheus.Labels{
"qtype": "TXT",
"type": "canceled",
"resolver": dnsLoopbackHost,
}))
if count != 1 {
t.Errorf("wrong count for timeoutCounter canceled: got %d, expected %d", count, 1)
}

count = test.CountCounter(dr.timeoutCounter.With(prometheus.Labels{
"qtype": "TXT",
"type": "deadline exceeded",
"resolver": dnsLoopbackHost,
}))
if count != 2 {
t.Errorf("wrong count for timeoutCounter deadline exceeded: got %d, expected %d", count, 2)
}
test.AssertMetricWithLabelsEquals(
t, dr.timeoutCounter, prometheus.Labels{
"qtype": "TXT",
"type": "canceled",
"resolver": dnsLoopbackHost,
}, 1)

test.AssertMetricWithLabelsEquals(
t, dr.timeoutCounter, prometheus.Labels{
"qtype": "TXT",
"type": "deadline exceeded",
"resolver": dnsLoopbackHost,
}, 2)
}

type tempError bool
Expand Down
8 changes: 4 additions & 4 deletions ca/ca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -990,8 +990,8 @@ func TestPrecertOrphanQueue(t *testing.T) {
strings.Join(testCtx.logger.GetAllMatching(".*"), "\n"))
}

orphanCount := test.CountCounterVec("type", "precert", ca.orphanCount)
test.AssertEquals(t, orphanCount, 1)
test.AssertMetricWithLabelsEquals(
t, ca.orphanCount, prometheus.Labels{"type": "precert"}, 1)

qsa.fail = false
err = ca.integrateOrphan()
Expand All @@ -1004,8 +1004,8 @@ func TestPrecertOrphanQueue(t *testing.T) {
t.Fatalf("Unexpected error, wanted %q, got %q", goque.ErrEmpty, err)
}

adoptedCount := test.CountCounterVec("type", "precert", ca.adoptedOrphanCount)
test.AssertEquals(t, adoptedCount, 1)
test.AssertMetricWithLabelsEquals(
t, ca.adoptedOrphanCount, prometheus.Labels{"type": "precert"}, 1)
}

func TestOrphanQueue(t *testing.T) {
Expand Down
15 changes: 9 additions & 6 deletions cmd/boulder-janitor/janitor/job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/letsencrypt/boulder/db"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/test"
"github.com/prometheus/client_golang/prometheus"
)

func setup() (*blog.Mock, clock.FakeClock) {
Expand Down Expand Up @@ -136,8 +137,8 @@ LIMIT :limit`
}

// We expect the work gauge for this table has been updated
workCount := test.CountCounterVec("table", table, workStat)
test.AssertEquals(t, workCount, len(mockIDs))
test.AssertMetricWithLabelsEquals(
t, workStat, prometheus.Labels{"table": table}, float64(len(mockIDs)))

// Set the third item in mockIDs to have an expiry after the purge cutoff
// so we expect to only get the first two items returned from getWork
Expand All @@ -153,8 +154,8 @@ LIMIT :limit`
got := <-workChan
test.AssertEquals(t, got, mockIDs[i].ID)
}
workCount = test.CountCounterVec("table", table, workStat)
test.AssertEquals(t, workCount, 2)
test.AssertMetricWithLabelsEquals(
t, workStat, prometheus.Labels{"table": table}, 2)
}

func TestDeleteResource(t *testing.T) {
Expand Down Expand Up @@ -185,14 +186,16 @@ func TestDeleteResource(t *testing.T) {
// We expect an err result back
test.AssertError(t, err, "no error returned from deleteHandler with bad DB")
// We expect no deletes to have been tracked in the deletedStat
test.AssertEquals(t, test.CountCounterVec("table", "certificates", deletedStat), 0)
test.AssertMetricWithLabelsEquals(
t, deletedStat, prometheus.Labels{"table": "certificates"}, 0)

// With the mock error removed we expect no error returned from simpleDeleteResource
testDB.errResult = nil
err = job.deleteHandler(job, testID)
test.AssertNotError(t, err, "unexpected error from deleteHandler")
// We expect a delete to have been tracked in the deletedStat
test.AssertEquals(t, test.CountCounterVec("table", "certificates", deletedStat), 1)
test.AssertMetricWithLabelsEquals(
t, deletedStat, prometheus.Labels{"table": "certificates"}, 1)
}

type slowDB struct{}
Expand Down
6 changes: 2 additions & 4 deletions cmd/ocsp-updater/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/letsencrypt/boulder/sa/satest"
"github.com/letsencrypt/boulder/test"
"github.com/letsencrypt/boulder/test/vars"
"github.com/prometheus/client_golang/prometheus"
"google.golang.org/grpc"
)

Expand Down Expand Up @@ -114,10 +115,7 @@ func TestStalenessHistogram(t *testing.T) {
test.AssertNotError(t, err, "Couldn't find stale responses")
test.AssertEquals(t, len(statuses), 2)

samples := test.CountHistogramSamples(updater.stalenessHistogram)
if samples != 2 {
t.Errorf("Wrong number of samples for invalid validation. Expected 1, got %d", samples)
}
test.AssertMetricWithLabelsEquals(t, updater.stalenessHistogram, prometheus.Labels{}, 2)
}

func TestGenerateAndStoreOCSPResponse(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions ctpolicy/ctpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ func TestGetSCTsMetrics(t *testing.T) {
}, nil, blog.NewMock(), metrics.NoopRegisterer)
_, err := ctp.GetSCTs(context.Background(), []byte{0}, time.Time{})
test.AssertNotError(t, err, "GetSCTs failed")
test.AssertEquals(t, test.CountCounter(ctp.winnerCounter.With(prometheus.Labels{"log": "ghi", "group": "a"})), 1)
test.AssertEquals(t, test.CountCounter(ctp.winnerCounter.With(prometheus.Labels{"log": "ghi", "group": "b"})), 1)
test.AssertMetricWithLabelsEquals(t, ctp.winnerCounter, prometheus.Labels{"log": "ghi", "group": "a"}, 1)
test.AssertMetricWithLabelsEquals(t, ctp.winnerCounter, prometheus.Labels{"log": "ghi", "group": "b"}, 1)
}

func TestGetSCTsFailMetrics(t *testing.T) {
Expand All @@ -194,7 +194,7 @@ func TestGetSCTsFailMetrics(t *testing.T) {
if err == nil {
t.Fatal("GetSCTs should have failed")
}
test.AssertEquals(t, test.CountCounter(ctp.winnerCounter.With(prometheus.Labels{"log": "all_failed", "group": "a"})), 1)
test.AssertMetricWithLabelsEquals(t, ctp.winnerCounter, prometheus.Labels{"log": "all_failed", "group": "a"}, 1)

// Same thing, but for when an entire log group times out.
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
Expand All @@ -212,7 +212,7 @@ func TestGetSCTsFailMetrics(t *testing.T) {
if err == nil {
t.Fatal("GetSCTs should have failed")
}
test.AssertEquals(t, test.CountCounter(ctp.winnerCounter.With(prometheus.Labels{"log": "timeout", "group": "a"})), 1)
test.AssertMetricWithLabelsEquals(t, ctp.winnerCounter, prometheus.Labels{"log": "timeout", "group": "a"}, 1)
}

// A mock publisher that counts submissions
Expand Down
14 changes: 3 additions & 11 deletions grpc/interceptors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,7 @@ func TestRequestTimeTagging(t *testing.T) {
}

// There should be one histogram sample in the serverInterceptor rpcLag stat
count := test.CountHistogramSamples(si.metrics.rpcLag)
test.AssertEquals(t, count, 1)
test.AssertMetricWithLabelsEquals(t, si.metrics.rpcLag, prometheus.Labels{}, 1)
}

// blockedServer implements a ChillerServer with a Chill method that:
Expand Down Expand Up @@ -322,21 +321,14 @@ func TestInFlightRPCStat(t *testing.T) {
"method": "Chill",
}

// Retrieve the gauge for inflight Chiller.Chill RPCs
inFlightCount, err := test.GaugeValueWithLabels(ci.metrics.inFlightRPCs, labels)
test.AssertNotError(t, err, "Error collecting gauge value for inFlightRPCs")
// We expect the inFlightRPCs gauge for the Chiller.Chill RPCs to be equal to numRPCs.
test.AssertEquals(t, inFlightCount, numRPCs)
test.AssertMetricWithLabelsEquals(t, ci.metrics.inFlightRPCs, labels, float64(numRPCs))

// Unblock the blockedServer to let all of the Chiller.Chill RPCs complete
server.roadblock.Done()
// Sleep for a little bit to let all the RPCs complete
time.Sleep(1 * time.Second)

// Check the gauge value again
inFlightCount, err = test.GaugeValueWithLabels(ci.metrics.inFlightRPCs, labels)
test.AssertNotError(t, err, "Error collecting gauge value for inFlightRPCs")
// There should now be zero in flight chill requests.
// What a ~ ~ Chill Sitch ~ ~
test.AssertEquals(t, inFlightCount, 0)
test.AssertMetricWithLabelsEquals(t, ci.metrics.inFlightRPCs, labels, 0)
}
11 changes: 5 additions & 6 deletions ocsp/responder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,16 @@ func TestOCSP(t *testing.T) {
t.Errorf("Incorrect response code: got %d, wanted %d", rw.Code, tc.expected)
}
if rw.Code == http.StatusOK {
test.AssertEquals(t, 1, test.CountCounterVec("type", "Success", responder.responseTypes))
test.AssertMetricWithLabelsEquals(
t, responder.responseTypes, prometheus.Labels{"type": "Success"}, 1)
} else if rw.Code == http.StatusBadRequest {
test.AssertEquals(t, 1, test.CountCounterVec("type", "Malformed", responder.responseTypes))
test.AssertMetricWithLabelsEquals(
t, responder.responseTypes, prometheus.Labels{"type": "Malformed"}, 1)
}
})
}
// Exactly two of the cases above result in an OCSP response being sent.
samples := test.CountHistogramSamples(responder.responseAges)
if samples != 2 {
t.Errorf("Ages histogram updated incorrect number of times: %d", samples)
}
test.AssertMetricWithLabelsEquals(t, responder.responseAges, prometheus.Labels{}, 2)
}

func TestRequestTooBig(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions publisher/publisher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,11 @@ func TestHTTPStatusMetric(t *testing.T) {
Der: leaf.Raw,
})
test.AssertError(t, err, "SubmitToSingleCTWithResult didn't fail")
test.AssertEquals(t, test.CountHistogramSamples(pub.metrics.submissionLatency.With(prometheus.Labels{
test.AssertMetricWithLabelsEquals(t, pub.metrics.submissionLatency, prometheus.Labels{
"log": logURI,
"status": "error",
"http_status": "400",
})), 1)
}, 1)

pub, leaf, k = setup(t)
pkDER, err = x509.MarshalPKIXPublicKey(&k.PublicKey)
Expand All @@ -368,11 +368,11 @@ func TestHTTPStatusMetric(t *testing.T) {
Der: leaf.Raw,
})
test.AssertNotError(t, err, "SubmitToSingleCTWithResult failed")
test.AssertEquals(t, test.CountHistogramSamples(pub.metrics.submissionLatency.With(prometheus.Labels{
test.AssertMetricWithLabelsEquals(t, pub.metrics.submissionLatency, prometheus.Labels{
"log": logURI,
"status": "success",
"http_status": "",
})), 1)
}, 1)
}
func Test_GetCTBundleForChain(t *testing.T) {
chain, err := issuance.LoadChain([]string{
Expand Down
14 changes: 7 additions & 7 deletions ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3488,7 +3488,7 @@ func TestCTPolicyMeasurements(t *testing.T) {
CSR: ExampleCSR,
}, accountID(Registration.ID), 0, 0)
test.AssertError(t, err, "ra.issueCertificate didn't fail when CTPolicy.GetSCTs timed out")
test.AssertEquals(t, test.CountHistogramSamples(ra.ctpolicyResults.With(prometheus.Labels{"result": "failure"})), 1)
test.AssertMetricWithLabelsEquals(t, ra.ctpolicyResults, prometheus.Labels{"result": "failure"}, 1)
}

func TestWildcardOverlap(t *testing.T) {
Expand Down Expand Up @@ -3894,17 +3894,17 @@ func TestRevocationAddBlockedKey(t *testing.T) {
err = ra.RevokeCertificateWithReg(context.Background(), *cert, ocsp.Unspecified, 0)
test.AssertNotError(t, err, "RevokeCertificateWithReg failed")
test.Assert(t, mockSA.added == nil, "blocked key was added when reason was not keyCompromise")
test.AssertEquals(t, test.CountCounterVec(
"reason", "unspecified", ra.revocationReasonCounter), 1)
test.AssertMetricWithLabelsEquals(
t, ra.revocationReasonCounter, prometheus.Labels{"reason": "unspecified"}, 1)

err = ra.RevokeCertificateWithReg(context.Background(), *cert, ocsp.KeyCompromise, 0)
test.AssertNotError(t, err, "RevokeCertificateWithReg failed")
test.Assert(t, mockSA.added != nil, "blocked key was not added when reason was keyCompromise")
test.Assert(t, bytes.Equal(digest[:], mockSA.added.KeyHash), "key hash mismatch")
test.AssertEquals(t, mockSA.added.Source, "API")
test.Assert(t, mockSA.added.Comment == "", "Comment is not empty")
test.AssertEquals(t, test.CountCounterVec(
"reason", "keyCompromise", ra.revocationReasonCounter), 1)
test.AssertMetricWithLabelsEquals(
t, ra.revocationReasonCounter, prometheus.Labels{"reason": "keyCompromise"}, 1)

mockSA.added = nil
err = ra.AdministrativelyRevokeCertificate(context.Background(), *cert, ocsp.KeyCompromise, "root")
Expand All @@ -3914,6 +3914,6 @@ func TestRevocationAddBlockedKey(t *testing.T) {
test.AssertEquals(t, mockSA.added.Source, "admin-revoker")
test.Assert(t, mockSA.added.Comment != "", "Comment is nil")
test.AssertEquals(t, mockSA.added.Comment, "revoked by root")
test.AssertEquals(t, test.CountCounterVec(
"reason", "keyCompromise", ra.revocationReasonCounter), 2)
test.AssertMetricWithLabelsEquals(
t, ra.revocationReasonCounter, prometheus.Labels{"reason": "keyCompromise"}, 2)
}
57 changes: 0 additions & 57 deletions test/test-tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,63 +204,6 @@ loop:
AssertEquals(t, total, expected)
}

// CountCounterVec returns the count by label and value of a prometheus metric
func CountCounterVec(labelName string, value string, counterVec *prometheus.CounterVec) int {
return CountCounter(counterVec.With(prometheus.Labels{labelName: value}))
}

// CountCounter returns the count by label and value of a prometheus metric
func CountCounter(counter prometheus.Counter) int {
ch := make(chan prometheus.Metric, 10)
counter.Collect(ch)
var m prometheus.Metric
select {
case <-time.After(time.Second):
panic("timed out collecting metrics")
case m = <-ch:
}
var iom io_prometheus_client.Metric
_ = m.Write(&iom)
return int(iom.Counter.GetValue())
}

func CountHistogramSamples(obs prometheus.Observer) int {
hist := obs.(prometheus.Histogram)
ch := make(chan prometheus.Metric, 10)
hist.Collect(ch)
var m prometheus.Metric
select {
case <-time.After(time.Second):
panic("timed out collecting metrics")
case m = <-ch:
}
var iom io_prometheus_client.Metric
_ = m.Write(&iom)
return int(iom.Histogram.GetSampleCount())
}

// GaugeValueWithLabels returns the current value with the provided labels from the
// the GaugeVec argument, or an error if there was a problem collecting the value.
func GaugeValueWithLabels(vecGauge *prometheus.GaugeVec, labels prometheus.Labels) (int, error) {
gauge, err := vecGauge.GetMetricWith(labels)
if err != nil {
return 0, err
}

ch := make(chan prometheus.Metric, 10)
gauge.Collect(ch)
var m prometheus.Metric
select {
case <-time.After(time.Second):
return 0, fmt.Errorf("timed out collecting gauge metrics")
case m = <-ch:
}
var iom io_prometheus_client.Metric
_ = m.Write(&iom)

return int(iom.Gauge.GetValue()), nil
}

var throwawayCertIssuer *x509.Certificate

// ThrowAwayCert is a small test helper function that creates a self-signed
Expand Down
7 changes: 2 additions & 5 deletions va/dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,11 @@ func TestDNSValidationEmpty(t *testing.T) {
test.AssertEquals(t, res.Problems.ProblemType, "unauthorized")
test.AssertEquals(t, res.Problems.Detail, "No TXT record found at _acme-challenge.empty-txts.com")

samples := test.CountHistogramSamples(va.metrics.validationTime.With(prometheus.Labels{
test.AssertMetricWithLabelsEquals(t, va.metrics.validationTime, prometheus.Labels{
"type": "dns-01",
"result": "invalid",
"problem_type": "unauthorized",
}))
if samples != 1 {
t.Errorf("Wrong number of samples for invalid validation. Expected 1, got %d", samples)
}
}, 1)
}

func TestDNSValidationWrong(t *testing.T) {
Expand Down
Loading

0 comments on commit ef1d3c4

Please sign in to comment.