Don't record assignment events for feature gates by jmileham · Pull Request #69 · Betterment/test_track

jmileham · 2018-04-12T17:35:07Z

Summary

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.

Other Information

This is compatible with all clients currently in the field, though behavior will change a bit. The chrome extension will reflect that no matter how many times you've experienced a gate, you are not assigned to a variant unless you assigned it via the extension or admin interface (or before this upgrade took place).

This should make slow rolling features much easier.

This is currently heuristically tagging splits as feature gates based on an _enabled suffix. The goal would be to later make creating feature gates (as distinct from experiments) a first class feature of the API so that you provision them differently, and for example, you only get true/false variants from a gate. But this is a very small step code-wise which should hopefully make a big difference to users of splits at scale that otherwise took a lot of dancing to slow roll.

/domain @Betterment/test_track_core
/domain @ceslami @klesse413 @betterzega or anybody else who has done big slow roll releases who has opinions.

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.

nanda-prbot · 2018-04-12T17:35:10Z

@jmileham needs to request domain and platform reviewers, or explicitly opt out, e.g.:

/no-platform

jmileham · 2018-04-12T17:38:08Z

/platform @klesse413 @aburgel @samandmoore @smudge @effron

nanda-prbot · 2018-04-12T17:38:10Z

Needs somebody from @Betterment/test_track_core to claim domain review
Needs somebody from @ceslami, @klesse413, and @betterzega to claim domain review
Needs somebody from @klesse413, @aburgel, @samandmoore, @smudge, and @effron to claim platform review

Use the shovel operator to claim, e.g.:

@myname << domain && platform

jmileham · 2018-04-12T17:53:03Z

@@ -0,0 +1,6 @@
+class AddSplitFeatureGate < ActiveRecord::Migration[5.0]
+  def change
+    add_column :splits, :feature_gate, :boolean, default: false, null: false


This is a high read throughput table but very small number of rows and there are effectively no long-running transactions on this service. My hope is this doesn't cause issues on deploy. :) /cc @Betterment/data for commentary.

👍 not sure what our tt deploy pattern is, maybe we want to deploy to stage first and watch the migration?

Won’t do much for lock contention confidence because of the vastly different load levels but yeah. Will do this and the other off hours unless data says no.

betterzega · 2018-04-12T18:08:50Z

+  end
+
+  def feature_gate?
+    name.end_with?("_enabled")


Would we want something more explicit on the split to tag the type of split coming from the client?

hm yeah, long term i would think probably something more explicit (although shorter term this gets the job done). we use this naming convention, but that's not to say others necessarily do. do we recommend it in documentation anywhere?

I believe so but it might just be in a blog post or something.

Yup, here's the blog post by the man himself @joejansen

🆒 no external documentation though right? seems worth highlighting for other users that they'll get this functionality based on naming convention

added to readme

asullivan210 · 2018-04-12T22:38:57Z

+    <% if @split.feature_gate? %>
+      <p>
+      <a id="gate_population"></a>
+      * Feature gates do not track assignment events and population reflects only visitors assigned to specific variants.


👍 can we make this even more explicit, maybe only visitors assigned to specific variants via the chrome extension or admin tool or something to make clear that if 10 Betterment employees have it on it will show population 10 even if the weight is 20% true and thousands fo people have seen the true experience

asullivan210 · 2018-04-12T22:40:33Z

-      context: context
-    )
+    unless split.feature_gate?
+      ArbitraryAssignmentCreation.create!(


chatted offline, @jmileham pointed out that if product still wants to know how many people have seen the enabled experience (e.g. we've turned the feature flag on for 20% of people and want to gauge if there are any blocking bugs) we can query redshift's customer_prod.split_assigned table because segment will still track assignment events 💯

excellent. for debugging purposes i think there is value in continuing to fire the SplitAssigned event to the analytics client that is configured by the client code.

jmileham · 2018-04-12T22:42:03Z

/domain @joejansen for analytics backend impact (that makes 3 domain reviews, I don't wanna jump the gun on this)

nanda-prbot · 2018-04-12T22:42:05Z

Needs somebody from @Betterment/test_track_core to claim domain review
Needs somebody from @ceslami, @klesse413, and @betterzega to claim domain review
Needs somebody from @klesse413, @aburgel, @samandmoore, @smudge, and @effron to claim platform review
Needs somebody from @joejansen to claim domain review

Use the shovel operator to claim, e.g.:

@myname << domain && platform

asullivan210 · 2018-04-12T22:42:07Z

nanda?

nanda-prbot · 2018-04-12T22:42:07Z

Needs somebody from @Betterment/test_track_core to claim domain review
Needs somebody from @ceslami, @klesse413, and @betterzega to claim domain review
Needs somebody from @klesse413, @aburgel, @samandmoore, @smudge, and @effron to claim platform review
Needs somebody from @joejansen to claim domain review

Use the shovel operator to claim, e.g.:

@myname << domain && platform

asullivan210 · 2018-04-12T22:42:29Z

<<domain

nanda-prbot · 2018-04-12T22:42:31Z

Needs somebody from @ceslami, @klesse413, and @betterzega to claim domain review
Needs somebody from @klesse413, @aburgel, @samandmoore, @smudge, and @effron to claim platform review
Needs somebody from @joejansen to claim domain review

Use the shovel operator to claim, e.g.:

@myname << domain && platform

klesse413 · 2018-04-13T00:41:22Z

+<% else %>
+  <p>
+  This split is an experiment. Changing weights will have no immediate effect
+  on the behavior of visitors who have already experienced a variant this


a variant of this split?

🤦‍♂️

jmileham · 2018-04-13T01:44:57Z

So in the interest of transparency, this will result in duplicate assignment events per visitor for gates experienced more than once. Normally a client retrieves all assignments from TT before displaying any content to the visitor, and if an assignment for a given split exists, it'll just render it. But since the server will have no record of an assignment, every single time a customer experiences a gate, another assignment event will be triggered. This will both hit segment and TT. So it'll increase event volume a bit for a given level of traffic, and if we cared about the event data (which we do not) it would be misleadingly noisey, with duplicate events happening a fair amount.

In future iterations of client libs we can phase out sending assignment events for feature gates and focus on only sending experience events, which we'd want to send each time.

samandmoore · 2018-04-13T01:48:08Z

@jmileham can you spec that out as a GitHub issue in one of the clients like the ruby one? And then link it up in a corresponding issue in the js client? It’d be great to have an record of that.

jmileham · 2018-04-13T02:06:30Z

Another side effect of this strategy is that signing in from a new computer or any other activity that ends up swapping your visitor ID will likely cause you to end up in a different variant for a given feature gate than you would have as your anonymous visitor, even if you've never experienced that feature gate before. Normal visitor supersession logic would've applied the variant from your anonymous visitor to your authenticated visitor if the authenticated visitor had no such assignment, but in this case, you will simply get a deterministic assignment appropriate to your visitor ID each time.

This isn't a big change for most folks. Once you've signed into a device, if you're the only user of the device, your visitor ID will remain constant between sessions so you'll have a consistent experience. And if multiple users use a device, they will have already encountered the same issue as they swap back and forth between their visitor IDs upon login.

jmileham · 2018-04-13T02:39:29Z

+customer with a stable variant given a constant split weighting, but
+probablistically increase the size of the population 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.


@klesse413 - I wrote out the definitions and behaviors of experiments and feature gates in the readme. I'm hypothetically calling this version 1.2. It seems big enough to be worth a point release, but I kinda want to save 2.0 for the version that enables a different migration DSL API contract for defining experiments vs gates, and does things like force them to have conventional names and variants.

klesse413 · 2018-04-13T14:50:36Z

+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
+therefor it is not important that the user remain in the same variant


therefor 👮

klesse413 · 2018-04-13T14:56:04Z

+customer with a stable variant given a constant split weighting, but
+probablistically increase the size of the population 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.


won't we only store the assignment if a customer is explicitly assigned via the admin panel? i.e. if we just increase the true weighting on incremental rollout, couldn't a customer randomly experience the true version and then come back and randomly experience the false version? so we have to incrementally release by explicit assignment of customer ids? or am i missing something?

The algo works something like this:

truncate_to_integer_and_mod_100(sha1(visitor_id + split_name))

we then sort the variants alphabetically and then assign them ranges making up the numbers between 0 and 99. You experience the variant that your number buckets to.

So as we ratchet up or down, the same users for a given split will be rolled out to and rolled back from in deterministic, pseudorandom order per split.

That said, I probably shouldn't use the word population in this definition in a way that differs from the population in the admin - none of these visitors will show up in the population in the admin, because they won't have any concrete assignments.

(this would work horribly if you had more than two variants in a gate as the middle variant would slide through the population, but what kind of gate has more than two sides? this is why we wanna formalize the API for making gates.)

because we use it in the admin to mean "recorded assignees of experiments."

jmileham · 2018-04-13T19:32:03Z

Alright, I'm being needy.

@klesse413 << domain platform - you can't comment this much without providing a review :)
@joejansen << domain

Would love to land this because it shouldn't be disruptive and will make feature rollouts Much Easier.

nanda-prbot · 2018-04-13T19:32:05Z

Needs @asullivan210 to provide domain review
Needs @klesse413 to provide platform review
Needs @klesse413 to provide domain review
Needs @joejansen to provide domain review

When you finish a round of review, be sure to say you've finished or sign off on the PR, e.g.:

TAFN or DomainLGTM

If you're too busy to review, unclaim the PR, e.g.:

@myname >> domain

joejansen · 2018-04-13T19:47:56Z

domainlgtm

nanda-prbot · 2018-04-13T19:47:58Z

Needs @asullivan210 to provide domain review
Needs @klesse413 to provide platform review
Needs @klesse413 to provide domain review

When you finish a round of review, be sure to say you've finished or sign off on the PR, e.g.:

TAFN or DomainLGTM

If you're too busy to review, unclaim the PR, e.g.:

@myname >> domain

klesse413 · 2018-04-13T19:55:15Z

  <div class="InfoCard-titleContainer">
    <h4>Variant Details</h4>
-    <%= 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' %>


🙌 👍 for more descriptive links. ive always been confused by the 5 different "edit" links on the split page - when I want to assign a user I never know which one to click 😂

klesse413 · 2018-04-13T19:57:22Z

domainlgtm platformlgtm 🙌 my team is super excited about this!

nanda-prbot · 2018-04-13T19:57:24Z

Needs @asullivan210 to provide domain review

When you finish a round of review, be sure to say you've finished or sign off on the PR, e.g.:

TAFN or DomainLGTM

If you're too busy to review, unclaim the PR, e.g.:

@myname >> domain

asullivan210 · 2018-04-13T20:58:25Z

domainLGTM

nanda-prbot · 2018-04-13T20:58:27Z

Approved! 🙆‍♀️ 🙏 🙌

jmileham commented Apr 12, 2018

View reviewed changes

betterzega reviewed Apr 12, 2018

View reviewed changes

asullivan210 reviewed Apr 12, 2018

View reviewed changes

more clarity on why specific variants are assigned

e859e22

klesse413 reviewed Apr 13, 2018

View reviewed changes

preposition

10739a6

Let's do this.

6e6b426

jmileham commented Apr 13, 2018

View reviewed changes

This was referenced Apr 13, 2018

Stop recording assignments for feature gates Betterment/test_track_rails_client#43

Closed

Stop recording assignments for feature gates Betterment/test_track_js_client#11

Open

Don't sync assignment events for gates Betterment/test_track_rails_client#44

Merged

klesse413 reviewed Apr 13, 2018

View reviewed changes

👮

890cdb4

klesse413 reviewed Apr 13, 2018

View reviewed changes

don't use the word population for gates

a4fa1f9

because we use it in the admin to mean "recorded assignees of experiments."

klesse413 reviewed Apr 13, 2018

View reviewed changes

jmileham merged commit 60cb87f into Betterment:master Apr 23, 2018

jmileham deleted the virtualize_gate_assignments branch April 23, 2018 18:55

Uh oh!

Conversation

jmileham commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Other Information

Uh oh!

nanda-prbot commented Apr 12, 2018

Uh oh!

jmileham commented Apr 12, 2018

Uh oh!

nanda-prbot commented Apr 12, 2018

Uh oh!

jmileham Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

betterzega Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmileham commented Apr 12, 2018

Uh oh!

nanda-prbot commented Apr 12, 2018

Uh oh!

asullivan210 commented Apr 12, 2018

Uh oh!

nanda-prbot commented Apr 12, 2018

Uh oh!

asullivan210 commented Apr 12, 2018

Uh oh!

nanda-prbot commented Apr 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmileham commented Apr 13, 2018

Uh oh!

samandmoore commented Apr 13, 2018

Uh oh!

jmileham commented Apr 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmileham Apr 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmileham commented Apr 13, 2018

Uh oh!

nanda-prbot commented Apr 13, 2018

jmileham commented Apr 12, 2018 •

edited

Loading

jmileham Apr 12, 2018 •

edited

Loading

betterzega Apr 12, 2018 •

edited

Loading

jmileham Apr 13, 2018 •

edited

Loading