Skip to content

Imp toggle tests #541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 10, 2025
Merged

Imp toggle tests #541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc499a6
updated tests and fixes
Jan 10, 2025
2530b2a
polish
Jan 10, 2025
16643ea
polish
Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions lib/splitclient-rb/clients/split_client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ def treatments(key, feature_flag_names, attributes = {}, calling_method = 'get_t
to_return = Hash.new
sanitized_feature_flag_names.each {|name|
to_return[name.to_sym] = control_treatment_with_config
impressions << @impressions_manager.build_impression(matching_key, bucketing_key, name.to_sym, control_treatment_with_config.merge({ label: Engine::Models::Label::NOT_READY }), { attributes: attributes, time: nil })
impressions << { :impression => @impressions_manager.build_impression(matching_key, bucketing_key, name.to_sym, control_treatment_with_config.merge({ :label => Engine::Models::Label::NOT_READY }), false, { attributes: attributes, time: nil }), :disabled => false }
}
@impressions_manager.track(impressions)
return to_return
Expand All @@ -278,7 +278,6 @@ def treatments(key, feature_flag_names, attributes = {}, calling_method = 'get_t
valid_feature_flag_names << feature_flag_name unless feature_flag_name.nil?
}
start = Time.now
impressions_total = []

feature_flags = @splits_repository.splits(valid_feature_flag_names)
treatments = Hash.new
Expand All @@ -291,15 +290,14 @@ def treatments(key, feature_flag_names, attributes = {}, calling_method = 'get_t
next
end
treatments_labels_change_numbers, impressions = evaluate_treatment(feature_flag, key, bucketing_key, matching_key, attributes, calling_method)
impressions_total.concat(impressions) unless impressions.nil?
treatments[key] =
{
treatment: treatments_labels_change_numbers[:treatment],
config: treatments_labels_change_numbers[:config]
}
@impressions_manager.track(impressions) unless impressions.empty?
end
record_latency(calling_method, start)
@impressions_manager.track(impressions_total) unless impressions_total.empty?

treatments.merge(invalid_treatments)
end
Expand Down Expand Up @@ -345,37 +343,37 @@ def evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_
if feature_flag.nil? && ready?
@config.logger.warn("#{calling_method}: you passed #{feature_flag_name} that " \
'does not exist in this environment, please double check what feature flags exist in the Split user interface')
return parsed_treatment(control_treatment.merge({ label: Engine::Models::Label::NOT_FOUND }), multiple), nil
return parsed_treatment(control_treatment.merge({ :label => Engine::Models::Label::NOT_FOUND }), multiple), nil
end
treatment_data =

if !feature_flag.nil? && ready?
@evaluator.evaluate_feature_flag(
treatment_data = @evaluator.evaluate_feature_flag(
{ bucketing_key: bucketing_key, matching_key: matching_key }, feature_flag, attributes
)
impressions_disabled = feature_flag[:impressionsDisabled]
else
@config.logger.error("#{calling_method}: the SDK is not ready, results may be incorrect for feature flag #{feature_flag_name}. Make sure to wait for SDK readiness before using this method.")
control_treatment.merge({ label: Engine::Models::Label::NOT_READY })
treatment_data = control_treatment.merge({ :label => Engine::Models::Label::NOT_READY })
impressions_disabled = false
end

record_latency(calling_method, start)
impression_decorator = { impression: @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, treatment_data, feature_flag[:impressionsDisabled], { attributes: attributes, time: nil }), disabled: feature_flag[:impressionsDisabled] }
impression_decorator = { :impression => @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, treatment_data, impressions_disabled, params={ attributes: attributes, time: nil }), :disabled => impressions_disabled }
impressions_decorator << impression_decorator unless impression_decorator.nil?
rescue StandardError => e
@config.log_found_exception(__method__.to_s, e)

record_exception(calling_method)

impression_decorator = { impression: @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, control_treatment, { attributes: attributes, time: nil }), disabled: false }
impression_decorator = { :impression => @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, control_treatment, false, { attributes: attributes, time: nil }), :disabled => false }
impressions_decorator << impression_decorator unless impression_decorator.nil?

return parsed_treatment(control_treatment.merge({ label: Engine::Models::Label::EXCEPTION }), multiple), impressions_decorator
return parsed_treatment(control_treatment.merge({ :label => Engine::Models::Label::EXCEPTION }), multiple), impressions_decorator
Copy link
Contributor

@sanzmauro sanzmauro Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this changes?

Copy link
Contributor Author

@chillaq chillaq Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In impressions_manager it uses [:label] to fetch it

end

return parsed_treatment(treatment_data, multiple), impressions_decorator
end

def control_treatment
{ treatment: Engine::Models::Treatment::CONTROL }
{ :treatment => Engine::Models::Treatment::CONTROL }
end

def control_treatment_with_config
Expand Down
8 changes: 4 additions & 4 deletions lib/splitclient-rb/engine/common/impressions_manager.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ def initialize(config,
@unique_keys_tracker = unique_keys_tracker
end

def build_impression(matching_key, bucketing_key, split_name, treatment, impressions_disabled, params = {})
impression_data = impression_data(matching_key, bucketing_key, split_name, treatment, params[:time])
def build_impression(matching_key, bucketing_key, split_name, treatment_data, impressions_disabled, params = {})
impression_data = impression_data(matching_key, bucketing_key, split_name, treatment_data, params[:time])
begin
if @config.impressions_mode == :none || impressions_disabled # In NONE mode we should track the total amount of evaluations and the unique keys.
@impression_counter.inc(split_name, impression_data[:m])
Expand All @@ -41,10 +41,11 @@ def track(impressions_decorator)
return if impressions_decorator.empty?

impressions_decorator.each do |impression_decorator|
impression_router.add_bulk([impression_decorator[:impression]])
stats = { dropped: 0, queued: 0, dedupe: 0 }
begin
if @config.impressions_mode == :none || impression_decorator[:disabled]
return
next
elsif @config.impressions_mode == :debug
track_debug_mode([impression_decorator[:impression]], stats)
elsif @config.impressions_mode == :optimized
Expand All @@ -54,7 +55,6 @@ def track(impressions_decorator)
@config.log_found_exception(__method__.to_s, e)
ensure
record_stats(stats)
impression_router.add_bulk([impression_decorator[:impression]])
end
end
end
Expand Down
32 changes: 16 additions & 16 deletions spec/cache/repositories/impressions_repository_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@
it 'adds impressions' do
params = { attributes: {}, time: 1_478_113_516_002 }
impressions = []
impressions << impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, params)
impressions << impressions_manager.build_impression('matching_key1', nil, :bar, treatment2, params)
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params), :disabled => false }
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :bar, treatment2, false, params), :disabled => false }
impressions_manager.track(impressions)

expect(repository.batch).to match_array(result)
Expand All @@ -67,8 +67,8 @@
it 'adds impressions in bulk' do
params = { attributes: {}, time: 1_478_113_516_002 }
impressions = []
impressions << impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, params)
impressions << impressions_manager.build_impression('matching_key1', nil, :bar, treatment2, params)
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params), :disabled => false }
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :bar, treatment2, false, params), :disabled => false }
impressions_manager.track(impressions)

expect(repository.batch).to match_array(result)
Expand All @@ -80,7 +80,7 @@
config.labels_enabled = false
params = { attributes: {}, time: 1_478_113_516_002 }
impressions = []
impressions << impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, params)
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo, treatment1, false, params), :disabled => false }
impressions_manager.track(impressions)

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

config.impressions_bulk_size = 1
Expand Down Expand Up @@ -142,8 +142,8 @@
treatment = { treatment: 'on', label: 'sample_rule', change_number: 1_533_177_602_748 }
params = { attributes: {}, time: 1_478_113_516_002 }
impressions = []
impressions << impressions_manager.build_impression('matching_key1', nil, :foo1, treatment, params)
impressions << impressions_manager.build_impression('matching_key2', nil, :foo1, treatment, params)
impressions << { :impression => impressions_manager.build_impression('matching_key1', nil, :foo1, treatment, false, params), :disabled => false }
impressions << { :impression => impressions_manager.build_impression('matching_key2', nil, :foo1, treatment, false, params), :disabled => false }
impressions_manager.track(impressions)

expect(repository.batch.size).to eq(1)
Expand Down Expand Up @@ -200,8 +200,8 @@
expect(config.impressions_adapter).to receive(:expire).once.with(anything, 3600)
params = { attributes: {}, time: 1_478_113_516_002 }
impressions = []
impressions << impressions_manager.build_impression('matching_key', nil, :foo1, treatment, params)
impressions << impressions_manager.build_impression('matching_key', nil, :foo1, treatment, params)
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params), :disabled => false }
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params), :disabled => false }
impressions_manager.track(impressions)
end

Expand All @@ -211,7 +211,7 @@

params = { attributes: {}, time: 1_478_113_516_002 }
impressions = []
impressions << impressions_manager.build_impression('matching_key', nil, :foo1, treatment, params)
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params), :disabled => false }
impressions_manager.track(impressions)

expect(repository.batch).to eq([])
Expand All @@ -221,8 +221,8 @@
other_treatment = { treatment: 'on', label: 'sample_rule_2', change_number: 1_533_177_602_748 }
params = { attributes: {}, time: 1_478_113_516_002 }
impressions = []
impressions << impressions_manager.build_impression('matching_key', nil, :foo1, treatment, params)
impressions << impressions_manager.build_impression('matching_key', nil, :foo2, other_treatment, params)
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params), :disabled => false }
impressions << { :impression => impressions_manager.build_impression('matching_key', nil, :foo2, other_treatment, false, params), :disabled => false }
impressions_manager.track(impressions)

adapter.get_from_queue('SPLITIO.impressions', 0).map do |e|
Expand Down Expand Up @@ -252,8 +252,8 @@

params = { attributes: {}, time: 1_478_113_516_002 }
impressions = []
impressions << custom_impressions_manager.build_impression('matching_key', nil, :foo1, treatment, params)
impressions << custom_impressions_manager.build_impression('matching_key', nil, :foo2, other_treatment, params)
impressions << { :impression => custom_impressions_manager.build_impression('matching_key', nil, :foo1, treatment, false, params), :disabled => false }
impressions << { :impression => custom_impressions_manager.build_impression('matching_key', nil, :foo2, other_treatment, false, params), :disabled => false }
custom_impressions_manager.track(impressions)

custom_adapter.get_from_queue('SPLITIO.impressions', 0).map do |e|
Expand Down
6 changes: 3 additions & 3 deletions spec/cache/senders/impressions_formatter_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
params2 = { attributes: {}, time: 1_478_113_518_285 }

@impressions = []
@impressions << impressions_manager.build_impression('matching_key', 'foo1', 'foo1', treatment1, params)
@impressions << impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, params2)
@impressions << { :impression => impressions_manager.build_impression('matching_key', 'foo1', 'foo1', treatment1, false, params), :disabled => false }
@impressions << { :impression => impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, false, params2), :disabled => false }
impressions_manager.track(@impressions)
end

Expand Down Expand Up @@ -75,7 +75,7 @@
it 'formats multiple impressions for one key' do
params = { attributes: {}, time: 1_478_113_518_900 }
impressions = []
impressions << impressions_manager.build_impression('matching_key3', nil, 'foo2', treatment3, params)
impressions << { :impression => impressions_manager.build_impression('matching_key3', nil, 'foo2', treatment3, false, params), :disabled => false }
impressions_manager.track(impressions)

expect(formatted_impressions.find { |i| i[:f] == :foo1 }[:i]).to match_array(
Expand Down
4 changes: 2 additions & 2 deletions spec/cache/senders/impressions_sender_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@
params = { attributes: {}, time: 1_478_113_516_002 }
params2 = { attributes: {}, time: 1_478_113_518_285 }
impressions = []
impressions << impressions_manager.build_impression('matching_key', 'foo1', 'foo1', treatment1, params)
impressions << impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, params2)
impressions << { :impression => impressions_manager.build_impression('matching_key', 'foo1', 'foo1', treatment1, false, params), :disabled => false }
impressions << { :impression => impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, false, params2), :disabled => false }
impressions_manager.track(impressions)
end

Expand Down
5 changes: 3 additions & 2 deletions spec/cache/senders/localhost_repo_cleaner_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,12 @@
treatment_data = { treatment: 'on', label: 'sample_rule', change_number: 1_533_177_602_748 }
params = { attributes: {}, time: nil }
impressions = []
impressions << impressions_manager.build_impression('matching_key',
impressions << { :impression => impressions_manager.build_impression('matching_key',
'foo1',
'foo1',
treatment_data,
params)
false,
params), :disabled => false }

impressions_manager.track(impressions)

Expand Down
19 changes: 13 additions & 6 deletions spec/engine_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,18 @@
stub_request(:any, /https:\/\/events.*/).to_return(status: 200, body: '')
stub_request(:get, /https:\/\/sdk\.split\.io\/api\/splitChanges\?s=1\.1&since/)
.to_return(status: 200, body: '')
stub_request(:post, "https://telemetry.split.io/api/v1/metrics/config")
.to_return(status: 200, body: '')
end

before :each do
Redis.new.flushall if @mode.equal?(:consumer)
end

after(:each) do
subject.destroy
end

context '#equal_to_set_matcher and get_treatment validation attributes' do
before do
equal_to_set_matcher_json = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/engine/equal_to_set_matcher.json'))
Expand Down Expand Up @@ -726,6 +732,7 @@

context 'whitelist matcher' do
before do
stub_request(:get, "https://sdk.split.io/api/segmentChanges/demo?since=-1").to_return(status: 200, body: "")
whitelist_matcher_json = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/engine/whitelist_matcher.json'))
load_splits(whitelist_matcher_json, flag_sets_json)
subject.block_until_ready
Expand Down Expand Up @@ -832,20 +839,18 @@
before do
impressions_test_json = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/engine/impressions_test.json'))
load_splits(impressions_test_json, flag_sets_json)
stub_request(:get, /https:\/\/sdk\.split\.io\/api\/splitChanges\?s=1\.1&since/).to_return(status: 200, body: '')
subject.block_until_ready(5)
end

it 'returns correct impressions for get_treatments checking ' do
subject.get_treatments('26', %w[sample_feature beta_feature])
# Need this because we're storing impressions in the Set
# Without sleep we may have identical impressions (including time)
# In that case only one impression with key "26" would be stored
sleep 1

subject.get_treatments('26', %w[sample_feature beta_feature])

sleep 1
impressions = customer_impression_listener.queue

expect(impressions.size >= 2).to be true
close_redis
end
Expand All @@ -872,7 +877,8 @@
before do
traffic_allocation_json = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/splits_traffic_allocation.json'))
load_splits(traffic_allocation_json, flag_sets_json)
subject.block_until_ready
subject.block_until_ready(5)
add_splits_to_repository(traffic_allocation_json)
end

it 'returns expected treatment' do
Expand Down Expand Up @@ -1267,7 +1273,8 @@

def load_splits(splits_json, flag_sets_json)
if @mode.equal?(:standalone)
stub_request(:get, /https:\/\/sdk\.split\.io\/api\/splitChanges\?s=1\.1&since.*/)
# stub_request(:get, /https:\/\/sdk\.split\.io\/api\/splitChanges\?s=1\.1&since.*/)
stub_request(:get, "https://sdk.split.io/api/splitChanges?s=1.1&since=-1")
.to_return(status: 200, body: splits_json)
else
add_splits_to_repository(splits_json)
Expand Down
Loading