Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LG-15187: Update Socure shadow mode A/B test logic (part 2/2) #11694

Merged
merged 14 commits into from
Jan 7, 2025

Conversation

matthinz
Copy link
Member

@matthinz matthinz commented Dec 31, 2024

🎫 Ticket

Link to the relevant ticket:
LG-15187

🛠 Summary of changes

This PR updates the logic ResolutionProofingJob uses to decide whether to kick off a SocureShadowModeProofingJob.

We're going from a single A/B test configuration, where a user is either in or not in the shadow mode test, to a more complex setup:

  1. If a user already went through Socure Docv, they can be enrolled in shadow mode testing
  2. We may also want to enroll a small percentage of users who did not go through Socure Docv in shadow mode
  3. We still want a global "off" switch for shadow mode testing

Thus this PR reworks the config structure slightly:

Config Description
idv_socure_shadow_mode_enabled If false, no shadow mode jobs will be scheduled.
idv_socure_shadow_mode_enabled_for_docv_users If true, users who went through Socure Docv will be included in shadow mode tests.
socure_idplus_shadow_mode_percent Percentage (0-100) of additional users to select for the shadow mode test on top of docv users.

Here is a table laying out the relationship between the various variables:

Scenario Shadow mode enabled globally? Shadow mode enabled for docv users? User went through Docv? User selected for A/B test? User included in shadow mode?
1 false N/A N/A N/A
2 true false N/A true
3 true false N/A false
4 true true true N/A
5 true true false true
6 true true false true

📜 Testing Plan

Before testing, make sure that shadow mode can run locally by configuring the connection to the Socure sandbox:

  socure_idplus_api_key: '<KEY FROM SOCURE DASHBOARD>'
  socure_idplus_base_url: 'https://sandbox.socure.us'

The easiest way to check whether the SocureShadowModeProofing job has run is to watch for the idv_socure_shadow_mode_proofing_result event in your logs.

Scenario 1: Shadow mode disabled

  1. Set idv_socure_shadow_mode_enabled: false
  2. Verify your identity up until the phone verification screen
  3. Check that no SocureShadowModeProofingJob ran

Scenario 2: shadow mode enabled, user included in A/B test

  1. Set idv_socure_shadow_mode_enabled: true and socure_idplus_shadow_mode_percent: 100
  2. Verify your identity up until the phone verification screen
  3. Check that aSocureShadowModeProofingJob ran

Scenario 3: shadow mode enabled, user not included in A/B test

  1. Set idv_socure_shadow_mode_enabled: true and socure_idplus_shadow_mode_percent: 0
  2. Verify your identity up until the phone verification screen
  3. Check that no SocureShadowModeProofingJob ran

🚨 These next scenarios require some setup outlined in here 🚨

To avoid having to wire up Socure doc auth locally, you can use a mocked Socure doc auth provider.

First, bring in the mocked provider implementation:

git cherry-pick 4300dd34c84f92063bdb74cc87abf105ce78dd26

Then, apply this so that mock socure users are treated as docv users:

diff --git a/app/jobs/resolution_proofing_job.rb b/app/jobs/resolution_proofing_job.rb
index 10f7b1baa4..e9c3cf085e 100644
--- a/app/jobs/resolution_proofing_job.rb
+++ b/app/jobs/resolution_proofing_job.rb
@@ -97,7 +97,10 @@ class ResolutionProofingJob < ApplicationJob
     # are thus eligible for shadow mode.
     enabled_for_docv_users =
       IdentityConfig.store.idv_socure_shadow_mode_enabled_for_docv_users
-    is_docv_user = proofing_components&.dig(:document_check) == Idp::Constants::Vendors::SOCURE
+    is_docv_user =
+      proofing_components&.dig(:document_check) == Idp::Constants::Vendors::SOCURE ||
+      proofing_components&.dig(:document_check) == Idp::Constants::Vendors::MOCK_SOCURE
+
     return true if enabled_for_docv_users && is_docv_user
 
     # Otherwise fall back to A/B test

(To apply, copy the patch, then do pbpaste | git apply in your terminal)

Then, you need to modify your application.yml to enable Socure docv:

  socure_docv_enabled: true  
  doc_auth_vendor_default: 'mock_socure'

Scenario 4: shadow mode enabled for docv users, user goes through docv

  1. Set idv_socure_shadow_mode_enabled: true
  2. Verify your identity up until the phone verification screen (you should go through mock Socure doc auth)
  3. Check that a SocureShadowModeProofingJob ran

Scenario 5: shadow mode enabled for docv users, user does not go through docv, but is selected for A/B test

  1. Set idv_socure_shadow_mode_enabled: true, doc_auth_vendor_default: 'mock', and socure_idplus_shadow_mode_percent: 100
  2. Verify your identity up until the phone verification screen (you should not go through mock Socure doc auth)
  3. Check that a SocureShadowModeProofingJob ran

Scenario 6: shadow mode enabled for docv users, user does not go through docv and is not selected for A/B test

  1. Set idv_socure_shadow_mode_enabled: true, doc_auth_default: 'mock', and socure_idplus_shadow_mode_percent: 0
  2. Verify your identity up until the phone verification screen (you should not go through mock Socure doc auth)
  3. Check that a SocureShadowModeProofingJob ran

lmgeorge and others added 12 commits December 20, 2024 18:00
Adds a new parameter to ResolutionProofingJob prior to implementation,
so that the new signature method can be used once all job queues are
running the same code.

[skip changelog]
Update this spec to use more let and subject blocks ahead of adding new tests to it.

[skip changelog]
Assert we're actually passing proofing components into ResolutionProofingJob
Handle logic around shadow mode enabled / disabled globally, shadow mode enabled for docv users, and shadow mode a/b test for non-docv users.
Tests can get stuck with mocked configs, leading to flakiness.

[skip changelog]
@matthinz matthinz marked this pull request as ready for review December 31, 2024 21:17
@matthinz matthinz requested a review from a team December 31, 2024 21:18
Copy link
Member

@n1zyy n1zyy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some trivial notes, but otherwise this looks good to me!

app/jobs/resolution_proofing_job.rb Outdated Show resolved Hide resolved
@@ -75,6 +79,7 @@
let(:session) do
{ document_capture_session_uuid: 'a-random-uuid' }
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@@ -97,11 +97,11 @@ def self.all
},
).freeze

SOCURE_IDV_SHADOW_MODE = AbTest.new(
SOCURE_IDV_SHADOW_MODE_FOR_NON_DOCV_USERS = AbTest.new(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an idle musing from someone who hasn't looked at IDP code since last year: I think if I didn't have all the context of this PR in mind, I'd expect this to have a SOCURE_IDV_SHADOW_MODE_FOR_DOCV_USERS corollary. This isn't particularly sensible, of course.

Co-authored-by: Matt Wagner <matt.wagner@gsa.gov>
@matthinz matthinz merged commit a46faf6 into main Jan 7, 2025
2 checks passed
@matthinz matthinz deleted the lmgeorge/LG-15187_2 branch January 7, 2025 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants