diff --git a/packages/bridge-controller/CHANGELOG.md b/packages/bridge-controller/CHANGELOG.md index 68a682c0b22..5df1e17952c 100644 --- a/packages/bridge-controller/CHANGELOG.md +++ b/packages/bridge-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add optional `active_ab_tests` property in Unified SwapBridge metrics event context and payload types, alongside existing `ab_tests`. ([#8152](https://github.com/MetaMask/core/pull/8152)) + ## [69.0.1] ### Changed diff --git a/packages/bridge-controller/src/utils/metrics/types.ts b/packages/bridge-controller/src/utils/metrics/types.ts index ed3a25a3d61..df0396cfe5e 100644 --- a/packages/bridge-controller/src/utils/metrics/types.ts +++ b/packages/bridge-controller/src/utils/metrics/types.ts @@ -246,11 +246,15 @@ type RequiredEventContextFromClientBase = { * This combines the event-specific properties from RequiredEventContextFromClientBase * with an optional `location` property. When `location` is omitted, the controller * falls back to the value stored via `setLocation()` (defaults to MainView). + * + * `ab_tests` is the legacy field and `active_ab_tests` is the newer field. + * Both are kept for a migration window and are treated as separate payloads. */ export type RequiredEventContextFromClient = { [K in keyof RequiredEventContextFromClientBase]: RequiredEventContextFromClientBase[K] & { location?: MetaMetricsSwapsEventSource; ab_tests?: Record; + active_ab_tests?: { key: string; value: string }[]; }; }; @@ -313,6 +317,9 @@ export type EventPropertiesFromControllerState = { /** * trackUnifiedSwapBridgeEvent payload properties consist of required properties from the client * and properties from the bridge controller + * + * `ab_tests` will be deprecated in favor of `active_ab_tests` in the future. + * `ab_tests` and `active_ab_tests` intentionally coexist during migration. */ export type CrossChainSwapsEventProperties< T extends UnifiedSwapBridgeEventName, @@ -321,6 +328,7 @@ export type CrossChainSwapsEventProperties< action_type: MetricsActionType; location: MetaMetricsSwapsEventSource; ab_tests?: Record; + active_ab_tests?: { key: string; value: string }[]; } | Pick[T] | Pick[T]; diff --git a/packages/bridge-status-controller/CHANGELOG.md b/packages/bridge-status-controller/CHANGELOG.md index eb64e5a7770..bd397d6b0ae 100644 --- a/packages/bridge-status-controller/CHANGELOG.md +++ b/packages/bridge-status-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added optional `activeAbTests` context support so Unified SwapBridge events can include `active_ab_tests` independently of `ab_tests`. ([#8152](https://github.com/MetaMask/core/pull/8152)) + ## [68.0.2] ### Changed diff --git a/packages/bridge-status-controller/src/bridge-status-controller.test.ts b/packages/bridge-status-controller/src/bridge-status-controller.test.ts index 5d4469f177f..c336673ce2a 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.test.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.test.ts @@ -4255,7 +4255,7 @@ describe('BridgeStatusController', () => { expect(messengerCallSpy.mock.lastCall).toMatchSnapshot(); }); - it('should include ab_tests from history in tracked event properties', () => { + it('should include ab_tests and active_ab_tests from history in tracked event properties', () => { const abTestsTxMetaId = 'bridgeTxMetaIdAbTests'; bridgeStatusController.startPollingForBridgeTxStatus({ ...getMockStartPollingForBridgeTxStatusArgs({ @@ -4263,6 +4263,7 @@ describe('BridgeStatusController', () => { srcTxHash: '0xsrcTxHashAbTests', }), abTests: { token_details_layout: 'treatment' }, + activeAbTests: [{ key: 'bridge_quote_sorting', value: 'variant_b' }], }); const messengerCallSpy = jest.spyOn(mockBridgeStatusMessenger, 'call'); @@ -4284,6 +4285,9 @@ describe('BridgeStatusController', () => { expect.anything(), expect.objectContaining({ ab_tests: { token_details_layout: 'treatment' }, + active_ab_tests: [ + { key: 'bridge_quote_sorting', value: 'variant_b' }, + ], }), ); }); diff --git a/packages/bridge-status-controller/src/bridge-status-controller.ts b/packages/bridge-status-controller/src/bridge-status-controller.ts index 1b444f28f09..144a68f470a 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.ts @@ -523,6 +523,7 @@ export class BridgeStatusController extends StaticIntervalPollingController { // Use actionId as key for pre-submission, or txMeta.id for post-submission @@ -1308,7 +1310,8 @@ export class BridgeStatusController extends StaticIntervalPollingController, + activeAbTests?: { key: string; value: string }[], ): Promise> => { this.messenger.call( 'BridgeController:stopPollingForQuotes', @@ -1341,6 +1345,7 @@ export class BridgeStatusController extends StaticIntervalPollingController; + activeAbTests?: { key: string; value: string }[]; }): Promise> => { - const { quoteResponse, accountAddress, location, abTests } = params; + const { quoteResponse, accountAddress, location, abTests, activeAbTests } = + params; this.messenger.call( 'BridgeController:stopPollingForQuotes', @@ -1625,6 +1635,7 @@ export class BridgeStatusController extends StaticIntervalPollingController[EventName], ): void => { - const { ab_tests: eventAbTests } = - (eventProperties as - | Record | undefined> - | undefined) ?? {}; + // Legacy/new metrics fields are intentionally kept independent during migration. const historyAbTests = txMetaId ? this.state.txHistory?.[txMetaId]?.abTests : undefined; - const resolvedAbTests = eventAbTests ?? historyAbTests ?? undefined; + const historyActiveAbTests = txMetaId + ? this.state.txHistory?.[txMetaId]?.activeAbTests + : undefined; + const resolvedAbTests = eventProperties?.ab_tests ?? historyAbTests; + const resolvedActiveAbTests = + eventProperties?.active_ab_tests ?? historyActiveAbTests; const baseProperties = { action_type: MetricsActionType.SWAPBRIDGE_V1, @@ -1841,6 +1855,10 @@ export class BridgeStatusController extends StaticIntervalPollingController 0 && { ab_tests: resolvedAbTests, }), + ...(resolvedActiveAbTests && + resolvedActiveAbTests.length > 0 && { + active_ab_tests: resolvedActiveAbTests, + }), }; // This will publish events for PERPS dropped tx failures as well diff --git a/packages/bridge-status-controller/src/types.ts b/packages/bridge-status-controller/src/types.ts index e4fe5193c3d..7473c502d49 100644 --- a/packages/bridge-status-controller/src/types.ts +++ b/packages/bridge-status-controller/src/types.ts @@ -139,10 +139,16 @@ export type BridgeHistoryItem = { */ location?: MetaMetricsSwapsEventSource; /** - * A/B test context to attribute swap/bridge events to specific experiments. + * Legacy A/B test metrics context (`ab_tests`) kept for backward compatibility. * Keys are test names, values are variant names (e.g. { token_details_layout: 'treatment' }). */ abTests?: Record; + /** + * New A/B test metrics context (`active_ab_tests`) that replaces `ab_tests`. + * Kept separate so migration can run both payloads in parallel. + * This field is an array of test objects. + */ + activeAbTests?: { key: string; value: string }[]; /** * Attempts tracking for exponential backoff on failed fetches. * We track the number of attempts and the last attempt time for each txMetaId that has failed at least once @@ -212,7 +218,10 @@ export type StartPollingForBridgeTxStatusArgs = { approvalTxId?: BridgeHistoryItem['approvalTxId']; isStxEnabled?: BridgeHistoryItem['isStxEnabled']; location?: BridgeHistoryItem['location']; + // Legacy field for `ab_tests` metrics payload. abTests?: BridgeHistoryItem['abTests']; + // New field for `active_ab_tests` metrics payload. + activeAbTests?: BridgeHistoryItem['activeAbTests']; accountAddress: string; }; diff --git a/packages/bridge-status-controller/src/utils/metrics.test.ts b/packages/bridge-status-controller/src/utils/metrics.test.ts index 2d17838369e..b7475214048 100644 --- a/packages/bridge-status-controller/src/utils/metrics.test.ts +++ b/packages/bridge-status-controller/src/utils/metrics.test.ts @@ -1,4 +1,9 @@ -import { StatusTypes, FeeType, ActionTypes } from '@metamask/bridge-controller'; +import { + StatusTypes, + FeeType, + ActionTypes, + MetaMetricsSwapsEventSource, +} from '@metamask/bridge-controller'; import { MetricsSwapType, MetricsActionType, @@ -17,6 +22,7 @@ import { getTradeDataFromHistory, getRequestMetadataFromHistory, getEVMTxPropertiesFromTransactionMeta, + getPreConfirmationPropertiesFromQuote, } from './metrics'; import type { BridgeHistoryItem } from '../types'; @@ -964,6 +970,36 @@ describe('metrics utils', () => { }); }); + describe('getPreConfirmationPropertiesFromQuote', () => { + it('should include both ab_tests and active_ab_tests when both sets are provided', () => { + const abTests = { token_details_layout: 'treatment' }; + const activeAbTests = [ + { key: 'bridge_quote_sorting', value: 'variant_b' }, + ]; + const result = getPreConfirmationPropertiesFromQuote( + { + quote: mockHistoryItem.quote, + estimatedProcessingTimeInSeconds: 900, + adjustedReturn: { usd: '1980' }, + sentAmount: { usd: '2000' }, + gasFee: { effective: { usd: '2.54739' } }, + } as never, + false, + false, + MetaMetricsSwapsEventSource.MainView, + abTests, + activeAbTests, + ); + + expect(result).toStrictEqual( + expect.objectContaining({ + ab_tests: abTests, + active_ab_tests: activeAbTests, + }), + ); + }); + }); + describe('getEVMSwapTxPropertiesFromTransactionMeta', () => { const mockTransactionMeta: TransactionMeta = { id: 'test-tx-id', diff --git a/packages/bridge-status-controller/src/utils/metrics.ts b/packages/bridge-status-controller/src/utils/metrics.ts index 462a92025aa..e24188fb114 100644 --- a/packages/bridge-status-controller/src/utils/metrics.ts +++ b/packages/bridge-status-controller/src/utils/metrics.ts @@ -177,7 +177,8 @@ export const getPriceImpactFromQuote = ( * @param isStxEnabledOnClient - Whether smart transactions are enabled on the client, for example the getSmartTransactionsEnabled selector value from the extension * @param isHardwareAccount - whether the tx is submitted using a hardware wallet * @param location - The entry point from which the user initiated the swap or bridge (e.g. Main View, Token View, Trending Explore) - * @param abTests - A/B test context to attribute events to specific experiments + * @param abTests - Legacy A/B test context for `ab_tests` (backward compatibility) + * @param activeAbTests - New A/B test context for `active_ab_tests` (migration target) * @returns The properties for the pre-confirmation event */ export const getPreConfirmationPropertiesFromQuote = ( @@ -186,6 +187,7 @@ export const getPreConfirmationPropertiesFromQuote = ( isHardwareAccount: boolean, location: MetaMetricsSwapsEventSource = MetaMetricsSwapsEventSource.MainView, abTests?: Record, + activeAbTests?: { key: string; value: string }[], ) => { const { quote } = quoteResponse; return { @@ -205,7 +207,14 @@ export const getPreConfirmationPropertiesFromQuote = ( action_type: MetricsActionType.SWAPBRIDGE_V1, custom_slippage: false, // TODO detect whether the user changed the default slippage location, - ...(abTests && Object.keys(abTests).length > 0 && { ab_tests: abTests }), + ...(abTests && + Object.keys(abTests).length > 0 && { + ab_tests: abTests, + }), + ...(activeAbTests && + activeAbTests.length > 0 && { + active_ab_tests: activeAbTests, + }), }; };