Skip to content

LG-14095: set up A/B testing for Socure#11104

Merged
solipet merged 4 commits intomainfrom
dprice/lg-14095-socure-ab-test
Aug 28, 2024
Merged

LG-14095: set up A/B testing for Socure#11104
solipet merged 4 commits intomainfrom
dprice/lg-14095-socure-ab-test

Conversation

@solipet
Copy link
Contributor

@solipet solipet commented Aug 16, 2024

🎫 Ticket

Link to the relevant ticket:
LG-14095

🛠 Summary of changes

This sets up a new A/B test to allow management of the distribution of doc_auth requests between LexisNexis TrueID and Socure.

There is an already existing A/B test for swapping out doc_auth vendors, DOC_AUTH_VENDOR, but that tests parameters are integrated with the current code base and is really set up for TrueID vs Acuant, two very similar vendors. Given that the Socure integration will be significantly different, I chose to start a new test, and expect that in time it will replace the preceding one.

@solipet solipet requested review from a team and theabrad and removed request for a team August 16, 2024 20:55
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we use/need this?

Copy link
Contributor Author

@solipet solipet Aug 19, 2024

Choose a reason for hiding this comment

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

Right now? Technically no, but that's true of the whole PR 😉

Copy link
Contributor

@amirbey amirbey Aug 21, 2024

Choose a reason for hiding this comment

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

@solipet
can you please share what this variable does?
currently we have the similarly named doc_auth_vendor variable and I'm worried this new variable could cause confusion

to me it seems the a/b options here are Socure Capture App vs our ReactApp? what are your thought about the variable namings leaning towards the capture experience?
or even better, based on @matthinz incoming A/B implementation it appears that we can have three buckets {a: 'mock', b: 'lexis_nexis', c: 'socure' } which is probably the optimal solution? and we could disregard this need for this new variable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my mind, doc_auth_vendor_default would eventually replace doc_auth_vendor as I think it's more descriptive of its intent - in the absence of an enabled A/B test, this is the default so use it. Currently, doc_auth_vendor sounds like it names the vendor in use, but that may be overridden by the A/B test.

I chose not to simply rename the var as I wasn't looking to disturb the status quo. Having now looked at where it is used, it's trivial to just rename it so I'm inclined to do that. I would add the default var (and use it) in one PR, then once that is deployed remove the older var. Sound good?

Copy link
Contributor

@amirbey amirbey Aug 21, 2024

Choose a reason for hiding this comment

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

hi @solipet -

in your implementation, it appears we have 3 vendors and 2 doc_auth A/B testing mechanisms with 2 buckets each:

  1. (mock, lexisnexis) doc_auth_vendor_randomize
  2. (socure, lexisnexis/mock?) doc_auth_vendor_switching_enabled

now that we have 3 vendors (mock, socure, and lexis_nexis), I think it would be simpler to maintain having 1 doc_auth A/B test but with 3 buckets [mock, socure, lexis_nexis]. We would then use the percentage values per bucket to control the routing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I've taken over the DOC_AUTH_VENDOR a/b test and removed the randomize vars. Originally, the test was between lexisnexis and acuant. For sandbox envs, the default is mock and the test is disabled. For staging/prod, default is lexis_nexis and the test let's us set a percentage to go to Socure.

@solipet solipet force-pushed the dprice/lg-14095-socure-ab-test branch from c36af50 to e2c73b8 Compare August 19, 2024 17:56
@solipet solipet requested a review from jmax-gsa August 19, 2024 18:01
Copy link
Contributor

@jmax-gsa jmax-gsa left a comment

Choose a reason for hiding this comment

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

LGTM

@amirbey amirbey self-requested a review August 21, 2024 15:08
This sets up a new A/B test to allow management of the distribution of
doc_auth requests between LexisNexis TrueID and Socure.

There is an already existing A/B test for swapping out doc_auth vendors,
DOC_AUTH_VENDOR, but that tests parameters are integrated with the
current code base and is really set up for TrueID vs Acuant, two very
similar vendors. Given that the Socure integration will be significantly
different, I chose to start a new test, and expect that in time it will
replace the preceding one.

[skip changelog]
@solipet solipet force-pushed the dprice/lg-14095-socure-ab-test branch from e2c73b8 to 50f9e98 Compare August 21, 2024 20:19
@solipet
Copy link
Contributor Author

solipet commented Aug 21, 2024

Rebased on main to pick up Matt's work in #11026
Uses doc_auth_vendor_default in prep for removing doc_auth_vendor

@solipet solipet force-pushed the dprice/lg-14095-socure-ab-test branch from 856a2a6 to 18fddbf Compare August 21, 2024 21:14
track_event('IdV: doc auth link_sent visited', **extra)
end

def idv_doc_auth_randomizer_defaulted(**extra)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This turned out to be unused.

case bucket
when :socure
Idp::Constants::Vendors::SOCURE
when :lexis_nexis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is here since they are a vendor, even though they are currently set to the default.

Copy link
Contributor

@amirbey amirbey Aug 26, 2024

Choose a reason for hiding this comment

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

this would never get a hit in the current implementation right? we would have to add a :lexis_nexis bucket in DOC_AUTH_VENDOR, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct

Copy link
Contributor

Choose a reason for hiding this comment

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

i'm wondering if there is a comment that could be added to make this clear ... so that someone could know that extra code is needed to make this bucket possible to be hit

Copy link
Contributor

@amirbey amirbey left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏿 ... pls see comment about lexis_nexis bucket in doc_auth_router.rb

@solipet solipet merged commit 7d7d2b7 into main Aug 28, 2024
@solipet solipet deleted the dprice/lg-14095-socure-ab-test branch August 28, 2024 17:34
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.

5 participants