diff --git a/.rubocop.yml b/.rubocop.yml index 85643225..91428927 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -9,7 +9,7 @@ Naming/PredicateName: Metrics/ClassLength: - Max: 130 + Max: 135 Layout/LineLength: Max: 140 Metrics/MethodLength: diff --git a/lib/unleash/feature_toggle.rb b/lib/unleash/feature_toggle.rb index 1e424664..2818f02b 100644 --- a/lib/unleash/feature_toggle.rb +++ b/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 diff --git a/lib/unleash/metrics.rb b/lib/unleash/metrics.rb index ceb5e73b..4dc8bc57 100644 --- a/lib/unleash/metrics.rb +++ b/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 diff --git a/spec/unleash/client_spec.rb b/spec/unleash/client_spec.rb index f92d591b..ecac0bd4 100644 --- a/spec/unleash/client_spec.rb +++ b/spec/unleash/client_spec.rb @@ -91,6 +91,7 @@ # 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") @@ -98,6 +99,7 @@ .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 diff --git a/spec/unleash/metrics_spec.rb b/spec/unleash/metrics_spec.rb index ef09b82f..be3956ea 100644 --- a/spec/unleash/metrics_spec.rb +++ b/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