Skip to content

feat(revenue-recovery): add A/B monitoring implementation - #14285

Open
rohanm13 wants to merge 3 commits into
mainfrom
rev-rec-a-b-monitoring-changes
Open

rohanm13 wants to merge 3 commits into
mainfrom
rev-rec-a-b-monitoring-changes

Conversation

@rohanm13

Copy link
Copy Markdown
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Adds the routing and assignment substrate for A/B testing revenue recovery retry
implementations, so that competing implementations can be run side by side on live recovery
traffic and compared.

Superposition configuration

Two new keys, declared in crates/router/src/consts.rs with config! blocks in
crates/router/src/core/configs/dimension_config.rs:

Key Type Default Targeting key
revenue_recovery.ab_enabled bool false none
revenue_recovery.ab_algorithm String "" GlobalPaymentId

ab_enabled is a plain gate, resolved per merchant/connector, that decides whether A/B routing
participates at all. ab_algorithm is the only bucketed config in the revenue recovery set —
its targeting key is the invoice, so an experiment on it splits traffic per invoice rather than
per merchant. Both carry a {merchant_id}_{key} database fallback, consistent with the
neighbouring revenue recovery configs.

The gate is a separate key rather than a field inside the bucketed value so that it cannot be
overridden by an experiment variant: variant overrides are themselves contexts keyed on
variantIds, and matching contexts are merged in server-delivered order, so a gate embedded in
the experiment-driven value could not be reliably forced off for a specific merchant.

TargetingKey for GlobalPaymentId

crates/common_utils/src/id_type/global_id/payment.rsGlobalPaymentId is generated by
global_id_type!, which does not emit this impl (unlike impl_id_type_methods!). Without it
the bucketed config does not compile, and a blank targeting key makes the bucket lookup return
no applicable experiment, so every invoice would silently resolve to the default.

Persisted assignment

recovery_routing added to PaymentRevenueRecoveryMetadata in both api_models and
diesel_models, with #[serde(default)] so existing rows deserialize unchanged. The field
rides payment_intent.feature_metadata, which is JSONB, so there is no migration. The
ApiModelToDieselModelConvertor implementations pass it through in both directions, and
PaymentIntent::get_updated_feature_metadata carries the existing value forward so that
rebuilding the metadata on a new failed attempt does not discard the assignment.

Routing in the CALCULATE workflow

crates/router/src/workflows/revenue_recovery.rs now resolves both keys at the top of the
Smart arm and branches on the gate. When A/B routing is enabled, a stored assignment is
replayed; when none is stored, the implementation is resolved from Superposition using the
invoice as the targeting key and recorded on the intent's feature metadata so subsequent
retries replay it. When the gate is off, the existing selection path runs unchanged.

payment_intent is threaded as &mut through perform_calculate_workflow and
get_token_with_schedule_time_based_on_retry_algorithm_type so the assignment can be recorded;
it reaches the database via
reset_connector_transmission_and_active_attempt_id_before_pushing_to_execute_workflow, which
builds its update request from that object. One borrow adjustment was needed in
crates/router/src/core/revenue_recovery.rs: active_payment_attempt_id is read on both sides
of the now-mutable call, so the later read is re-derived rather than reusing the earlier
binding, which would otherwise hold a shared borrow across it.

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

API contract: recovery_routing: Option<String> is added to
api_models::payments::PaymentRevenueRecoveryMetadata. Additive and optional, so existing
clients are unaffected.

Database schema: no migration. The field lives inside the existing feature_metadata JSONB
column on payment_intent.

Configuration: no file under config/, crates/router/src/configs/ or loadtest/config/
changed. The two new keys are Superposition configs declared in code
(crates/router/src/consts.rs), and they must be created in the Superposition workspace before
the feature can be switched on — with both keys absent, the resolves miss, the database
fallback misses, and the declared defaults (false and "") apply, which is the existing
behaviour.

Motivation and Context

The retry implementation used inside the Smart arm is currently selected from a Superposition
flag that is re-read on every retry. That has two consequences:

  1. No traffic split. Every invoice under a merchant resolves to the same implementation, so
    two implementations cannot be compared against each other on live traffic.
  2. No stickiness. If the flag changes mid-recovery, an invoice switches implementation
    part-way through its retry chain, which makes its outcome unattributable to either.

Superposition owns the bucketing, but bucketing alone is not sufficient for stickiness. The
bucket is hash((targeting_key, group_id)) % 100, and for user-created groups the toss is
rescaled by the experiment's traffic percentage — so changing the ramp re-buckets invoices
that are already in flight
. Recovery spans the grace window (30 days by default), which makes
a ramp change during an invoice's life likely rather than exceptional. The assignment therefore
has to be resolved once, persisted on the invoice, and replayed.

Storing it on the intent rather than in process_tracker.tracking_data is deliberate:
process-tracker rows are keyed one per (runner, task, invoice) and are finished and later
recreated, so the assignment would reset mid-recovery; and tracking_data cannot be joined to
payment_intent.status, which the measurement query needs.

How did you test it?

Compile and lint only — this PR is the routing substrate and is inert by default.

  • just check_v2 — clean
  • just clippy_v2 — clean

No functional testing yet. ab_enabled defaults to false, and both keys are absent from the
Superposition workspace, so every existing code path resolves exactly as it did before this
change and behaviour is unchanged.

The A/B branch is not yet wired to dispatch to an implementation, so end-to-end verification
(traffic split, stickiness across retries, and confirming the assigned implementation actually
runs) is deferred to the follow-up that completes the dispatch. That verification needs a live
stack with Superposition plus a seeded experiment, and is not meaningful until then.

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

@rohanm13
rohanm13 requested review from a team as code owners September 17, 2026 10:55
@semanticdiff-com

semanticdiff-com Bot commented Sep 17, 2026

Copy link
Copy Markdown

@rohanm13 rohanm13 changed the title a-b monitoring changes feat(revenue-recovery): add A/B monitoring implementation Sep 17, 2026
@hyperswitch-bot hyperswitch-bot Bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

M-api-contract-changes Metadata: This PR involves API contract changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant