Fix/variants dont add to toggle metrics (#126)

Patch an issue where variant metrics aren't considered when the parent toggle is disabled or make variants increment toggle metrics

.rubocop.yml

@@ -9,7 +9,7 @@ Naming/PredicateName:
 
 
 Metrics/ClassLength:
-  Max: 130
+  Max: 135
 Layout/LineLength:
   Max: 140
 Metrics/MethodLength:

lib/unleash/feature_toggle.rb

@@ -37,12 +37,11 @@ def get_variant(context, fallback_variant = Unleash::FeatureToggle.disabled_vari
 
       context = ensure_valid_context(context)
 
-      return Unleash::FeatureToggle.disabled_variant unless self.enabled && am_enabled?(context)
-      return Unleash::FeatureToggle.disabled_variant if sum_variant_defs_weights <= 0
-
-      variant = variant_from_override_match(context) || variant_from_weights(context, resolve_stickiness)
+      toggle_enabled = am_enabled?(context)
+      variant = resolve_variant(context, toggle_enabled)
 
-      Unleash.toggle_metrics.increment_variant(self.name, variant.name) unless Unleash.configuration.disable_metrics
+      choice = toggle_enabled ? :yes : :no
+      Unleash.toggle_metrics.increment_variant(self.name, choice, variant.name) unless Unleash.configuration.disable_metrics
       variant
     end
 
@@ -52,6 +51,13 @@ def self.disabled_variant
 
     private
 
+    def resolve_variant(context, toggle_enabled)
+      return Unleash::FeatureToggle.disabled_variant unless toggle_enabled
+      return Unleash::FeatureToggle.disabled_variant if sum_variant_defs_weights <= 0
+
+      variant_from_override_match(context) || variant_from_weights(context, resolve_stickiness)
+    end
+
     def resolve_stickiness
       self.variant_definitions&.map(&:stickiness)&.compact&.first || "default"
     end

lib/unleash/metrics.rb

@@ -22,9 +22,10 @@ def increment(feature, choice)
       end
     end
 
-    def increment_variant(feature, variant)
+    def increment_variant(feature, choice, variant)
       self.features_lock.synchronize do
         self.features[feature] = { yes: 0, no: 0 } unless self.features.include? feature
+        self.features[feature][choice] += 1
         self.features[feature]['variant'] = {}     unless self.features[feature].include? 'variant'
         self.features[feature]['variant'][variant] = 0 unless self.features[feature]['variant'].include? variant
         self.features[feature]['variant'][variant] += 1

spec/unleash/client_spec.rb

@@ -91,13 +91,15 @@
 
     # Sending metrics, if they have been evaluated:
     unleash_client.is_enabled?("Feature.A")
+    unleash_client.get_variant("Feature.A")
     Unleash.reporter.post
     expect(
       a_request(:post, "http://test-url/client/metrics")
       .with(headers: { 'Content-Type': 'application/json' })
       .with(headers: { 'X-API-KEY': '123', 'Content-Type': 'application/json' })
       .with(headers: { 'UNLEASH-APPNAME': 'my-test-app' })
       .with(headers: { 'UNLEASH-INSTANCEID': 'rspec/test' })
+      .with{ |request| JSON.parse(request.body)['bucket']['toggles']['Feature.A']['yes'] == 2 }
     ).to have_been_made.once
   end
 

spec/unleash/metrics_spec.rb

@@ -52,16 +52,21 @@
 
   describe "when dealing with variants" do
     it "counts up correctly" do
-      metrics.increment('featureA', :yes)
+      metrics.increment_variant('featureA', :yes, 'variantA')
+      metrics.increment_variant('featureA', :yes, 'variantA')
+      metrics.increment_variant('featureA', :yes, 'variantB')
 
-      metrics.increment_variant('featureA', 'variantA')
-      metrics.increment_variant('featureA', 'variantA')
-      metrics.increment_variant('featureA', 'variantB')
-
-      expect(metrics.features['featureA'][:yes]).to eq(1)
+      expect(metrics.features['featureA'][:yes]).to eq(3)
       expect(metrics.features['featureA'][:no]).to eq(0)
       expect(metrics.features['featureA']['variant']['variantA']).to eq(2)
       expect(metrics.features['featureA']['variant']['variantB']).to eq(1)
     end
   end
+
+  it "increments feature toggle counter when variant is resolved" do
+    metrics.increment_variant('featureA', :yes, 'variantA')
+
+    expect(metrics.features['featureA'][:yes]).to eq(1)
+    expect(metrics.features['featureA'][:no]).to eq(0)
+  end
 end