From b700f343424b05a7c23759a37c4b95431a918325 Mon Sep 17 00:00:00 2001 From: bussyjd Date: Sun, 24 May 2026 10:20:19 +0400 Subject: [PATCH] feat(x402-metrics): add asset_symbol label for per-token queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../base/templates/x402-prometheus-rules.yaml | 27 ++++ internal/x402/metrics.go | 52 +++--- internal/x402/verifier.go | 18 ++- internal/x402/verifier_test.go | 148 +++++++++++++++++- 4 files changed, 218 insertions(+), 27 deletions(-) diff --git a/internal/embed/infrastructure/base/templates/x402-prometheus-rules.yaml b/internal/embed/infrastructure/base/templates/x402-prometheus-rules.yaml index f2d2071..77b6429 100644 --- a/internal/embed/infrastructure/base/templates/x402-prometheus-rules.yaml +++ b/internal/embed/infrastructure/base/templates/x402-prometheus-rules.yaml @@ -33,12 +33,28 @@ spec: # 24h charged-request count per (offer, chain). Replaces the # frontend's `increase(charged_requests_total[24h])` query — same # math, pre-computed every 30s. + # + # Kept unchanged for backwards compatibility. The + # _by_offer_chain_asset_symbol sibling below is the migration + # target for the frontend's per-token EarningsStrip columns. - record: x402:revenue:24h_by_offer_chain expr: | sum by (offer_namespace, offer_name, chain) ( increase(obol_x402_verifier_charged_requests_total[24h]) ) + # 24h charged-request count per (offer, chain, asset_symbol). + # Same math as :24h_by_offer_chain but keeps the asset dimension + # so the frontend can answer "what's my OBOL revenue?" with a + # single PromQL query instead of joining metrics with the + # ServiceOffer CR. Adding asset_symbol is non-multiplicative + # because each offer pins exactly one asset (A=1 per offer). + - record: x402:revenue:24h_by_offer_chain_asset_symbol + expr: | + sum by (offer_namespace, offer_name, chain, asset_symbol) ( + increase(obol_x402_verifier_charged_requests_total[24h]) + ) + # 7d charged-request count per (offer, chain). Powers the # EarningsStrip per-chain × CRD price multiplication. - record: x402:revenue:7d_by_offer_chain @@ -47,6 +63,17 @@ spec: increase(obol_x402_verifier_charged_requests_total[7d]) ) + # 7d charged-request count per (offer, chain, asset_symbol). + # Sibling of :7d_by_offer_chain — once the frontend migrates to + # the per-asset rule, the EarningsStrip can drop its + # CR-join-at-query-time for per-token columns. Cardinality is + # non-multiplicative because each offer pins exactly one asset. + - record: x402:revenue:7d_by_offer_chain_asset_symbol + expr: | + sum by (offer_namespace, offer_name, chain, asset_symbol) ( + increase(obol_x402_verifier_charged_requests_total[7d]) + ) + # 7d charged-request count per offer (chain-agnostic). Used in the # My Listings "7d · X earned" header text and the Browse catalog # usage badge. diff --git a/internal/x402/metrics.go b/internal/x402/metrics.go index b445d4c..2779d14 100644 --- a/internal/x402/metrics.go +++ b/internal/x402/metrics.go @@ -10,12 +10,12 @@ import ( type verifierMetrics struct { registry *prometheus.Registry - requestsTotal *prometheus.CounterVec - paymentRequired *prometheus.CounterVec - paymentVerified *prometheus.CounterVec - paymentFailed *prometheus.CounterVec - chargedRequests *prometheus.CounterVec - lastPaymentSuccess *prometheus.GaugeVec + requestsTotal *prometheus.CounterVec + paymentRequired *prometheus.CounterVec + paymentVerified *prometheus.CounterVec + paymentFailed *prometheus.CounterVec + chargedRequests *prometheus.CounterVec + lastPaymentSuccess *prometheus.GaugeVec } func newVerifierMetrics() *verifierMetrics { @@ -26,42 +26,42 @@ func newVerifierMetrics() *verifierMetrics { Name: "obol_x402_verifier_requests_total", Help: "Requests evaluated by the x402 verifier for matched paid routes.", }, - []string{"offer_namespace", "offer_name", "chain"}, + []string{"offer_namespace", "offer_name", "chain", "asset_symbol"}, ), paymentRequired: prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "obol_x402_verifier_payment_required_total", Help: "Requests rejected with 402 because payment was required.", }, - []string{"offer_namespace", "offer_name", "chain"}, + []string{"offer_namespace", "offer_name", "chain", "asset_symbol"}, ), paymentVerified: prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "obol_x402_verifier_payment_verified_total", Help: "Requests approved after successful x402 payment verification.", }, - []string{"offer_namespace", "offer_name", "chain"}, + []string{"offer_namespace", "offer_name", "chain", "asset_symbol"}, ), paymentFailed: prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "obol_x402_verifier_payment_failed_total", Help: "Requests rejected after a provided x402 payment failed verification.", }, - []string{"offer_namespace", "offer_name", "chain"}, + []string{"offer_namespace", "offer_name", "chain", "asset_symbol"}, ), chargedRequests: prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "obol_x402_verifier_charged_requests_total", Help: "Requests that incurred a paid x402 charge.", }, - []string{"offer_namespace", "offer_name", "chain"}, + []string{"offer_namespace", "offer_name", "chain", "asset_symbol"}, ), lastPaymentSuccess: prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "obol_x402_verifier_last_payment_success_seconds", Help: "Unix timestamp (seconds) of the most recent successful paid x402 charge for a route.", }, - []string{"offer_namespace", "offer_name", "chain"}, + []string{"offer_namespace", "offer_name", "chain", "asset_symbol"}, ), } @@ -81,16 +81,19 @@ func (m *verifierMetrics) handler() http.Handler { return promhttp.HandlerFor(m.registry, promhttp.HandlerOpts{}) } -// pruneSeriesNotIn drops every (offer_namespace, offer_name, chain) series -// from the verifier's counter/gauge vecs that is not present in `keep`. -// Called from Verifier.load whenever the route set changes so deleted offers -// (e.g. `obol sell delete`) stop emitting stale series — most importantly the -// last_payment_success_seconds gauge, which would otherwise hold the deleted -// offer's last-success timestamp forever and falsely satisfy "recent activity" -// alerts and dashboards. +// pruneSeriesNotIn drops every (offer_namespace, offer_name, chain, +// asset_symbol) series from the verifier's counter/gauge vecs that is not +// present in `keep`. Called from Verifier.load whenever the route set changes +// so deleted offers (e.g. `obol sell delete`) stop emitting stale series — +// most importantly the last_payment_success_seconds gauge, which would +// otherwise hold the deleted offer's last-success timestamp forever and +// falsely satisfy "recent activity" alerts and dashboards. // -// Key shape: "ns\x00name\x00chain" — \x00 is forbidden in Kubernetes object -// names and CAIP-2 chain ids, so the byte-join can't collide. +// Key shape: "ns\x00name\x00chain\x00asset" — \x00 is forbidden in +// Kubernetes object names, CAIP-2 chain ids, and ERC-20 symbols, so the +// byte-join can't collide. Including asset_symbol in the key means an +// asset-repin (USDC → OBOL on the same offer) prunes the old series rather +// than leaking a stale per-asset timestamp. func (m *verifierMetrics) pruneSeriesNotIn(keep map[string]struct{}) { vecs := []interface { DeletePartialMatch(prometheus.Labels) int @@ -110,7 +113,7 @@ func (m *verifierMetrics) pruneSeriesNotIn(keep map[string]struct{}) { for _, family := range gathered { for _, metric := range family.GetMetric() { labels := metric.GetLabel() - ns, name, chain := "", "", "" + ns, name, chain, asset := "", "", "", "" for _, l := range labels { switch l.GetName() { case "offer_namespace": @@ -119,18 +122,21 @@ func (m *verifierMetrics) pruneSeriesNotIn(keep map[string]struct{}) { name = l.GetValue() case "chain": chain = l.GetValue() + case "asset_symbol": + asset = l.GetValue() } } if ns == "" && name == "" { continue } - if _, ok := keep[ns+"\x00"+name+"\x00"+chain]; ok { + if _, ok := keep[ns+"\x00"+name+"\x00"+chain+"\x00"+asset]; ok { continue } match := prometheus.Labels{ "offer_namespace": ns, "offer_name": name, "chain": chain, + "asset_symbol": asset, } for _, vec := range vecs { vec.DeletePartialMatch(match) diff --git a/internal/x402/verifier.go b/internal/x402/verifier.go index 60e2fa8..77e42db 100644 --- a/internal/x402/verifier.go +++ b/internal/x402/verifier.go @@ -74,7 +74,7 @@ func (v *Verifier) load(cfg *PricingConfig) error { if r.OfferNamespace == "" && r.OfferName == "" { continue } - live[r.OfferNamespace+"\x00"+r.OfferName+"\x00"+r.Network] = struct{}{} + live[r.OfferNamespace+"\x00"+r.OfferName+"\x00"+r.Network+"\x00"+r.AssetSymbol] = struct{}{} } v.metrics.pruneSeriesNotIn(live) @@ -466,9 +466,25 @@ func prometheusLabels(rule *RouteRule) prometheus.Labels { // offer_name) which already uniquely identifies a paid route — the // pattern was redundant and unbounded by path fragments, which would // have ballooned series count for sellers running many granular routes. + // + // asset_symbol is included for direct per-token aggregation in PromQL + // (e.g. "what's my OBOL revenue?") without having to join the metric + // against the ServiceOffer CR at query time. Cardinality cost is zero + // because each offer pins exactly one asset — the new dimension is + // functionally constant within the existing (ns, name) group. + asset := rule.AssetSymbol + if asset == "" { + // Defensive: a missing symbol is operationally ugly in PromQL. + // Empty-string labels are legal in Prometheus but render as a + // bare "asset_symbol=" in selectors, which makes dashboard + // filters harder to write. "unknown" is unambiguous and matches + // the convention we use elsewhere for under-populated metadata. + asset = "unknown" + } return prometheus.Labels{ "offer_namespace": rule.OfferNamespace, "offer_name": rule.OfferName, "chain": rule.Network, + "asset_symbol": asset, } } diff --git a/internal/x402/verifier_test.go b/internal/x402/verifier_test.go index 3b62c81..b41a274 100644 --- a/internal/x402/verifier_test.go +++ b/internal/x402/verifier_test.go @@ -756,6 +756,7 @@ func TestVerifier_MetricsPaymentRequired(t *testing.T) { "offer_namespace": "llm", "offer_name": "paid-rpc", "chain": "", + "asset_symbol": "unknown", } assertVerifierMetricValue(t, metrics["obol_x402_verifier_requests_total"], labels, 1) assertVerifierMetricValue(t, metrics["obol_x402_verifier_payment_required_total"], labels, 1) @@ -769,6 +770,7 @@ func TestVerifier_MetricsVerifiedAndRejectedPayments(t *testing.T) { "offer_namespace": "llm", "offer_name": "paid-rpc", "chain": "", + "asset_symbol": "unknown", } okFac := newMockFacilitator(t, mockFacilitatorOpts{}) @@ -828,12 +830,15 @@ func TestVerifier_MetricsVerifiedAndRejectedPayments(t *testing.T) { // when an unpaid request is rejected with 402. // // The gauge is labeled identically to the verifier counters; for this rule -// `chain` is the empty string because the test RouteRule has no Network set. +// `chain` is the empty string because the test RouteRule has no Network set, +// and `asset_symbol` is "unknown" because AssetSymbol is unset (the defensive +// fallback emitted by prometheusLabels). func TestVerifier_LastPaymentSuccessGauge(t *testing.T) { labels := map[string]string{ "offer_namespace": "llm", "offer_name": "paid-rpc", "chain": "", + "asset_symbol": "unknown", } tests := []struct { @@ -950,8 +955,8 @@ func TestVerifier_Reload_PrunesDeletedOfferSeries(t *testing.T) { } } - keptLabels := map[string]string{"offer_namespace": "llm", "offer_name": "keep", "chain": ""} - goneLabels := map[string]string{"offer_namespace": "llm", "offer_name": "gone", "chain": ""} + keptLabels := map[string]string{"offer_namespace": "llm", "offer_name": "keep", "chain": "", "asset_symbol": "unknown"} + goneLabels := map[string]string{"offer_namespace": "llm", "offer_name": "gone", "chain": "", "asset_symbol": "unknown"} families := scrapeVerifierMetrics(t, v) for _, name := range []string{ @@ -1086,3 +1091,140 @@ func verifierMetricValue(metric *dto.Metric) float64 { return 0 } } + +// TestVerifier_PrometheusLabels_IncludesAssetSymbol asserts that the +// asset_symbol label is emitted with the value from RouteRule.AssetSymbol +// (which the serviceoffer_source populates from +// offer.Spec.Payment.Asset.Symbol). This is what makes "what's my OBOL +// revenue?" a single PromQL aggregation instead of a metric × CR join. +func TestVerifier_PrometheusLabels_IncludesAssetSymbol(t *testing.T) { + rule := &RouteRule{ + OfferNamespace: "llm", + OfferName: "demo-hello", + Network: "eip155:84532", + AssetSymbol: "USDC", + } + labels := prometheusLabels(rule) + if got := labels["asset_symbol"]; got != "USDC" { + t.Errorf("asset_symbol = %q, want %q (full labels: %v)", got, "USDC", labels) + } + if got := labels["chain"]; got != "eip155:84532" { + t.Errorf("chain = %q, want %q", got, "eip155:84532") + } +} + +// TestVerifier_PrometheusLabels_DefaultsToUnknownIfEmpty asserts the +// defensive fallback: when AssetSymbol is empty (legacy offers, parsing +// hiccup, etc.) the label value is "unknown" rather than "" — empty-string +// labels are legal in Prometheus but render as bare selectors that are +// awkward to filter in dashboards. +func TestVerifier_PrometheusLabels_DefaultsToUnknownIfEmpty(t *testing.T) { + rule := &RouteRule{ + OfferNamespace: "llm", + OfferName: "no-asset", + Network: "eip155:84532", + AssetSymbol: "", + } + labels := prometheusLabels(rule) + if got := labels["asset_symbol"]; got != "unknown" { + t.Errorf("asset_symbol = %q, want %q (full labels: %v)", got, "unknown", labels) + } +} + +// TestVerifier_PruneSeriesNotIn_DistinguishesAssetSymbol asserts that +// pruning treats asset_symbol as part of the series key, so an asset-repin +// scenario (USDC route gets dropped, OBOL route for the same offer is +// retained) prunes the dead USDC series without taking the live OBOL one +// with it. Without asset_symbol in the key, both series would map to the +// same (ns, name, chain) tuple and pruning would either drop both or +// neither — leaking a stale per-asset series. +func TestVerifier_PruneSeriesNotIn_DistinguishesAssetSymbol(t *testing.T) { + fac := newMockFacilitator(t, mockFacilitatorOpts{}) + usdcRoute := RouteRule{ + Pattern: "/svc/*", + Price: "0.0001", + OfferNamespace: "llm", + OfferName: "demo", + Network: "base-sepolia", + AssetSymbol: "USDC", + } + obolRoute := RouteRule{ + Pattern: "/svc-obol/*", + Price: "0.0001", + OfferNamespace: "llm", + OfferName: "demo", + Network: "base-sepolia", + AssetSymbol: "OBOL", + } + v := newTestVerifier(t, fac.URL, []RouteRule{usdcRoute, obolRoute}) + + // Stamp a successful paid request through each asset variant so both + // series exist in the registry before pruning. + for _, path := range []string{"/svc/x", "/svc-obol/x"} { + req := httptest.NewRequest(http.MethodPost, "/verify", nil) + req.Header.Set("X-Forwarded-Uri", path) + req.Header.Set("X-Forwarded-Host", "obol.stack") + req.Header.Set("X-PAYMENT", testPaymentHeader(t)) + rec := httptest.NewRecorder() + v.HandleVerify(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("setup paid request to %s: status=%d", path, rec.Code) + } + } + + usdcLabels := map[string]string{ + "offer_namespace": "llm", + "offer_name": "demo", + "chain": "base-sepolia", + "asset_symbol": "USDC", + } + obolLabels := map[string]string{ + "offer_namespace": "llm", + "offer_name": "demo", + "chain": "base-sepolia", + "asset_symbol": "OBOL", + } + + families := scrapeVerifierMetrics(t, v) + for _, name := range []string{ + "obol_x402_verifier_charged_requests_total", + "obol_x402_verifier_last_payment_success_seconds", + } { + family := families[name] + if family == nil { + t.Fatalf("baseline: missing %s before reload", name) + } + findVerifierMetricValue(t, family, usdcLabels) + findVerifierMetricValue(t, family, obolLabels) + } + + // Drop the USDC route, keep OBOL. If pruneSeriesNotIn ignored + // asset_symbol, both series would key to (llm, demo, base-sepolia) + // and the OBOL series would survive (because the OBOL route is in + // the keep set) — masking the bug. Conversely, if the key didn't + // distinguish at all, both could be wiped. Including asset_symbol + // in the key keeps USDC prunable and OBOL alive. + if err := v.Reload(&PricingConfig{ + Wallet: "0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + Chain: "base-sepolia", + FacilitatorURL: fac.URL, + Routes: []RouteRule{obolRoute}, + }); err != nil { + t.Fatalf("Reload: %v", err) + } + + families = scrapeVerifierMetrics(t, v) + for _, name := range []string{ + "obol_x402_verifier_requests_total", + "obol_x402_verifier_charged_requests_total", + "obol_x402_verifier_last_payment_success_seconds", + } { + assertVerifierMetricMissing(t, families[name], usdcLabels) + } + + if charged := families["obol_x402_verifier_charged_requests_total"]; charged != nil { + findVerifierMetricValue(t, charged, obolLabels) + } else { + t.Errorf("OBOL charged series was pruned along with USDC — asset_symbol was ignored in prune key") + } +}