Skip to content

Commit e18d900

Browse files
authored
Merge pull request #564 from splitio/FME-4369-imp-prop-engine
Updated engine and sync classes
2 parents 116da66 + f8a9d93 commit e18d900

File tree

6 files changed

+73
-43
lines changed

6 files changed

+73
-43
lines changed

lib/splitclient-rb/cache/senders/impressions_formatter.rb

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,30 @@ def feature_impressions(filtered_impressions, feature)
3636
end
3737

3838
def current_impressions(feature_impressions)
39+
formatted = []
3940
feature_impressions.map do |impression|
40-
{
41-
k: impression[:i][:k],
42-
t: impression[:i][:t],
43-
m: impression[:i][:m],
44-
b: impression[:i][:b],
45-
r: impression[:i][:r],
46-
c: impression[:i][:c],
47-
pt: impression[:i][:pt]
48-
}
41+
if impression[:i][:properties].nil?
42+
impression = {
43+
k: impression[:i][:k],
44+
t: impression[:i][:t],
45+
m: impression[:i][:m],
46+
b: impression[:i][:b],
47+
r: impression[:i][:r],
48+
c: impression[:i][:c],
49+
pt: impression[:i][:pt]
50+
}
51+
else
52+
impression = {
53+
k: impression[:i][:k],
54+
t: impression[:i][:t],
55+
m: impression[:i][:m],
56+
b: impression[:i][:b],
57+
r: impression[:i][:r],
58+
c: impression[:i][:c],
59+
pt: impression[:i][:pt],
60+
properties: impression[:i][:properties].to_json.to_s
61+
}
62+
end
4963
end
5064
end
5165

@@ -73,7 +87,8 @@ def impression_hash(impression)
7387
"#{impression[:i][:b]}:" \
7488
"#{impression[:i][:c]}:" \
7589
"#{impression[:i][:t]}:" \
76-
"#{impression[:i][:pt]}"
90+
"#{impression[:i][:pt]}" \
91+
"#{impression[:i][:properties]}" \
7792
end
7893
end
7994
end

lib/splitclient-rb/engine/common/impressions_manager.rb

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,20 @@ def initialize(config,
1818
@unique_keys_tracker = unique_keys_tracker
1919
end
2020

21-
def build_impression(matching_key, bucketing_key, split_name, treatment_data, impressions_disabled, params = {})
22-
impression_data = impression_data(matching_key, bucketing_key, split_name, treatment_data, params[:time])
21+
def build_impression(matching_key, bucketing_key, split_name, treatment_data, impressions_disabled, params = {},
22+
properties = nil)
23+
impression_data = impression_data(matching_key, bucketing_key, split_name, treatment_data, params[:time], properties)
2324
begin
2425
if @config.impressions_mode == :none || impressions_disabled
2526
@impression_counter.inc(split_name, impression_data[:m])
2627
@unique_keys_tracker.track(split_name, matching_key)
2728
elsif @config.impressions_mode == :debug # In DEBUG mode we should calculate the pt only.
29+
return impression(impression_data, params[:attributes]) unless properties.nil?
30+
2831
impression_data[:pt] = @impression_observer.test_and_set(impression_data)
2932
else # In OPTIMIZED mode we should track the total amount of evaluations and deduplicate the impressions.
33+
return impression(impression_data, params[:attributes]) unless properties.nil?
34+
3035
impression_data[:pt] = @impression_observer.test_and_set(impression_data)
3136
@impression_counter.inc(split_name, impression_data[:m]) unless impression_data[:pt].nil?
3237
end
@@ -79,7 +84,8 @@ def record_stats(stats)
7984
end
8085

8186
# added param time for test
82-
def impression_data(matching_key, bucketing_key, split_name, treatment, time = nil)
87+
def impression_data(matching_key, bucketing_key, split_name, treatment, time = nil,
88+
properties = nil)
8389
{
8490
k: matching_key,
8591
b: bucketing_key,
@@ -88,7 +94,8 @@ def impression_data(matching_key, bucketing_key, split_name, treatment, time = n
8894
r: applied_rule(treatment[:label]),
8995
c: treatment[:change_number],
9096
m: time || (Time.now.to_f * 1000.0).to_i,
91-
pt: nil
97+
pt: nil,
98+
properties: properties
9299
}
93100
end
94101

spec/cache/repositories/impressions_repository_spec.rb

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
r: 'sample_rule',
2626
c: 1_533_177_602_748,
2727
m: 1_478_113_516_002,
28-
pt: nil
28+
pt: nil,
29+
properties: nil
2930
},
3031
attributes: {}
3132
},
@@ -39,7 +40,8 @@
3940
r: 'sample_rule',
4041
c: 1_533_177_602_749,
4142
m: 1_478_113_516_002,
42-
pt: nil
43+
pt: nil,
44+
properties: nil
4345
},
4446
attributes: {}
4547
}
@@ -55,8 +57,8 @@
5557
it 'adds impressions' do
5658
params = { attributes: {}, time: 1_478_113_516_002 }
5759
impressions = []
58-
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params), :disabled => false }
59-
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :bar, treatment2, false, params), :disabled => false }
60+
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params, nil), :disabled => false }
61+
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :bar, treatment2, false, params, nil), :disabled => false }
6062
impressions_manager.track(impressions)
6163

6264
expect(repository.batch).to match_array(result)
@@ -67,8 +69,8 @@
6769
it 'adds impressions in bulk' do
6870
params = { attributes: {}, time: 1_478_113_516_002 }
6971
impressions = []
70-
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params), :disabled => false }
71-
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :bar, treatment2, false, params), :disabled => false }
72+
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params, nil), :disabled => false }
73+
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :bar, treatment2, false, params, nil), :disabled => false }
7274
impressions_manager.track(impressions)
7375

7476
expect(repository.batch).to match_array(result)
@@ -80,7 +82,7 @@
8082
config.labels_enabled = false
8183
params = { attributes: {}, time: 1_478_113_516_002 }
8284
impressions = []
83-
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params), :disabled => false }
85+
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params, nil), :disabled => false }
8486
impressions_manager.track(impressions)
8587

8688
expect(repository.batch.first[:i][:r]).to be_nil
@@ -89,8 +91,8 @@
8991
it 'bulk size less than the actual queue' do
9092
params = { attributes: {}, time: 1_478_113_516_002 }
9193
impressions = []
92-
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params), :disabled => false }
93-
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment2, false, params), :disabled => false }
94+
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params, nil), :disabled => false }
95+
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment2, false, params, nil), :disabled => false }
9496
impressions_manager.track(impressions)
9597

9698
config.impressions_bulk_size = 1
@@ -142,8 +144,8 @@
142144
treatment = { treatment: 'on', label: 'sample_rule', change_number: 1_533_177_602_748 }
143145
params = { attributes: {}, time: 1_478_113_516_002 }
144146
impressions = []
145-
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo1, treatment, false, params), :disabled => false }
146-
impressions << { :impression => impressions_manager.build_impression('matching_key2', nil, :foo1, treatment, false, params), :disabled => false }
147+
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo1, treatment, false, params, nil), :disabled => false }
148+
impressions << { :impression => impressions_manager.build_impression('matching_key2', nil, :foo1, treatment, false, params, nil), :disabled => false }
147149
impressions_manager.track(impressions)
148150

149151
expect(repository.batch.size).to eq(1)
@@ -200,8 +202,8 @@
200202
expect(config.impressions_adapter).to receive(:expire).once.with(anything, 3600)
201203
params = { attributes: {}, time: 1_478_113_516_002 }
202204
impressions = []
203-
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params), :disabled => false }
204-
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params), :disabled => false }
205+
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params, nil), :disabled => false }
206+
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params, nil), :disabled => false }
205207
impressions_manager.track(impressions)
206208
end
207209

@@ -211,7 +213,7 @@
211213

212214
params = { attributes: {}, time: 1_478_113_516_002 }
213215
impressions = []
214-
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params), :disabled => false }
216+
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params, nil), :disabled => false }
215217
impressions_manager.track(impressions)
216218

217219
expect(repository.batch).to eq([])
@@ -221,8 +223,8 @@
221223
other_treatment = { treatment: 'on', label: 'sample_rule_2', change_number: 1_533_177_602_748 }
222224
params = { attributes: {}, time: 1_478_113_516_002 }
223225
impressions = []
224-
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params), :disabled => false }
225-
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo2, other_treatment, false, params), :disabled => false }
226+
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params, nil), :disabled => false }
227+
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo2, other_treatment, false, params, nil), :disabled => false }
226228
impressions_manager.track(impressions)
227229

228230
adapter.get_from_queue('SPLITIO.impressions', 0).map do |e|
@@ -252,8 +254,8 @@
252254

253255
params = { attributes: {}, time: 1_478_113_516_002 }
254256
impressions = []
255-
impressions << { :impression => custom_impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params), :disabled => false }
256-
impressions << { :impression => custom_impressions_manager.build_impression('matching_key', nil, :foo2, other_treatment, false, params), :disabled => false }
257+
impressions << { :impression => custom_impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params, nil), :disabled => false }
258+
impressions << { :impression => custom_impressions_manager.build_impression('matching_key', nil, :foo2, other_treatment, false, params, nil), :disabled => false }
257259
custom_impressions_manager.track(impressions)
258260

259261
custom_adapter.get_from_queue('SPLITIO.impressions', 0).map do |e|

spec/cache/senders/impressions_formatter_spec.rb

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
b: 'foo1',
5959
r: 'custom_label1',
6060
c: 123_456,
61-
pt: nil }]
61+
pt: nil}]
6262
},
6363
{
6464
f: :foo2,
@@ -68,14 +68,14 @@
6868
b: 'foo2',
6969
r: 'custom_label2',
7070
c: 123_499,
71-
pt: nil }]
71+
pt: nil}]
7272
}])
7373
end
7474

7575
it 'formats multiple impressions for one key' do
7676
params = { attributes: {}, time: 1_478_113_518_900 }
7777
impressions = []
78-
impressions << { :impression => impressions_manager.build_impression('matching_key3', nil, 'foo2', treatment3, false, params), :disabled => false }
78+
impressions << { :impression => impressions_manager.build_impression('matching_key3', nil, 'foo2', treatment3, false, params, {"prop": "val"}), :disabled => false }
7979
impressions_manager.track(impressions)
8080

8181
expect(formatted_impressions.find { |i| i[:f] == :foo1 }[:i]).to match_array(
@@ -110,7 +110,8 @@
110110
b: nil,
111111
r: nil,
112112
c: nil,
113-
pt: nil
113+
pt: nil,
114+
properties: '{"prop":"val"}'
114115
}
115116
]
116117
)

spec/cache/senders/impressions_sender_spec.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
params2 = { attributes: {}, time: 1_478_113_518_285 }
4747
impressions = []
4848
impressions << { :impression => impressions_manager.build_impression('matching_key', 'foo1', 'foo1', treatment1, false, params), :disabled => false }
49-
impressions << { :impression => impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, false, params2), :disabled => false }
49+
impressions << { :impression => impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, false, params2, {:prop=>"val"}), :disabled => false }
5050
impressions_manager.track(impressions)
5151
end
5252

@@ -89,7 +89,8 @@
8989
b: 'foo2',
9090
r: 'custom_label2',
9191
c: 123_499,
92-
pt: nil
92+
pt: nil,
93+
properties: '{"prop":"val"}'
9394
}
9495
]
9596
}

spec/engine/common/impression_manager_spec.rb

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@
5959
r: 'default label',
6060
c: 1_478_113_516_002,
6161
m: 1_478_113_516_222,
62-
pt: nil
62+
pt: nil,
63+
properties: {"prop":"val"}
6364
},
6465
attributes: {}
6566
}
@@ -71,7 +72,7 @@
7172
'split_name_test',
7273
treatment,
7374
false,
74-
params)
75+
params, {"prop":"val"})
7576
expect(impression).to match(expected)
7677

7778
result_count = impression_counter.pop_all
@@ -97,7 +98,8 @@
9798
r: 'default label',
9899
c: 1_478_113_516_002,
99100
m: 1_478_113_516_222,
100-
pt: nil
101+
pt: nil,
102+
properties: nil
101103
},
102104
attributes: {}
103105
}
@@ -155,7 +157,8 @@
155157
r: 'default label',
156158
c: 1_478_113_516_002,
157159
m: 1_478_113_516_222,
158-
pt: nil
160+
pt: nil,
161+
properties: nil
159162
},
160163
attributes: {}
161164
}
@@ -297,7 +300,8 @@
297300
r: 'default label',
298301
c: 1_478_113_516_002,
299302
m: 1_478_113_516_222,
300-
pt: nil
303+
pt: nil,
304+
properties: nil
301305
},
302306
attributes: {}
303307
}

0 commit comments

Comments
 (0)