Budget gauge writes are not gated when budgets are disabled #940

Activity

1. coderabbitai commented on Aug 12, 2026

   

   coderabbitaiBot

   on Aug 12, 2026 – with coderabbitai

   More actions

   🔗 Related PRs

   #925 - perf(metrics): per-worker handle cache for every per-request emit path [merged]

   ---

   📝 Issue Planner

   _{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

   • Create Plan

   ---

   🧪 Issue enrichment is currently in open beta.

   You can configure auto-planning by selecting labels in the issue_enrichment configuration.

   To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

   issue_enrichment:
     auto_enrich:
       enabled: false

   💬 Have feedback or questions? Drop into our discord!
2. coderabbitai

   added

   enhancementNew feature or requestNew feature or request

   P2Long-tail integrations — backlogLong-tail integrations — backlog

   on Aug 12, 2026
3. membphis commented on Aug 12, 2026

   

   membphis

   on Aug 12, 2026

   ContributorAuthor

   More actions

   Closing as not planned. The design question does have a fourth candidate, and it costs more structure than the 0.2µs is worth.

   Option 2 cannot skip the work it exists to skip

   The per-request cost of clear_budget_gauges is: build the lookup key into the worker's reusable buffer (instance prefix + site + the three label values), FNV-hash it, probe the thread-local handle map, then gauge.set(0.0).

   To answer "has this label set already been published?" you have to identify the label set — which is the key build and the hash, i.e. everything except the final atomic store. Memoising the value saves only that store, which the issue already identifies as the small half.

   Making the memo cheaper means keying it on something other than the label values

   Each candidate breaks something:

   • A "published" flag on the Arc<ResourceEntry<ApiKey>>. Genuinely cheap — one relaxed load — and it keeps the series set exactly "keys with traffic". But a series is registered in a recorder, and Metrics deliberately supports more than one instance in a process; worker_key_prefix exists for precisely that reason. An entry-level flag is recorder-agnostic, so the first instance publishes and the second never does. Making it correct means keying by (entry, recorder), which is the keyed lookup we were trying to avoid.
   • Caching the metrics::Gauge handle on the entry. Same problem, and additionally puts a recorder handle inside a config-snapshot type.

   Option 1 is the only form that removes the work outright, and it spends an invariant

   Not emitting aisix_budget_details_present when budgets are disabled changes /metrics output. The zero-config fast-path package (#944) shipped under a byte-identical series-set invariant, verified against a 209-series diff. Spending that invariant here is not a trade worth making.

   Size

   ~0.2µs against the current ~102.4µs/request baseline is 0.2% — roughly 7× below the resolution of a single same-rig A/B leg (±1.5µs), so it would not be measurable even if it were free. Same call, and the same reasoning, as #924.

   Left as is. Reopen if the series-compatibility invariant is ever relaxed deliberately, at which point option 1 becomes a two-line change.
4. membphis

   closed this as not plannedon Aug 12, 2026