ratelimits: Fix cardinality explosion in overrideUsageGauge (#7548)

This commit is contained in:
Samantha 2024-06-14 13:34:40 -04:00 committed by GitHub
parent 0f0c3e1432
commit 063db40db2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 23 additions and 17 deletions

View File

@ -344,7 +344,7 @@ func (builder *TransactionBuilder) CertificatesPerDomainTransactions(regId int64
if err != nil { if err != nil {
return nil, err return nil, err
} }
if perAccountLimit.isOverride { if perAccountLimit.isOverride() {
// An override is configured for the CertificatesPerDomainPerAccount // An override is configured for the CertificatesPerDomainPerAccount
// limit. // limit.
perAccountPerDomainKey, err := newRegIdDomainBucketKey(CertificatesPerDomainPerAccount, regId, name) perAccountPerDomainKey, err := newRegIdDomainBucketKey(CertificatesPerDomainPerAccount, regId, name)

View File

@ -11,9 +11,8 @@ import (
func TestDecide(t *testing.T) { func TestDecide(t *testing.T) {
clk := clock.NewFake() clk := clock.NewFake()
limit := precomputeLimit( limit := limit{Burst: 10, Count: 1, Period: config.Duration{Duration: time.Second}}
limit{Burst: 10, Count: 1, Period: config.Duration{Duration: time.Second}}, limit.precompute()
)
// Begin by using 1 of our 10 requests. // Begin by using 1 of our 10 requests.
d := maybeSpend(clk, limit, clk.Now(), 1) d := maybeSpend(clk, limit, clk.Now(), 1)
@ -135,9 +134,8 @@ func TestDecide(t *testing.T) {
func TestMaybeRefund(t *testing.T) { func TestMaybeRefund(t *testing.T) {
clk := clock.NewFake() clk := clock.NewFake()
limit := precomputeLimit( limit := limit{Burst: 10, Count: 1, Period: config.Duration{Duration: time.Second}}
limit{Burst: 10, Count: 1, Period: config.Duration{Duration: time.Second}}, limit.precompute()
)
// Begin by using 1 of our 10 requests. // Begin by using 1 of our 10 requests.
d := maybeSpend(clk, limit, clk.Now(), 1) d := maybeSpend(clk, limit, clk.Now(), 1)

View File

@ -44,15 +44,19 @@ type limit struct {
// precomputed to avoid doing the same calculation on every request. // precomputed to avoid doing the same calculation on every request.
burstOffset int64 burstOffset int64
// isOverride is true if this limit is an override limit, false if it is a // overrideKey is the key used to look up this limit in the overrides map.
// default limit. overrideKey string
isOverride bool
} }
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.emissionInterval = l.Period.Nanoseconds() / l.Count
l.burstOffset = l.emissionInterval * l.Burst l.burstOffset = l.emissionInterval * l.Burst
return l
} }
func validateLimit(l limit) error { 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) return nil, fmt.Errorf("unrecognized name %q in override limit, must be one of %v", k, limitNames)
} }
v.limit.name = name v.limit.name = name
v.limit.isOverride = true
for _, entry := range v.Ids { for _, entry := range v.Ids {
limit := v.limit
id := entry.Id id := entry.Id
err = validateIdForName(name, id) err = validateIdForName(name, id)
if err != nil { if err != nil {
return nil, fmt.Errorf( return nil, fmt.Errorf(
"validating name %s and id %q for override limit %q: %w", name, id, k, err) "validating name %s and id %q for override limit %q: %w", name, id, k, err)
} }
limit.overrideKey = joinWithColon(name.EnumString(), id)
if name == CertificatesPerFQDNSet { if name == CertificatesPerFQDNSet {
// FQDNSet hashes are not a nice thing to ask for in a // FQDNSet hashes are not a nice thing to ask for in a
// config file, so we allow the user to specify a // config file, so we allow the user to specify a
// comma-separated list of FQDNs and compute the hash here. // comma-separated list of FQDNs and compute the hash here.
id = fmt.Sprintf("%x", core.HashNames(strings.Split(id, ","))) 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) return nil, fmt.Errorf("unrecognized name %q in default limit, must be one of %v", k, limitNames)
} }
v.name = name v.name = name
parsed[name.EnumString()] = precomputeLimit(v) v.precompute()
parsed[name.EnumString()] = v
} }
return parsed, nil return parsed, nil
} }

View File

@ -196,9 +196,9 @@ func (l *Limiter) BatchSpend(ctx context.Context, txns []Transaction) (*Decision
d := maybeSpend(l.clk, txn.limit, tat, txn.cost) 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) 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 { if d.Allowed && (tat != d.newTAT) && txn.spend {