Experience sampling weight (#80)

jmileham · web-flow · commit 5098de6eb048 · 2018-06-11T12:49:27.000-04:00
* upgrade robocop

* Add SplitRegistry model with sampling weight support

increase brittleness if env var retrieval impl changes

* V2 split registries controller with sampling weight

Document EXPERIENCE_SAMPLING_WEIGHT

* Expand split representation

* correct var name
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -256,8 +256,7 @@ GEM
       method_source
       rake (>= 0.8.7)
       thor (>= 0.18.1, < 2.0)
-    rainbow (2.2.2)
-      rake
+    rainbow (3.0.0)
     rake (12.3.1)
     rb-fsevent (0.10.2)
     rb-inotify (0.9.10)
@@ -294,15 +293,15 @@ GEM
     rspec-support (3.7.0)
     rspec_junit_formatter (0.3.0)
       rspec-core (>= 2, < 4, != 2.12.0)
-    rubocop (0.51.0)
+    rubocop (0.56.0)
       parallel (~> 1.10)
-      parser (>= 2.3.3.1, < 3.0)
+      parser (>= 2.5)
       powerpack (~> 0.1)
-      rainbow (>= 2.2.2, < 3.0)
+      rainbow (>= 2.2.2, < 4.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (~> 1.0, >= 1.0.1)
-    rubocop-betterment (1.0.0)
-      rubocop (= 0.51)
+    rubocop-betterment (1.3.0)
+      rubocop (= 0.56)
     ruby-progressbar (1.9.0)
     ruby-saml (1.5.0)
       nokogiri (>= 1.5.10)
@@ -375,7 +374,7 @@ GEM
       thread_safe (~> 0.1)
     uglifier (3.2.0)
       execjs (>= 0.3.0, < 3)
-    unicode-display_width (1.3.2)
+    unicode-display_width (1.4.0)
     warden (1.2.7)
       rack (>= 1.0)
     web-console (2.3.0)
diff --git a/README.md b/README.md
@@ -132,6 +132,20 @@ Variants can be associated with metadata to describe their effects in human-read
 
 For more on how paths are constructed, see the [paperclip documentation](https://github.com/thoughtbot/paperclip).
 
+## Advanced topics
+
+### Feature Gate Experience Sampling Weight
+
+Feature gates report to your analytics provider differently relative to experiments. Where an experiment is assigned once per visitor and the assignment is recorded on the server for later reuse, a feature gate assignment is recalculated on the client side at each interaction. As a result, the analytics provider receives a different kind of event - `feature_gate_experienced` instead of `split_assigned`. These experience events are much higher velocity than assignment events because they occur each time a customer or backend codepath encounters the feature gate.
+
+While this experience event information is valuable to developers who are evaluating the number of customers who have experienced a feature, for example during a slow feature rollout to establish confidence that error rate remains low, there is little value in having comprehensive records. Sampling a fraction of customer interactions provides the same signal developers need at much lower cost.
+
+To switch to a sampling strategy, set the `EXPERIENCE_SAMPLING_WEIGHT` env var to a non-negative integer value as follows:
+
+* The default is `1`, which means that every experience event will be reported to analytics
+* To disable reporting of feature gate assignments altogether, set it to `0`
+* A value of `10` tells clients to report experience events to analytics probablistically one out of ten times. Conformant (>= v4) TestTrack clients will then reduce their reporting rate accordingly across all feature gates.
+
 ## Concepts
 
 ### App
diff --git a/app/controllers/api/v2/split_registries_controller.rb b/app/controllers/api/v2/split_registries_controller.rb
@@ -0,0 +1,7 @@
+class Api::V2::SplitRegistriesController < UnauthenticatedApiController
+  include CorsSupport
+
+  def show
+    @split_registry = SplitRegistry.instance
+  end
+end
diff --git a/app/inputs/collection_select_input.rb b/app/inputs/collection_select_input.rb
@@ -75,7 +75,7 @@ def value_method
   end
 
   def collection_methods
-    @_collection_methods ||= detect_collection_methods
+    @_collection_methods ||= detect_collection_methods # rubocop:disable Naming/MemoizedInstanceVariableName
   end
 
   def extract_label(option)
diff --git a/app/inputs/grouped_collection_select_input.rb b/app/inputs/grouped_collection_select_input.rb
@@ -13,7 +13,9 @@ def selected(_wrapper_options)
 
   def selected_option
     current_value = object.public_send(attribute_name)
+    # rubocop:disable Naming/MemoizedInstanceVariableName
     @_selected ||= collections.find { |option| option_value(option) == current_value || option == current_value }
+    # rubocop:enable Naming/MemoizedInstanceVariableName
   end
 
   def collections
@@ -53,15 +55,15 @@ def group_dropdown(parent)
   end
 
   def label_method
-    @_label_method ||= collection_methods.first
+    @_label_method ||= collection_methods.first # rubocop:disable Naming/MemoizedInstanceVariableName
   end
 
   def value_method
-    @_value_method ||= collection_methods.last
+    @_value_method ||= collection_methods.last # rubocop:disable Naming/MemoizedInstanceVariableName
   end
 
   def collection_methods
-    @_collection_methods ||= detect_collection_methods
+    @_collection_methods ||= detect_collection_methods # rubocop:disable Naming/MemoizedInstanceVariableName
   end
 
   def option_label(option)
diff --git a/app/models/app.rb b/app/models/app.rb
@@ -10,7 +10,7 @@ class App < ActiveRecord::Base
   private
 
   def auth_secret_must_be_sufficiently_strong
-    return if auth_secret && auth_secret.size >= 43 # rubocop:disable Style/SafeNavigation
+    return if auth_secret && auth_secret.size >= 43
     errors.add(:auth_secret, "must be at least 32-bytes, Base64 encoded")
   end
 end
diff --git a/app/models/split.rb b/app/models/split.rb
@@ -94,7 +94,7 @@ def variants_must_be_snake_case
   end
 
   def registry_weights_must_sum_to_100
-    sum = registry && registry.values.sum # rubocop:disable Style/SafeNavigation
+    sum = registry && registry.values.sum
     errors.add(:registry, "must contain weights that sum to 100% (got #{sum})") unless sum == 100
   end
 
diff --git a/app/models/split_registry.rb b/app/models/split_registry.rb
@@ -0,0 +1,21 @@
+class SplitRegistry
+  include Singleton
+
+  def splits
+    Split.active
+  end
+
+  def experience_sampling_weight
+    @experience_sampling_weight ||= _experience_sampling_weight
+  end
+
+  private
+
+  def _experience_sampling_weight
+    Integer(ENV.fetch('EXPERIENCE_SAMPLING_WEIGHT', '1')).tap do |weight|
+      raise <<~TEXT if weight.negative?
+        EXPERIENCE_SAMPLING_WEIGHT, if specified, must be greater than or equal to 0. Use 0 to disable experience events.
+      TEXT
+    end
+  end
+end
diff --git a/app/views/api/v2/split_registries/show.json.jbuilder b/app/views/api/v2/split_registries/show.json.jbuilder
@@ -0,0 +1,10 @@
+json.splits do
+  @split_registry.splits.each do |split|
+    json.set! split.name do
+      json.weights split.registry
+      json.feature_gate split.feature_gate?
+    end
+  end
+  json.merge!({})
+end
+json.(@split_registry, :experience_sampling_weight)
diff --git a/config/routes.rb b/config/routes.rb
@@ -33,6 +33,10 @@
       resources :split_configs, only: [:create, :destroy]
       resource :identifier_type, only: :create
     end
+
+    namespace :v2 do
+      resource :split_registry, only: :show
+    end
   end
 
   if ENV['SAML_ISSUER'].present?
diff --git a/spec/controllers/api/v2/split_registries_controller_spec.rb b/spec/controllers/api/v2/split_registries_controller_spec.rb
@@ -0,0 +1,45 @@
+require 'rails_helper'
+
+RSpec.describe Api::V2::SplitRegistriesController, type: :controller do
+  let(:split_1) { FactoryBot.create :split, name: "one", finished_at: Time.zone.now, registry: { all: 100 } }
+  let(:split_2) { FactoryBot.create :split, name: "two", registry: { on: 50, off: 50 } }
+  let(:split_3) { FactoryBot.create :split, name: "three_enabled", registry: { true: 99, false: 1 }, feature_gate: true }
+
+  describe "#show" do
+    before do
+      allow(SplitRegistry.instance).to receive(:experience_sampling_weight).and_return(10)
+    end
+
+    it "includes sampling weight" do
+      get :show
+      expect(response).to have_http_status :ok
+      expect(response_json['experience_sampling_weight']).to eq(10)
+    end
+
+    it "returns empty with no active splits" do
+      get :show
+      expect(response).to have_http_status :ok
+      expect(response_json['splits']).to eq({})
+    end
+
+    it "returns the full split registry" do
+      expect(split_1).to be_finished
+      expect(split_2).not_to be_finished
+      expect(split_3).not_to be_finished
+
+      get :show
+
+      expect(response).to have_http_status :ok
+      expect(response_json['splits']).to eq(
+        "two" => {
+          "weights" => { "on" => 50, "off" => 50 },
+          "feature_gate" => false
+        },
+        "three_enabled" => {
+          "weights" => { "true" => 99, "false" => 1 },
+          "feature_gate" => true
+        }
+      )
+    end
+  end
+end
diff --git a/spec/models/assignment_spec.rb b/spec/models/assignment_spec.rb
@@ -71,12 +71,12 @@
 
   describe ".to_hash" do
     it "is a hash of the split names to the variants" do
-      split1 = FactoryBot.create(:split, name: "split1")
-      split2 = FactoryBot.create(:split, name: "split2")
-      FactoryBot.create(:assignment, split: split1, variant: :hammer_time)
-      FactoryBot.create(:assignment, split: split2, variant: :touch_this)
+      split_1 = FactoryBot.create(:split, name: "split_1")
+      split_2 = FactoryBot.create(:split, name: "split_2")
+      FactoryBot.create(:assignment, split: split_1, variant: :hammer_time)
+      FactoryBot.create(:assignment, split: split_2, variant: :touch_this)
 
-      expect(described_class.to_hash).to eq(split1: :hammer_time, split2: :touch_this)
+      expect(described_class.to_hash).to eq(split_1: :hammer_time, split_2: :touch_this)
     end
   end
 
diff --git a/spec/models/split_registry_spec.rb b/spec/models/split_registry_spec.rb
@@ -0,0 +1,63 @@
+require 'rails_helper'
+
+RSpec.describe SplitRegistry do
+  subject { SplitRegistry.instance }
+
+  describe "#splits" do
+    it "doesn't cache the instance" do
+      expect(subject.splits).to eq(subject.splits)
+      expect(subject.splits).not_to eql(subject.splits)
+    end
+
+    it "returns active splits" do
+      split = FactoryBot.create(:split)
+
+      expect(subject.splits.all).to include(split)
+    end
+
+    it "doesn't return inactive splits" do
+      split = FactoryBot.create(:split, finished_at: Time.zone.now)
+
+      expect(subject.splits.all).not_to include(split)
+    end
+  end
+
+  describe "#experience_sampling_weight" do
+    context "bypassing singleton memoization" do
+      subject { SplitRegistry.send(:new) }
+
+      it "memoizes the env var fetch" do
+        allow(ENV).to receive(:fetch).and_call_original
+
+        subject.experience_sampling_weight
+        subject.experience_sampling_weight
+
+        expect(ENV).to have_received(:fetch).with('EXPERIENCE_SAMPLING_WEIGHT', any_args).exactly(:once)
+      end
+
+      it "returns 1 with no env var" do
+        with_env EXPERIENCE_SAMPLING_WEIGHT: nil do
+          expect(subject.experience_sampling_weight).to eq 1
+        end
+      end
+
+      it "returns an positive value specified" do
+        with_env EXPERIENCE_SAMPLING_WEIGHT: '10' do
+          expect(subject.experience_sampling_weight).to eq 10
+        end
+      end
+
+      it "returns zero when specified" do
+        with_env EXPERIENCE_SAMPLING_WEIGHT: '0' do
+          expect(subject.experience_sampling_weight).to eq 0
+        end
+      end
+
+      it "blows up on negative" do
+        with_env EXPERIENCE_SAMPLING_WEIGHT: '-1' do
+          expect { subject.experience_sampling_weight }.to raise_error(/greater than or equal/)
+        end
+      end
+    end
+  end
+end
