diff --git a/ratelimits/bucket.go b/ratelimits/bucket.go index f4698770c..ba555c2db 100644 --- a/ratelimits/bucket.go +++ b/ratelimits/bucket.go @@ -344,7 +344,7 @@ func (builder *TransactionBuilder) CertificatesPerDomainTransactions(regId int64 if err != nil { return nil, err } - if perAccountLimit.isOverride { + if perAccountLimit.isOverride() { // An override is configured for the CertificatesPerDomainPerAccount // limit. perAccountPerDomainKey, err := newRegIdDomainBucketKey(CertificatesPerDomainPerAccount, regId, name) diff --git a/ratelimits/gcra_test.go b/ratelimits/gcra_test.go index aa7557044..c1ebcf53c 100644 --- a/ratelimits/gcra_test.go +++ b/ratelimits/gcra_test.go @@ -11,9 +11,8 @@ import ( func TestDecide(t *testing.T) { clk := clock.NewFake() - limit := precomputeLimit( - limit{Burst: 10, Count: 1, Period: config.Duration{Duration: time.Second}}, - ) + limit := limit{Burst: 10, Count: 1, Period: config.Duration{Duration: time.Second}} + limit.precompute() // Begin by using 1 of our 10 requests. d := maybeSpend(clk, limit, clk.Now(), 1) @@ -135,9 +134,8 @@ func TestDecide(t *testing.T) { func TestMaybeRefund(t *testing.T) { clk := clock.NewFake() - limit := precomputeLimit( - limit{Burst: 10, Count: 1, Period: config.Duration{Duration: time.Second}}, - ) + limit := limit{Burst: 10, Count: 1, Period: config.Duration{Duration: time.Second}} + limit.precompute() // Begin by using 1 of our 10 requests. d := maybeSpend(clk, limit, clk.Now(), 1) diff --git a/ratelimits/limit.go b/ratelimits/limit.go index 73c4fe953..df2cd268c 100644 --- a/ratelimits/limit.go +++ b/ratelimits/limit.go @@ -44,15 +44,19 @@ type limit struct { // precomputed to avoid doing the same calculation on every request. burstOffset int64 - // isOverride is true if this limit is an override limit, false if it is a - // default limit. - isOverride bool + // overrideKey is the key used to look up this limit in the overrides map. + overrideKey string } -func precomputeLimit(l limit) limit { +// isOverride returns true if the limit is an override. +func (l *limit) isOverride() bool { + return l.overrideKey != "" +} + +// precompute calculates the emissionInterval and burstOffset for the limit. +func (l *limit) precompute() { l.emissionInterval = l.Period.Nanoseconds() / l.Count l.burstOffset = l.emissionInterval * l.Burst - return l } func validateLimit(l limit) error { @@ -157,21 +161,24 @@ func loadAndParseOverrideLimits(path string) (limits, error) { return nil, fmt.Errorf("unrecognized name %q in override limit, must be one of %v", k, limitNames) } v.limit.name = name - v.limit.isOverride = true + for _, entry := range v.Ids { + limit := v.limit id := entry.Id err = validateIdForName(name, id) if err != nil { return nil, fmt.Errorf( "validating name %s and id %q for override limit %q: %w", name, id, k, err) } + limit.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, ","))) } - parsed[joinWithColon(name.EnumString(), id)] = precomputeLimit(v.limit) + limit.precompute() + parsed[joinWithColon(name.EnumString(), id)] = limit } } } @@ -197,7 +204,8 @@ func loadAndParseDefaultLimits(path string) (limits, error) { return nil, fmt.Errorf("unrecognized name %q in default limit, must be one of %v", k, limitNames) } v.name = name - parsed[name.EnumString()] = precomputeLimit(v) + v.precompute() + parsed[name.EnumString()] = v } return parsed, nil } diff --git a/ratelimits/limiter.go b/ratelimits/limiter.go index 6eecd48ee..557a83304 100644 --- a/ratelimits/limiter.go +++ b/ratelimits/limiter.go @@ -196,9 +196,9 @@ func (l *Limiter) BatchSpend(ctx context.Context, txns []Transaction) (*Decision d := maybeSpend(l.clk, txn.limit, tat, txn.cost) - if txn.limit.isOverride { + if txn.limit.isOverride() { utilization := float64(txn.limit.Burst-d.Remaining) / float64(txn.limit.Burst) - l.overrideUsageGauge.WithLabelValues(txn.limit.name.String(), txn.bucketKey).Set(utilization) + l.overrideUsageGauge.WithLabelValues(txn.limit.name.String(), txn.limit.overrideKey).Set(utilization) } if d.Allowed && (tat != d.newTAT) && txn.spend {