feat(x402-metrics): add asset_symbol label for per-token queries#531
Closed
bussyjd wants to merge 1 commit into
Closed
feat(x402-metrics): add asset_symbol label for per-token queries#531bussyjd wants to merge 1 commit into
bussyjd wants to merge 1 commit into
Conversation
Currently the verifier emits (offer_namespace, offer_name, chain).
Answering "what's my OBOL revenue?" requires joining metrics with
the ServiceOffer CR's spec.payment.asset.symbol at the frontend.
With asset_symbol on the label set, the answer is a direct PromQL
aggregation.
Cardinality cost: zero. Each offer pins exactly one asset (A=1
per offer), so the new dimension is functionally constant within
the existing (ns, name) group — no series multiplication. The
"don't label what you can derive" guidance exists to prevent
*multiplicative* blowups (chain x pod x pod_owner style); the
single-asset-per-offer invariant means there's no multiplication
to prevent.
The argument for adding asset_symbol is identical to the argument
that already justifies `chain` on these vecs: both are
CR-derived, both are query-meaningful, both have bounded values.
Changes:
- 6 metric vecs: label slice gains "asset_symbol"
- pruneSeriesNotIn key now (ns, name, chain, asset_symbol) so
asset-repin doesn't leak the old series
- verifier.load() live-set built with the same 4-tuple
- prometheusLabels() emits rule.AssetSymbol (or "unknown" if
empty as defensive fallback)
- New _asset_symbol-suffixed recording rules added side-by-side
with existing rules; existing rules unchanged (non-breaking)
- Tests: emission asserts asset_symbol; prune test asserts
asset-repin doesn't leak
Frontend can simplify the existing metric x CR join in a future PR
once it migrates to the _asset_symbol-suffixed rule.
Findings from: plans/integration-test-L7-paid-flow-20260524.md
(OBOL parity smoke surfaced this as a real gap when validating
the WalletStrip / EarningsStrip per-token columns).
0ead594 to
b700f34
Compare
Collaborator
Author
|
Superseded by bundle PR #536 — closing in favor of the consolidated merge target. Original branch and history preserved. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
asset_symbolto the 6 verifier metric vecs so per-token revenue is queryable in PromQL directly, without the frontend having to join metrics with the ServiceOffer CR'sspec.payment.asset.symbolat query time.Cardinality math (no series multiplication)
Each ServiceOffer pins exactly one asset via
spec.payment.asset.symbol. That means within the existing(offer_namespace, offer_name)group,asset_symbolis functionally constant — adding it does not multiply series count.Per-vec series count before:
6 vecs * N offers * C chains-per-offerPer-vec series count after:
6 vecs * N offers * C chains-per-offer * 1 asset-per-offerThe
don't label what you can deriveheuristic exists to prevent multiplicative blowups (chain x pod x pod_ownerstyle). The single-asset-per-offer invariant means there's no multiplication to prevent — the same internal-consistency argument that already justifieschain(also CR-derived) on these vecs applies toasset_symbol.Current test cluster (N=3 offers):
demo-hello,silvermesh-paid,silvermesh-obol-> +3 series per vec, max +18 series total cluster-wide. Negligible.Before / after PromQL
Before (frontend joins charged-requests metric with CR
spec.payment.asset.symbolin JS):After (direct):
Recording-rule migration plan
Non-breaking. Existing rules stay in place for one release:
x402:revenue:24h_by_offer_chain-> kept (frontend currently reads this)x402:revenue:24h_by_offer_chain_asset_symbol-> new siblingx402:revenue:7d_by_offer_chain-> keptx402:revenue:7d_by_offer_chain_asset_symbol-> new siblingFrontend can migrate to the
_asset_symbolrules in a follow-up PR; the old rules continue to produce the same series (adding a label to the raw counter is non-breaking for queries that aggregate it away).x402:settlement_rate:1h_by_offer_chainis intentionally not duplicated — it's a ratio; the asset wouldn't change the answer meaningfully.Changes
internal/x402/metrics.go-> 6 vecs gainasset_symbol;pruneSeriesNotInkey now 4-tupleinternal/x402/verifier.go->load()live-set uses 4-tuple;prometheusLabels()emitsrule.AssetSymbolwith"unknown"defensive fallbackinternal/embed/infrastructure/base/templates/x402-prometheus-rules.yaml-> two new_asset_symbol-suffixed recording rulesinternal/x402/verifier_test.go-> existing label-map assertions updated; 3 new tests addedTest plan
go build ./...cleango vet ./internal/x402/... ./internal/embed/...cleango test ./internal/x402/...green (all 6 vecs + new tests pass)go test ./internal/embed/...green (CRD/manifest tests no regression)TestVerifier_Reload_PrunesDeletedOfferSeriesfrom PR A still passes — asset-symbol-aware key doesn't break basic GCTestVerifier_PrometheusLabels_IncludesAssetSymbol-> assertsUSDCis emittedTestVerifier_PrometheusLabels_DefaultsToUnknownIfEmpty-> asserts"unknown"fallback whenAssetSymbolis emptyTestVerifier_PruneSeriesNotIn_DistinguishesAssetSymbol-> dropping USDC route while keeping OBOL route on the same offer leaves OBOL series alive (catches asset-repin leaks)obol_x402_verifier_charged_requests_total{asset_symbol="OBOL"}after a paid OBOL request to silvermesh-obol returns 1kubectl get prometheusrule -n x402 x402-verifier -o yamlshows both_by_offer_chainand_by_offer_chain_asset_symbolrules