A/B testing infrastructure updates (#11026)

* Rename AbTestBucket to AbTest

AbTests have multiple `buckets`, so this commit renames the class to be a little clearer.

* Refactor ab_test_spec.rb

- Move tests into #bucket method block
- Set up a `let` for bucket configs

* Move discriminator calculation into AbTest

Provide a proc that can be used to determine a discriminator from user/user_session/service_provider/request.

changelog: Internal, A/B testing, Rework A/B testing system

* Automatically log AB tests with analytics events.

Augment analytics events with a top-level `ab_tests` property that lists each active test and which bucket the event is in.

(This will likely break a lot of tests)

* Add AbTestingConcern

- Add new method, ab_test_bucket, for controllers to figure out what bucket the user is in

* Update ACUANT_SDK AB test to use new system

* Update DOC_AUTH_VENDOR A/B test to use new system

* Allow more control over what events log A/B tests

should_log can be a Proc, RegExp, etc. and is matched against the event name.

* Limit existing A/B tests to IdV events

* Improve use of document_capture_session_uuid as a discriminator

- Handle case where UUID is present in session (hybrid flow)
- Handle case where UUID is in Idv::Session

* Limit should_log to RegExp only

Right now all we're doing with this is checking to see if it's an idv-related event, which we can do with a Regexp.

* Pass acuant_sdk_upgrade_ab_test_bucket into ApiImageUploadForm

- Tell the form what bucket it's in so that it can log properly
- Add test coverage for form submission when Acuant A/B test is enabled

* Remove stray method accidentally added to Idv::Session

* Fix lint issues in api_image_upload_form_spec.rb

* Remove stray _test_ for method accidentally committed

Earlier I was playing with having Idv::Session own discriminator calculation, but I didn't like it. I previously removed a method I accidentally committed--this removes a test for that removed method.

* Add test coverage for A/B test initializers

Run intialize tests under different conditions and actually verify they can return buckets

app/controllers/application_controller.rb

@@ -7,6 +7,7 @@ class ApplicationController < ActionController::Base
   include VerifySpAttributesConcern
   include SecondMfaReminderConcern
   include TwoFactorAuthenticatableMethods
+  include AbTestingConcern
 
   # Prevent CSRF attacks by raising an exception.
   # For APIs, you may want to use :null_session instead.

app/controllers/concerns/ab_testing_concern.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module AbTestingConcern
+  # @param [Symbol] test Name of the test, which should correspond to an A/B test defined in
+  # #                    config/initializer/ab_tests.rb.
+  # @return [Symbol,nil] Bucket to use for the given test, or nil if the test is not active.
+  def ab_test_bucket(test_name)
+    test = AbTests.all[test_name]
+    raise "Unknown A/B test: #{test_name}" unless test
+
+    test.bucket(
+      request:,
+      service_provider: current_sp&.issuer,
+      session:,
+      user: current_user,
+      user_session:,
+    )
+  end
+end

app/controllers/concerns/idv/ab_test_analytics_concern.rb

@@ -13,7 +13,7 @@ def ab_test_analytics_buckets
         buckets = buckets.merge(opt_in_analytics_properties)
       end
 
-      buckets.merge(acuant_sdk_ab_test_analytics_args)
+      buckets
     end
   end
 end

app/controllers/concerns/idv/acuant_concern.rb

@@ -2,18 +2,12 @@
 
 module Idv
   module AcuantConcern
-    def acuant_sdk_ab_test_analytics_args
-      return {} if document_capture_session_uuid.blank?
-
-      {
-        acuant_sdk_upgrade_ab_test_bucket:
-          AbTests::ACUANT_SDK.bucket(document_capture_session_uuid),
-      }
-    end
+    include AbTestingConcern
 
     def acuant_sdk_upgrade_a_b_testing_variables
-      bucket = AbTests::ACUANT_SDK.bucket(document_capture_session_uuid)
-      testing_enabled = IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_enabled
+      bucket = ab_test_bucket(:ACUANT_SDK)
+      testing_enabled = IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_enabled &&
+                        bucket.present?
       use_alternate_sdk = (bucket == :use_alternate_sdk)
 
       if use_alternate_sdk

app/controllers/concerns/idv/doc_auth_vendor_concern.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+module Idv
+  module DocAuthVendorConcern
+    include AbTestingConcern
+
+    # @returns[String] String identifying the vendor to use for doc auth.
+    def doc_auth_vendor
+      bucket = ab_test_bucket(:DOC_AUTH_VENDOR)
+      DocAuthRouter.doc_auth_vendor_for_bucket(bucket)
+    end
+  end
+end

app/controllers/concerns/idv/document_capture_concern.rb

@@ -4,14 +4,11 @@ module Idv
   module DocumentCaptureConcern
     extend ActiveSupport::Concern
 
+    include DocAuthVendorConcern
+
     def save_proofing_components(user)
       return unless user
 
-      doc_auth_vendor = DocAuthRouter.doc_auth_vendor(
-        discriminator: document_capture_session_uuid,
-        analytics: analytics,
-      )
-
       component_attributes = {
         document_check: doc_auth_vendor,
         document_type: 'state_id',

app/controllers/idv/document_capture_controller.rb

@@ -46,6 +46,7 @@ def update
     def extra_view_variables
       {
         document_capture_session_uuid: document_capture_session_uuid,
+        mock_client: doc_auth_vendor == 'mock',
         flow_path: 'standard',
         sp_name: decorated_sp_session.sp_name,
         failure_to_proof_url: return_to_sp_failure_to_proof_url(step: 'document_capture'),

app/controllers/idv/hybrid_mobile/document_capture_controller.rb

@@ -42,6 +42,7 @@ def update
       def extra_view_variables
         {
           flow_path: 'hybrid',
+          mock_client: doc_auth_vendor == 'mock',
           document_capture_session_uuid: document_capture_session_uuid,
           failure_to_proof_url: return_to_sp_failure_to_proof_url(step: 'document_capture'),
           doc_auth_selfie_capture: resolved_authn_context_result.biometric_comparison?,

app/controllers/idv/image_uploads_controller.rb

@@ -2,6 +2,8 @@
 
 module Idv
   class ImageUploadsController < ApplicationController
+    include DocAuthVendorConcern
+
     respond_to :json
 
     def create
@@ -20,6 +22,8 @@ def create
     def image_upload_form
       @image_upload_form ||= Idv::ApiImageUploadForm.new(
         params,
+        doc_auth_vendor:,
+        acuant_sdk_upgrade_ab_test_bucket: ab_test_bucket(:ACUANT_SDK),
         service_provider: current_sp,
         analytics: analytics,
         uuid_prefix: current_sp&.app_id,

app/forms/idv/api_image_upload_form.rb

@@ -17,12 +17,16 @@ class ApiImageUploadForm
     def initialize(
       params,
       service_provider:,
+      doc_auth_vendor:,
+      acuant_sdk_upgrade_ab_test_bucket:,
       analytics: nil,
       uuid_prefix: nil,
       liveness_checking_required: false
     )
       @params = params
       @service_provider = service_provider
+      @doc_auth_vendor = doc_auth_vendor
+      @acuant_sdk_upgrade_ab_test_bucket = acuant_sdk_upgrade_ab_test_bucket
       @analytics = analytics
       @readable = {}
       @uuid_prefix = uuid_prefix
@@ -61,7 +65,7 @@ def submit
     private
 
     attr_reader :params, :analytics, :service_provider, :form_response, :uuid_prefix,
-                :liveness_checking_required
+                :liveness_checking_required, :acuant_sdk_upgrade_ab_test_bucket
 
     def increment_rate_limiter!
       return unless document_capture_session
@@ -315,7 +319,7 @@ def document_capture_session_uuid
 
     def doc_auth_client
       @doc_auth_client ||= DocAuthRouter.client(
-        vendor_discriminator: document_capture_session_uuid,
+        vendor: @doc_auth_vendor,
         warn_notifier: proc do |attrs|
           analytics&.doc_auth_warning(
             **attrs,
@@ -364,11 +368,9 @@ def update_analytics(client_response:, vendor_request_time_in_ms:)
     end
 
     def acuant_sdk_upgrade_ab_test_data
-      return {} unless IdentityConfig.store.idv_acuant_sdk_upgrade_a_b_testing_enabled
       {
-        acuant_sdk_upgrade_ab_test_bucket:
-          AbTests::ACUANT_SDK.bucket(document_capture_session.uuid),
-      }
+        acuant_sdk_upgrade_ab_test_bucket:,
+      }.compact
     end
 
     def acuant_sdk_captured?

app/services/analytics.rb

@@ -28,6 +28,7 @@ def track_event(event, attributes = {})
 
     analytics_hash.merge!(request_attributes) if request
     analytics_hash.merge!(sp_request_attributes) if sp_request_attributes
+    analytics_hash.merge!(ab_test_attributes(event))
 
     ahoy.track(event, analytics_hash)
 
@@ -71,6 +72,32 @@ def request_attributes
     attributes.merge!(browser_attributes)
   end
 
+  def ab_test_attributes(event)
+    user_session = session.dig('warden.user.user.session')
+    ab_tests = AbTests.all.each_with_object({}) do |(test_id, test), obj|
+      next if !test.include_in_analytics_event?(event)
+
+      bucket = test.bucket(
+        request:,
+        service_provider: sp,
+        session:,
+        user:,
+        user_session:,
+      )
+      if !bucket.blank?
+        obj[test_id.downcase] = {
+          bucket:,
+        }
+      end
+    end
+
+    ab_tests.empty? ?
+      {} :
+      {
+        ab_tests: ab_tests,
+      }
+  end
+
   def browser
     @browser ||= BrowserCache.parse(request.user_agent)
   end

app/services/doc_auth_router.rb

@@ -159,8 +159,8 @@ def translate_generic_errors!(response)
 
   # rubocop:disable Layout/LineLength
   # @param [Proc,nil] warn_notifier proc takes a hash, and should log that hash to events.log
-  def self.client(vendor_discriminator: nil, warn_notifier: nil, analytics: nil)
-    case doc_auth_vendor(discriminator: vendor_discriminator, analytics: analytics)
+  def self.client(vendor:, warn_notifier: nil)
+    case vendor
     when Idp::Constants::Vendors::LEXIS_NEXIS, 'lexisnexis' # Use constant once configured in prod
       DocAuthErrorTranslatorProxy.new(
         DocAuth::LexisNexis::LexisNexisClient.new(
@@ -190,19 +190,32 @@ def self.client(vendor_discriminator: nil, warn_notifier: nil, analytics: nil)
         ),
       )
     else
-      raise "#{doc_auth_vendor(discriminator: vendor_discriminator)} is not a valid doc auth vendor"
+      raise "#{vendor} is not a valid doc auth vendor"
     end
   end
   # rubocop:enable Layout/LineLength
 
-  def self.doc_auth_vendor(discriminator: nil, analytics: nil)
-    case AbTests::DOC_AUTH_VENDOR.bucket(discriminator)
-    when :alternate_vendor
-      IdentityConfig.store.doc_auth_vendor_randomize_alternate_vendor
-    else
-      analytics&.idv_doc_auth_randomizer_defaulted if discriminator.blank?
-
+  def self.doc_auth_vendor_for_bucket(bucket)
+    bucket == :alternate_vendor ?
+      IdentityConfig.store.doc_auth_vendor_randomize_alternate_vendor :
       IdentityConfig.store.doc_auth_vendor
-    end
+  end
+
+  def self.doc_auth_vendor(
+    request:,
+    service_provider:,
+    session:,
+    user:,
+    user_session:
+  )
+    bucket = AbTests::DOC_AUTH_VENDOR.bucket(
+      request:,
+      service_provider:,
+      session:,
+      user:,
+      user_session:,
+    )
+
+    doc_auth_vendor_for_bucket(bucket)
   end
 end

app/services/idv/analytics_events_enhancer.rb

@@ -185,15 +185,19 @@ def profile_history
     def proofing_components
       return if !user
 
+      user_session = session&.dig('warden.user.user.session') || {}
+
       idv_session = Idv::Session.new(
-        user_session: session&.dig('warden.user.user.session') || {},
+        user_session:,
         current_user: user,
         service_provider: sp,
       )
 
       proofing_components_hash = ProofingComponents.new(
-        user:,
         idv_session:,
+        session:,
+        user:,
+        user_session:,
       ).to_h
 
       proofing_components_hash.empty? ? nil : proofing_components_hash

app/services/idv/proofing_components.rb

@@ -3,19 +3,27 @@
 module Idv
   class ProofingComponents
     def initialize(
-        user:,
-        idv_session:
-      )
-      @user = user
+      idv_session:,
+      session:,
+      user:,
+      user_session:
+    )
       @idv_session = idv_session
+      @session = session
+      @user = user
+      @user_session = user_session
     end
 
     def document_check
       if user.establishing_in_person_enrollment || user.pending_in_person_enrollment
         Idp::Constants::Vendors::USPS
       elsif idv_session.remote_document_capture_complete?
         DocAuthRouter.doc_auth_vendor(
-          discriminator: idv_session.document_capture_session_uuid,
+          request: nil,
+          service_provider: idv_session.service_provider,
+          session:,
+          user_session:,
+          user:,
         )
       end
     end
@@ -65,6 +73,6 @@ def to_h
 
     private
 
-    attr_reader :user, :idv_session
+    attr_reader :idv_session, :session, :user, :user_session
   end
 end

app/views/idv/document_capture/show.html.erb

@@ -12,4 +12,5 @@
       skip_doc_auth_from_how_to_verify: skip_doc_auth_from_how_to_verify,
       skip_doc_auth_from_handoff: skip_doc_auth_from_handoff,
       doc_auth_selfie_capture: doc_auth_selfie_capture,
+      mock_client: mock_client,
     ) %>

app/views/idv/hybrid_mobile/document_capture/show.html.erb

@@ -12,4 +12,5 @@
       skip_doc_auth_from_how_to_verify: false,
       skip_doc_auth_from_handoff: nil,
       doc_auth_selfie_capture: doc_auth_selfie_capture,
+      mock_client: mock_client,
     ) %>

app/views/idv/shared/_document_capture.html.erb

@@ -7,7 +7,7 @@
 <%= tag.div id: 'document-capture-form', data: {
       app_name: APP_NAME,
       liveness_required: nil,
-      mock_client: (DocAuthRouter.doc_auth_vendor(discriminator: document_capture_session_uuid) == 'mock').presence,
+      mock_client: mock_client,
       help_center_redirect_url: help_center_redirect_url(
         flow: :idv,
         step: :document_capture,