From 60cb87fe1c6c87c2176ba092919face1b268d0b8 Mon Sep 17 00:00:00 2001 From: John Mileham Date: Mon, 23 Apr 2018 14:54:30 -0400 Subject: [PATCH] Don't record assignment events for feature gates (#69) * Don't record assignment events for feature gates Because feature gates are not used for analysis, there's no need to record anything but overrides for feature gates. This will vastly reduce the data that needs to change when deciding a feature gate split. It also will cause reweighting feature gates to affect clients in the field instantly unless they've been overridden. Also explain the difference between experiments and gates to admins. * more clarity on why specific variants are assigned * preposition * Let's do this. * :cop: * don't use the word population for gates because we use it in the admin to mean "recorded assignees of experiments." --- README.md | 38 +++++++++++++++++++ .../deterministic_assignment_creation.rb | 16 ++++---- app/models/split_creation.rb | 6 ++- app/views/admin/split_configs/new.html.erb | 18 ++++++++- app/views/admin/splits/_details.html.erb | 10 ++++- app/views/admin/splits/_variants.html.erb | 2 +- app/views/admin/variant_details/edit.html.erb | 2 +- .../20180412153251_add_split_feature_gate.rb | 6 +++ db/schema.rb | 7 ++-- .../deterministic_assignment_creation_spec.rb | 12 ++++++ spec/models/split_creation_spec.rb | 8 +++- 11 files changed, 109 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20180412153251_add_split_feature_gate.rb diff --git a/README.md b/README.md index eedf277d..872fbb6a 100644 --- a/README.md +++ b/README.md @@ -154,6 +154,44 @@ One the values that a given visitor will be assigned for a split, e.g. `true` or ### Weighting Variants are assigned pseudo-randomly to visitors based on their visitor IDs and the weightings for the variants. Weightings describe the probability of a visitor being assigned to a given variant in integer percentages. All the variant weightings for a given split must sum to 100, though variants may have a weighting of 0. +### Experiment + +Experiments are the standard flavor of splits in TestTrack. They are +intended to be used for A/B testing, and the TestTrack server records +visitors' experienced variants so that those visitors will continue to +experience the same variant regardless of subsequent changes to the +weightings of those variants via the admin interface. + +Storing the variant a visitor experienced for an experiment also allows +TestTrack to provide a consistent UX to a customer who experienced a +new-to-them experiment before logging in on a new device, only to be +recognized as an existing visitor upon sign-in. TestTrack will merge +all variant assignments from the anonymous visitor into the +authenticated visitor at sign-in as long as the authenticated visitor +doesn't have conflicting assignments. In that case, the authenticated +visitor's previous assignments win. + +### Feature Gate + +As of TestTrack version 1.2, splits with names ending in the `_enabled` +suffix will be treated as feature gates. Feature gates differ from +experiments in that they are not intended to be used for analysis, and +therefore it is not important that the user remain in the same variant +throughout the entire split lifecycle. Feature gates are meant to be +slow-rolled (incrementally increasing the percentage of customers +experiencing the new feature), released en masse, or instantly rolled +back. + +To facilitate these smooth transitions and rapid toggles, the TestTrack +server will not record variant assignments when a visitor experiences a +split. This means that every time a visitor experiences a split, they +will be deterministically (pseudorandomly) assigned to a variant based +on their visitor ID and the name of the split. This will provide the +customer with a stable variant given a constant split weighting, but +probablistically increase the percentage of visitors experiencing the +the `true` variant as the split weightings are increased via the admin +panel, giving an admin full control over the feature's release. + ### IdentifierType A name for a customer identifier that is meaningful in your application, typically things that people sign up as, log in as. They should be expressed in `snake_case` and conventionally are prefixed with the application name that the identifier is for, e.g. `myapp_user_id`, `myapp_lead_id`. diff --git a/app/models/deterministic_assignment_creation.rb b/app/models/deterministic_assignment_creation.rb index 2f02dba5..5731e7c6 100644 --- a/app/models/deterministic_assignment_creation.rb +++ b/app/models/deterministic_assignment_creation.rb @@ -14,13 +14,15 @@ def self.create!(params) end def save! - ArbitraryAssignmentCreation.create!( - visitor_id: visitor_id, - split_name: split_name, - variant: variant, - mixpanel_result: mixpanel_result, - context: context - ) + unless split.feature_gate? + ArbitraryAssignmentCreation.create!( + visitor_id: visitor_id, + split_name: split_name, + variant: variant, + mixpanel_result: mixpanel_result, + context: context + ) + end end def variant diff --git a/app/models/split_creation.rb b/app/models/split_creation.rb index 19d70c27..d9a8dd77 100644 --- a/app/models/split_creation.rb +++ b/app/models/split_creation.rb @@ -25,7 +25,11 @@ def weighting_registry=(registry) end def split - @split ||= app.splits.create_with(registry: merged_registry).find_or_initialize_by(name: name) + @split ||= app.splits.create_with(registry: merged_registry, feature_gate: feature_gate?).find_or_initialize_by(name: name) + end + + def feature_gate? + name.end_with?("_enabled") end private diff --git a/app/views/admin/split_configs/new.html.erb b/app/views/admin/split_configs/new.html.erb index 58d21189..c2bb6f9c 100644 --- a/app/views/admin/split_configs/new.html.erb +++ b/app/views/admin/split_configs/new.html.erb @@ -5,6 +5,22 @@

Split: <%= @split_creation.name %>

+<% if @split_creation.feature_gate? %> +

+ This split is a feature gate. Changing weights will immediately change + behavior of visitors who do not have an explicit assignment, even if they've + already experienced a specific variant of this split. This is usually + desirable for slow-rolling features. +

+<% else %> +

+ This split is an experiment. Changing weights will have no immediate effect + on the behavior of visitors who have already experienced a variant of this + split. Experiments rarely benefit from changing weightings unless you are + performing analysis over a date range. +

+<% end %> + <%= simple_form_for(@split_creation, url: admin_split_split_config_path) do |f| %> <% f.simple_fields_for :weighting_registry do |ff| %> @@ -18,6 +34,6 @@ <%= ff.input variant, as: :percent, input_html: { value: weight, class: "weight-input" } %> <% end %> - <%= render "shared/form_footer", f: f, submit_text: "Edit", submit_disable_with_text: "Changing..." %> + <%= render "shared/form_footer", f: f, submit_text: "Save", submit_disable_with_text: "Changing..." %> <% end %> <% end %> diff --git a/app/views/admin/splits/_details.html.erb b/app/views/admin/splits/_details.html.erb index a22e8cac..7a9c497e 100644 --- a/app/views/admin/splits/_details.html.erb +++ b/app/views/admin/splits/_details.html.erb @@ -9,7 +9,9 @@ Population Size - <%= @split.assignments.count %> + + <%= @split.assignments.count %><% if @split.feature_gate? %>* [feature gate]<% end %> + <%= link_to "Edit", new_admin_split_bulk_assignment_path(@split), class: 'upload-new-assignments-link' %> @@ -33,5 +35,11 @@   + <% if @split.feature_gate? %> +

+ + * Feature gates no longer track assignment events and population reflects only visitors assigned to specific variants via the chrome extension or admin tool. +

+ <% end %> diff --git a/app/views/admin/splits/_variants.html.erb b/app/views/admin/splits/_variants.html.erb index 1984b9b2..f878eef8 100644 --- a/app/views/admin/splits/_variants.html.erb +++ b/app/views/admin/splits/_variants.html.erb @@ -1,7 +1,7 @@

Variant Details

- <%= link_to "Edit", new_admin_split_split_config_path(split), class: 'change-weights-link' %> + <%= link_to "Change Weights", new_admin_split_split_config_path(split), class: 'change-weights-link' %>
diff --git a/app/views/admin/variant_details/edit.html.erb b/app/views/admin/variant_details/edit.html.erb index 1d9c82ae..f52b96aa 100644 --- a/app/views/admin/variant_details/edit.html.erb +++ b/app/views/admin/variant_details/edit.html.erb @@ -22,7 +22,7 @@ method: :post, data: { confirm: "You're redistributing #{@variant_detail.variant} assignees to the other variants according to their weights. Do you wish to proceed?" } %> <% end %> - <%= f.submit 'Continue', data: { disable_with: 'Updating variant...' }, class: 'u-button ft-confirmButton' %> + <%= f.submit 'Save', data: { disable_with: 'Updating variant...' }, class: 'u-button ft-confirmButton' %>
<% end %> diff --git a/db/migrate/20180412153251_add_split_feature_gate.rb b/db/migrate/20180412153251_add_split_feature_gate.rb new file mode 100644 index 00000000..f5253328 --- /dev/null +++ b/db/migrate/20180412153251_add_split_feature_gate.rb @@ -0,0 +1,6 @@ +class AddSplitFeatureGate < ActiveRecord::Migration[5.0] + def change + add_column :splits, :feature_gate, :boolean, default: false, null: false + execute "update splits set feature_gate = true where name like '%_enabled'" + end +end diff --git a/db/schema.rb b/db/schema.rb index 26e9a978..b337a2ca 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170501180350) do +ActiveRecord::Schema.define(version: 20180412153251) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -133,17 +133,18 @@ create_table "splits", id: :uuid, default: -> { "uuid_generate_v4()" }, force: :cascade do |t| t.string "name" - t.uuid "owner_app_id", null: false + t.uuid "owner_app_id", null: false t.datetime "created_at" t.datetime "updated_at" t.datetime "finished_at" - t.json "registry", null: false + t.json "registry", null: false t.text "hypothesis" t.text "assignment_criteria" t.text "description" t.string "owner" t.string "location" t.integer "platform" + t.boolean "feature_gate", default: false, null: false t.index ["name"], name: "index_splits_on_name", unique: true, using: :btree t.index ["owner_app_id"], name: "index_splits_on_owner_app_id", using: :btree end diff --git a/spec/models/deterministic_assignment_creation_spec.rb b/spec/models/deterministic_assignment_creation_spec.rb index ca6fd56a..8aa0ada7 100644 --- a/spec/models/deterministic_assignment_creation_spec.rb +++ b/spec/models/deterministic_assignment_creation_spec.rb @@ -70,6 +70,18 @@ expect(ArbitraryAssignmentCreation).to have_received(:create!) .with(hash_including(context: "the_context")) end + + context "with a feature gate" do + let!(:split) do + FactoryBot.create(:split, name: "split", registry: { variant1: 61, variant2: 1, variant3: 38 }, feature_gate: true) + end + + it "skips creating for feature gates" do + subject.save! + + expect(ArbitraryAssignmentCreation).not_to have_received(:create!) + end + end end describe "#variant_calculator" do diff --git a/spec/models/split_creation_spec.rb b/spec/models/split_creation_spec.rb index 19d13069..d16d4d11 100644 --- a/spec/models/split_creation_spec.rb +++ b/spec/models/split_creation_spec.rb @@ -12,7 +12,13 @@ it 'creates a new split config for a new name' do expect(Split.find_by(name: "amazing")).to be_falsey SplitCreation.create(app: default_app, name: "amazing", weighting_registry: { awesome: 100 }) - expect(Split.find_by(name: "amazing")).to be_truthy + expect(Split.find_by(name: "amazing", feature_gate: false)).to be_truthy + end + + it 'creates feature gates when name ends in _enabled' do + expect(Split.find_by(name: "foo_enabled")).to be_falsey + SplitCreation.create(app: default_app, name: "foo_enabled", weighting_registry: { awesome: 100 }) + expect(Split.find_by(name: "foo_enabled", feature_gate: true)).to be_truthy end it 'updates existing split config for a known name' do