diff --git a/app/jobs/outdated_ab_test_assignment_cleanup_job.rb b/app/jobs/outdated_ab_test_assignment_cleanup_job.rb new file mode 100644 index 00000000000..35e55c9a9b4 --- /dev/null +++ b/app/jobs/outdated_ab_test_assignment_cleanup_job.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class OutdatedAbTestAssignmentCleanupJob < ApplicationJob + queue_as :low + + def perform + AbTestAssignment.where.not(experiment: active_experiments).destroy_all + end + + private + + def active_experiments + AbTests.all.values.map(&:experiment_name) + end +end diff --git a/app/models/ab_test_assignment.rb b/app/models/ab_test_assignment.rb new file mode 100644 index 00000000000..52fcda0aa55 --- /dev/null +++ b/app/models/ab_test_assignment.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AbTestAssignment < ApplicationRecord + class << self + def bucket(**) + find_by(**)&.bucket&.to_sym + end + end +end diff --git a/app/services/analytics.rb b/app/services/analytics.rb index 8e7c59cf5c4..289aaa92463 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -83,6 +83,7 @@ def ab_test_attributes(event) session:, user:, user_session:, + persisted_read_only: true, ) if !bucket.blank? obj[test_id.downcase] = { diff --git a/config/initializers/ab_tests.rb b/config/initializers/ab_tests.rb index 7150e1d1e14..fa5779dac0b 100644 --- a/config/initializers/ab_tests.rb +++ b/config/initializers/ab_tests.rb @@ -65,6 +65,7 @@ def self.all RECAPTCHA_SIGN_IN = AbTest.new( experiment_name: 'reCAPTCHA at Sign-In', + persist: true, should_log: [ 'Email and Password Authentication', 'IdV: doc auth verify proofing results', @@ -72,11 +73,5 @@ def self.all :idv_enter_password_submitted, ].to_set, buckets: { sign_in_recaptcha: IdentityConfig.store.sign_in_recaptcha_percent_tested }, - ) do |user:, user_session:, **| - if user_session&.[](:captcha_validation_performed_at_sign_in) == false - nil - else - user&.uuid - end - end.freeze + ).freeze end diff --git a/config/initializers/job_configurations.rb b/config/initializers/job_configurations.rb index 2b5f2c7c323..ec229420b22 100644 --- a/config/initializers/job_configurations.rb +++ b/config/initializers/job_configurations.rb @@ -235,6 +235,11 @@ cron: cron_every_monday_2am, args: -> { [Time.zone.yesterday.end_of_day] }, }, + # Outdated A/B test assignment clean-up + outdated_ab_test_assignment_cleanup: { + class: 'OutdatedAbTestAssignmentCleanupJob', + cron: cron_every_monday, + }, }.compact end # rubocop:enable Metrics/BlockLength diff --git a/db/primary_migrate/20240905125027_create_ab_test_assignments.rb b/db/primary_migrate/20240905125027_create_ab_test_assignments.rb new file mode 100644 index 00000000000..80c3a00c968 --- /dev/null +++ b/db/primary_migrate/20240905125027_create_ab_test_assignments.rb @@ -0,0 +1,10 @@ +class CreateAbTestAssignments < ActiveRecord::Migration[7.1] + def change + create_table :ab_test_assignments do |t| + t.string :experiment, null: false + t.string :discriminator, null: false + t.string :bucket, null: false + t.index [:experiment, :discriminator], unique: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 385018287e3..59fb43d35bf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,12 +10,20 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_08_28_182041) do +ActiveRecord::Schema[7.1].define(version: 2024_09_05_125027) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_stat_statements" + enable_extension "pgcrypto" enable_extension "plpgsql" + create_table "ab_test_assignments", force: :cascade do |t| + t.string "experiment", null: false + t.string "discriminator", null: false + t.string "bucket", null: false + t.index ["experiment", "discriminator"], name: "index_ab_test_assignments_on_experiment_and_discriminator", unique: true + end + create_table "account_reset_requests", force: :cascade do |t| t.integer "user_id", null: false t.datetime "requested_at", precision: nil @@ -664,7 +672,7 @@ add_foreign_key "iaa_gtcs", "partner_accounts" add_foreign_key "iaa_orders", "iaa_gtcs" add_foreign_key "in_person_enrollments", "profiles" - add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer", validate: false + add_foreign_key "in_person_enrollments", "service_providers", column: "issuer", primary_key: "issuer" add_foreign_key "in_person_enrollments", "users" add_foreign_key "integration_usages", "iaa_orders" add_foreign_key "integration_usages", "integrations" diff --git a/lib/ab_test.rb b/lib/ab_test.rb index 1cc266c215f..bc663f09ef3 100644 --- a/lib/ab_test.rb +++ b/lib/ab_test.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true class AbTest - attr_reader :buckets, :experiment_name, :default_bucket, :should_log + attr_reader :buckets, :experiment_name, :default_bucket, :should_log, :persist + alias_method :experiment, :experiment_name MAX_SHA = (16 ** 64) - 1 @@ -18,6 +19,7 @@ def initialize( buckets: {}, should_log: nil, default_bucket: :default, + persist: false, &discriminator ) @buckets = buckets @@ -25,6 +27,7 @@ def initialize( @experiment_name = experiment_name @default_bucket = default_bucket @should_log = should_log + @persist = persist raise 'invalid bucket data structure' unless valid_bucket_data_structure? ensure_numeric_percentages raise 'bucket percentages exceed 100' unless within_100_percent? @@ -36,25 +39,44 @@ def initialize( # @params [Hash] session # @param [User] user # @param [Hash] user_session - def bucket(request:, service_provider:, session:, user:, user_session:) - return nil if no_percentages? - + # @param [Boolean] persisted_read_only Avoid new bucket assignment if test is configured to be + # persisted but there is no persisted value. + def bucket( + request:, + service_provider:, + session:, + user:, + user_session:, + persisted_read_only: false + ) discriminator = resolve_discriminator( request:, service_provider:, session:, user:, user_session: ) return nil if discriminator.blank? + persisted_value = AbTestAssignment.bucket(experiment:, discriminator:) if persist + return persisted_value if persisted_value || (persist && persisted_read_only) + + return nil if no_percentages? + user_value = percent(discriminator) + bucket = @default_bucket + min = 0 buckets.keys.each do |key| max = min + buckets[key] - return key if user_value > min && user_value <= max + if user_value > min && user_value <= max + bucket = key + break + end min = max end - @default_bucket + AbTestAssignment.create(experiment:, discriminator:, bucket:) if persist + + bucket end def include_in_analytics_event?(event_name)