Sitelet https://github.com/splitrb/split/commit/6130cd5e42c715c0bc0fa71e596b9b7e6a205ec6
Skip to content

Commit 6130cd5

Browse files
authored
Merge pull request #755 from splitrb/metric-lookup-short-circuit
Metric lookup short circuit to avoid an extra redis call
2 parents 8f67693 + 153aa96 commit 6130cd5

3 files changed

Lines changed: 34 additions & 4 deletions

File tree

‎README.md‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -636,11 +636,15 @@ the experiment name:
636636
ab_finished(:my_metric)
637637
```
638638

639-
You can also create a new metric by instantiating and saving a new Metric object.
639+
You can also create a new metric by instantiating and saving a new Metric
640+
object. A metric groups one or more experiments (`Split::Experiment` objects)
641+
under a name and is persisted to Redis:
640642

641643
```ruby
642-
Split::Metric.new(:my_metric)
643-
Split::Metric.save
644+
signup = Split::ExperimentCatalog.find_or_create("signup_form", "control", "blue")
645+
checkout = Split::ExperimentCatalog.find_or_create("checkout_flow", "control", "one_page")
646+
647+
Split::Metric.new(name: :conversion, experiments: [signup, checkout]).save
644648
```
645649

646650
#### Goals

‎lib/split/user.rb‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def cleanup_old_versions!(experiment)
5656

5757
def active_experiments
5858
experiments_by_key = keys_without_finished(user.keys).each_with_object({}) do |key, memo|
59-
memo[key] = Metric.possible_experiments(key_without_version(key))
59+
memo[key] = experiments_for(key_without_version(key))
6060
end
6161

6262
names = experiments_by_key.values.flatten.map(&:name).uniq
@@ -95,5 +95,19 @@ def keys_without_finished(keys)
9595
def key_without_version(key)
9696
key.split(/\:\d(?!\:)/)[0]
9797
end
98+
99+
def experiments_for(name)
100+
if metrics_defined?
101+
Metric.possible_experiments(name)
102+
else
103+
Array(Experiment.find(name))
104+
end
105+
end
106+
107+
def metrics_defined?
108+
return @metrics_defined if defined?(@metrics_defined)
109+
@metrics_defined =
110+
Split.configuration.metrics.any? || Split.redis.exists?(:metrics)
111+
end
98112
end
99113
end

‎spec/user_spec.rb‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,18 @@
137137
expect(Split.redis).to receive(:hmget).with(:experiment_winner, any_args).once.and_call_original
138138
@subject.active_experiments
139139
end
140+
141+
it "resolves experiments directly, without per-key metric lookups, when no metrics are defined" do
142+
expect(Split::Metric).not_to receive(:possible_experiments)
143+
expect(@subject.active_experiments).to eq("active" => "red")
144+
end
145+
146+
it "still honors a metric persisted only in Redis" do
147+
Split::Metric.new(name: :conversion, experiments: [Split::ExperimentCatalog.find("active")]).save
148+
149+
expect(Split::Metric).to receive(:possible_experiments).at_least(:once).and_call_original
150+
expect(@subject.active_experiments).to eq("active" => "red")
151+
end
140152
end
141153

142154
context "allows user to be loaded from adapter" do

0 commit comments

Comments
 (0)