diff --git a/ratelimits/limit.go b/ratelimits/limit.go
index 16dc65ac962..72fc3a1a555 100644
--- a/ratelimits/limit.go
+++ b/ratelimits/limit.go
@@ -69,13 +69,8 @@ type limit struct {
// precomputed to avoid doing the same calculation on every request.
burstOffset int64
- // overrideKey is the key used to look up this limit in the overrides map.
- overrideKey string
-}
-
-// isOverride returns true if the limit is an override.
-func (l *limit) isOverride() bool {
- return l.overrideKey != ""
+ // isOverride is true if the limit is an override.
+ isOverride bool
}
// precompute calculates the emissionInterval and burstOffset for the limit.
@@ -178,11 +173,13 @@ func parseOverrideLimits(newOverridesYAML overridesYAML) (limits, error) {
}
lim := &limit{
- burst: v.Burst,
- count: v.Count,
- period: v.Period,
- name: name,
+ burst: v.Burst,
+ count: v.Count,
+ period: v.Period,
+ name: name,
+ isOverride: true,
}
+ lim.precompute()
err := validateLimit(lim)
if err != nil {
@@ -196,14 +193,12 @@ func parseOverrideLimits(newOverridesYAML overridesYAML) (limits, error) {
return nil, fmt.Errorf(
"validating name %s and id %q for override limit %q: %w", name, id, k, err)
}
- lim.overrideKey = joinWithColon(name.EnumString(), id)
if name == CertificatesPerFQDNSet {
// FQDNSet hashes are not a nice thing to ask for in a
// config file, so we allow the user to specify a
// comma-separated list of FQDNs and compute the hash here.
id = fmt.Sprintf("%x", core.HashNames(strings.Split(id, ",")))
}
- lim.precompute()
parsed[joinWithColon(name.EnumString(), id)] = lim
}
}
diff --git a/ratelimits/limiter.go b/ratelimits/limiter.go
index ef119d1819a..f886cdc482a 100644
--- a/ratelimits/limiter.go
+++ b/ratelimits/limiter.go
@@ -37,8 +37,7 @@ type Limiter struct {
source Source
clk clock.Clock
- spendLatency *prometheus.HistogramVec
- overrideUsageGauge *prometheus.GaugeVec
+ spendLatency *prometheus.HistogramVec
}
// NewLimiter returns a new *Limiter. The provided source must be safe for
@@ -52,17 +51,10 @@ func NewLimiter(clk clock.Clock, source Source, stats prometheus.Registerer) (*L
}, []string{"limit", "decision"})
stats.MustRegister(spendLatency)
- overrideUsageGauge := prometheus.NewGaugeVec(prometheus.GaugeOpts{
- Name: "ratelimits_override_usage",
- Help: "Proportion of override limit used, by limit name and bucket key.",
- }, []string{"limit", "bucket_key"})
- stats.MustRegister(overrideUsageGauge)
-
return &Limiter{
- source: source,
- clk: clk,
- spendLatency: spendLatency,
- overrideUsageGauge: overrideUsageGauge,
+ source: source,
+ clk: clk,
+ spendLatency: spendLatency,
}, nil
}
@@ -284,11 +276,6 @@ func (l *Limiter) BatchSpend(ctx context.Context, txns []Transaction) (*Decision
storedTAT, bucketExists := tats[txn.bucketKey]
d := maybeSpend(l.clk, txn, storedTAT)
- if txn.limit.isOverride() {
- utilization := float64(txn.limit.burst-d.remaining) / float64(txn.limit.burst)
- l.overrideUsageGauge.WithLabelValues(txn.limit.name.String(), txn.limit.overrideKey).Set(utilization)
- }
-
if d.allowed && (storedTAT != d.newTAT) && txn.spend {
if !bucketExists {
newBuckets[txn.bucketKey] = d.newTAT
diff --git a/ratelimits/limiter_test.go b/ratelimits/limiter_test.go
index 902f4c13435..5ccb7dfa0a9 100644
--- a/ratelimits/limiter_test.go
+++ b/ratelimits/limiter_test.go
@@ -8,7 +8,6 @@ import (
"time"
"github.com/jmhodges/clock"
- "github.com/prometheus/client_golang/prometheus"
"github.com/letsencrypt/boulder/config"
berrors "github.com/letsencrypt/boulder/errors"
@@ -60,12 +59,6 @@ func TestLimiter_CheckWithLimitOverrides(t *testing.T) {
testCtx, limiters, txnBuilder, clk, testIP := setup(t)
for name, l := range limiters {
t.Run(name, func(t *testing.T) {
- // Verify our overrideUsageGauge is being set correctly. 0.0 == 0%
- // of the bucket has been consumed.
- test.AssertMetricWithLabelsEquals(t, l.overrideUsageGauge, prometheus.Labels{
- "limit": NewRegistrationsPerIPAddress.String(),
- "bucket_key": joinWithColon(NewRegistrationsPerIPAddress.EnumString(), tenZeroZeroTwo)}, 0)
-
overriddenBucketKey, err := newIPAddressBucketKey(NewRegistrationsPerIPAddress, net.ParseIP(tenZeroZeroTwo))
test.AssertNotError(t, err, "should not error")
overriddenLimit, err := txnBuilder.getLimit(NewRegistrationsPerIPAddress, overriddenBucketKey)
@@ -87,12 +80,6 @@ func TestLimiter_CheckWithLimitOverrides(t *testing.T) {
test.AssertEquals(t, d.remaining, int64(0))
test.AssertEquals(t, d.resetIn, time.Second)
- // Verify our overrideUsageGauge is being set correctly. 1.0 == 100%
- // of the bucket has been consumed.
- test.AssertMetricWithLabelsEquals(t, l.overrideUsageGauge, prometheus.Labels{
- "limit_name": NewRegistrationsPerIPAddress.String(),
- "bucket_key": joinWithColon(NewRegistrationsPerIPAddress.EnumString(), tenZeroZeroTwo)}, 1.0)
-
// Verify our RetryIn is correct. 1 second == 1000 milliseconds and
// 1000/40 = 25 milliseconds per request.
test.AssertEquals(t, d.retryIn, time.Millisecond*25)
diff --git a/ratelimits/source_redis.go b/ratelimits/source_redis.go
index 4d32f7c2a6d..ff32931efc2 100644
--- a/ratelimits/source_redis.go
+++ b/ratelimits/source_redis.go
@@ -99,10 +99,6 @@ func (r *RedisSource) BatchSet(ctx context.Context, buckets map[string]time.Time
}
totalLatency := r.clk.Since(start)
- perSetLatency := totalLatency / time.Duration(len(buckets))
- for range buckets {
- r.observeLatency("batchset_entry", perSetLatency, nil)
- }
r.observeLatency("batchset", totalLatency, nil)
return nil
@@ -128,17 +124,14 @@ func (r *RedisSource) BatchSetNotExisting(ctx context.Context, buckets map[strin
alreadyExists := make(map[string]bool, len(buckets))
totalLatency := r.clk.Since(start)
- perSetLatency := totalLatency / time.Duration(len(buckets))
for bucketKey, cmd := range cmds {
success, err := cmd.Result()
if err != nil {
- r.observeLatency("batchsetnotexisting_entry", perSetLatency, err)
return nil, err
}
if !success {
alreadyExists[bucketKey] = true
}
- r.observeLatency("batchsetnotexisting_entry", perSetLatency, nil)
}
r.observeLatency("batchsetnotexisting", totalLatency, nil)
@@ -163,11 +156,6 @@ func (r *RedisSource) BatchIncrement(ctx context.Context, buckets map[string]inc
}
totalLatency := r.clk.Since(start)
- perSetLatency := totalLatency / time.Duration(len(buckets))
- for range buckets {
- r.observeLatency("batchincrby_entry", perSetLatency, nil)
- }
-
r.observeLatency("batchincrby", totalLatency, nil)
return nil
}
@@ -211,7 +199,6 @@ func (r *RedisSource) BatchGet(ctx context.Context, bucketKeys []string) (map[st
}
totalLatency := r.clk.Since(start)
- perEntryLatency := totalLatency / time.Duration(len(bucketKeys))
tats := make(map[string]time.Time, len(bucketKeys))
notFoundCount := 0
@@ -224,13 +211,10 @@ func (r *RedisSource) BatchGet(ctx context.Context, bucketKeys []string) (map[st
r.observeLatency("batchget", r.clk.Since(start), err)
return nil, err
}
- // Bucket key does not exist.
- r.observeLatency("batchget_entry", perEntryLatency, err)
notFoundCount++
continue
}
tats[bucketKeys[i]] = time.Unix(0, tatNano).UTC()
- r.observeLatency("batchget_entry", perEntryLatency, nil)
}
var batchErr error
diff --git a/ratelimits/transaction.go b/ratelimits/transaction.go
index b5fd1653269..fa4b5e88715 100644
--- a/ratelimits/transaction.go
+++ b/ratelimits/transaction.go
@@ -403,7 +403,7 @@ func (builder *TransactionBuilder) certificatesPerDomainCheckOnlyTransactions(re
return nil, err
}
if accountOverride {
- if !perAccountLimit.isOverride() {
+ if !perAccountLimit.isOverride {
return nil, fmt.Errorf("shouldn't happen: CertificatesPerDomainPerAccount limit is not an override")
}
perAccountPerDomainKey, err := NewRegIdDomainBucketKey(CertificatesPerDomainPerAccount, regId, name)
@@ -481,7 +481,7 @@ func (builder *TransactionBuilder) CertificatesPerDomainSpendOnlyTransactions(re
return nil, err
}
if accountOverride {
- if !perAccountLimit.isOverride() {
+ if !perAccountLimit.isOverride {
return nil, fmt.Errorf("shouldn't happen: CertificatesPerDomainPerAccount limit is not an override")
}
perAccountPerDomainKey, err := NewRegIdDomainBucketKey(CertificatesPerDomainPerAccount, regId, name)
diff --git a/ratelimits/transaction_test.go b/ratelimits/transaction_test.go
index 8cf0b798a1e..b4a25e837f0 100644
--- a/ratelimits/transaction_test.go
+++ b/ratelimits/transaction_test.go
@@ -79,14 +79,14 @@ func TestFailedAuthorizationsPerDomainPerAccountTransactions(t *testing.T) {
test.AssertEquals(t, len(txns), 1)
test.AssertEquals(t, txns[0].bucketKey, "4:123456789:so.many.labels.here.example.com")
test.Assert(t, txns[0].checkOnly(), "should be check-only")
- test.Assert(t, !txns[0].limit.isOverride(), "should not be an override")
+ test.Assert(t, !txns[0].limit.isOverride, "should not be an override")
// A spend-only transaction for the default per-account limit.
txn, err := tb.FailedAuthorizationsPerDomainPerAccountSpendOnlyTransaction(123456789, "so.many.labels.here.example.com")
test.AssertNotError(t, err, "creating transaction")
test.AssertEquals(t, txn.bucketKey, "4:123456789:so.many.labels.here.example.com")
test.Assert(t, txn.spendOnly(), "should be spend-only")
- test.Assert(t, !txn.limit.isOverride(), "should not be an override")
+ test.Assert(t, !txn.limit.isOverride, "should not be an override")
// A check-only transaction for the per-account limit override.
txns, err = tb.FailedAuthorizationsPerDomainPerAccountCheckOnlyTransactions(13371338, []string{"so.many.labels.here.example.com"})
@@ -94,14 +94,14 @@ func TestFailedAuthorizationsPerDomainPerAccountTransactions(t *testing.T) {
test.AssertEquals(t, len(txns), 1)
test.AssertEquals(t, txns[0].bucketKey, "4:13371338:so.many.labels.here.example.com")
test.Assert(t, txns[0].checkOnly(), "should be check-only")
- test.Assert(t, txns[0].limit.isOverride(), "should be an override")
+ test.Assert(t, txns[0].limit.isOverride, "should be an override")
// A spend-only transaction for the per-account limit override.
txn, err = tb.FailedAuthorizationsPerDomainPerAccountSpendOnlyTransaction(13371338, "so.many.labels.here.example.com")
test.AssertNotError(t, err, "creating transaction")
test.AssertEquals(t, txn.bucketKey, "4:13371338:so.many.labels.here.example.com")
test.Assert(t, txn.spendOnly(), "should be spend-only")
- test.Assert(t, txn.limit.isOverride(), "should be an override")
+ test.Assert(t, txn.limit.isOverride, "should be an override")
}
func TestFailedAuthorizationsForPausingPerDomainPerAccountTransactions(t *testing.T) {
@@ -115,7 +115,7 @@ func TestFailedAuthorizationsForPausingPerDomainPerAccountTransactions(t *testin
test.AssertNotError(t, err, "creating transaction")
test.AssertEquals(t, txn.bucketKey, "8:13371338:so.many.labels.here.example.com")
test.Assert(t, txn.check && txn.spend, "should be check and spend")
- test.Assert(t, txn.limit.isOverride(), "should be an override")
+ test.Assert(t, txn.limit.isOverride, "should be an override")
}
func TestCertificatesPerDomainTransactions(t *testing.T) {
@@ -153,7 +153,7 @@ func TestCertificatesPerDomainPerAccountTransactions(t *testing.T) {
test.AssertEquals(t, len(txns), 1)
test.AssertEquals(t, txns[0].bucketKey, "6:13371338:example.com")
test.Assert(t, txns[0].checkOnly(), "should be check-only")
- test.Assert(t, txns[0].limit.isOverride(), "should be an override")
+ test.Assert(t, txns[0].limit.isOverride, "should be an override")
// Same as above, but with multiple example.com domains.
txns, err = tb.certificatesPerDomainCheckOnlyTransactions(13371338, []string{"so.many.labels.here.example.com", "z.example.com"})
@@ -161,7 +161,7 @@ func TestCertificatesPerDomainPerAccountTransactions(t *testing.T) {
test.AssertEquals(t, len(txns), 1)
test.AssertEquals(t, txns[0].bucketKey, "6:13371338:example.com")
test.Assert(t, txns[0].checkOnly(), "should be check-only")
- test.Assert(t, txns[0].limit.isOverride(), "should be an override")
+ test.Assert(t, txns[0].limit.isOverride, "should be an override")
// Same as above, but with different domains.
txns, err = tb.certificatesPerDomainCheckOnlyTransactions(13371338, []string{"so.many.labels.here.example.com", "z.example.net"})
@@ -170,10 +170,10 @@ func TestCertificatesPerDomainPerAccountTransactions(t *testing.T) {
test.AssertEquals(t, len(txns), 2)
test.AssertEquals(t, txns[0].bucketKey, "6:13371338:example.com")
test.Assert(t, txns[0].checkOnly(), "should be check-only")
- test.Assert(t, txns[0].limit.isOverride(), "should be an override")
+ test.Assert(t, txns[0].limit.isOverride, "should be an override")
test.AssertEquals(t, txns[1].bucketKey, "6:13371338:example.net")
test.Assert(t, txns[1].checkOnly(), "should be check-only")
- test.Assert(t, txns[1].limit.isOverride(), "should be an override")
+ test.Assert(t, txns[1].limit.isOverride, "should be an override")
// Two spend-only transactions, one for the global limit and one for the
// per-account limit override.
@@ -183,11 +183,11 @@ func TestCertificatesPerDomainPerAccountTransactions(t *testing.T) {
txns = sortTransactions(txns)
test.AssertEquals(t, txns[0].bucketKey, "5:example.com")
test.Assert(t, txns[0].spendOnly(), "should be spend-only")
- test.Assert(t, !txns[0].limit.isOverride(), "should not be an override")
+ test.Assert(t, !txns[0].limit.isOverride, "should not be an override")
test.AssertEquals(t, txns[1].bucketKey, "6:13371338:example.com")
test.Assert(t, txns[1].spendOnly(), "should be spend-only")
- test.Assert(t, txns[1].limit.isOverride(), "should be an override")
+ test.Assert(t, txns[1].limit.isOverride, "should be an override")
}
func TestCertificatesPerFQDNSetTransactions(t *testing.T) {
@@ -202,7 +202,7 @@ func TestCertificatesPerFQDNSetTransactions(t *testing.T) {
namesHash := fmt.Sprintf("%x", core.HashNames([]string{"example.com", "example.net", "example.org"}))
test.AssertEquals(t, txn.bucketKey, "7:"+namesHash)
test.Assert(t, txn.checkOnly(), "should be check-only")
- test.Assert(t, !txn.limit.isOverride(), "should not be an override")
+ test.Assert(t, !txn.limit.isOverride, "should not be an override")
}
func TestNewTransactionBuilder(t *testing.T) {