diff --git a/lib/splitclient-rb.rb b/lib/splitclient-rb.rb index b4ffcdeb..30e0a2b3 100644 --- a/lib/splitclient-rb.rb +++ b/lib/splitclient-rb.rb @@ -109,6 +109,7 @@ require 'splitclient-rb/engine/models/segment_type' require 'splitclient-rb/engine/models/treatment' require 'splitclient-rb/engine/models/split_http_response' +require 'splitclient-rb/engine/models/evaluation_options' require 'splitclient-rb/engine/auth_api_client' require 'splitclient-rb/engine/back_off' require 'splitclient-rb/engine/push_manager' diff --git a/lib/splitclient-rb/clients/split_client.rb b/lib/splitclient-rb/clients/split_client.rb index 4e6e17a7..523c5d0d 100644 --- a/lib/splitclient-rb/clients/split_client.rb +++ b/lib/splitclient-rb/clients/split_client.rb @@ -35,23 +35,23 @@ def initialize(sdk_key, repositories, status_manager, config, impressions_manage end def get_treatment( - key, split_name, attributes = {}, options = nil, split_data = nil, store_impressions = true, + key, split_name, attributes = {}, evaluation_options = nil, split_data = nil, store_impressions = true, multiple = false, evaluator = nil ) - result = treatment(key, split_name, attributes, split_data, store_impressions, GET_TREATMENT, multiple, options) + result = treatment(key, split_name, attributes, split_data, store_impressions, GET_TREATMENT, multiple, evaluation_options) return result.tap { |t| t.delete(:config) } if multiple result[:treatment] end def get_treatment_with_config( - key, split_name, attributes = {}, options = nil, split_data = nil, store_impressions = true, + key, split_name, attributes = {}, evaluation_options = nil, split_data = nil, store_impressions = true, multiple = false, evaluator = nil ) - treatment(key, split_name, attributes, split_data, store_impressions, GET_TREATMENT_WITH_CONFIG, multiple, options) + treatment(key, split_name, attributes, split_data, store_impressions, GET_TREATMENT_WITH_CONFIG, multiple, evaluation_options) end - def get_treatments(key, split_names, attributes = {}, options = nil) - treatments = treatments(key, split_names, attributes, options) + def get_treatments(key, split_names, attributes = {}, evaluation_options = nil) + treatments = treatments(key, split_names, attributes, evaluation_options) return treatments if treatments.nil? keys = treatments.keys @@ -59,40 +59,40 @@ def get_treatments(key, split_names, attributes = {}, options = nil) Hash[keys.zip(treats)] end - def get_treatments_with_config(key, split_names, attributes = {}, options = nil) - treatments(key, split_names, attributes, options, GET_TREATMENTS_WITH_CONFIG) + def get_treatments_with_config(key, split_names, attributes = {}, evaluation_options = nil) + treatments(key, split_names, attributes, evaluation_options, GET_TREATMENTS_WITH_CONFIG) end - def get_treatments_by_flag_set(key, flag_set, attributes = {}, options = nil) + def get_treatments_by_flag_set(key, flag_set, attributes = {}, evaluation_options = nil) valid_flag_set = @split_validator.valid_flag_sets(GET_TREATMENTS_BY_FLAG_SET, [flag_set]) split_names = @splits_repository.get_feature_flags_by_sets(valid_flag_set) - treatments = treatments(key, split_names, attributes, options, GET_TREATMENTS_BY_FLAG_SET) + treatments = treatments(key, split_names, attributes, evaluation_options, GET_TREATMENTS_BY_FLAG_SET) return treatments if treatments.nil? keys = treatments.keys treats = treatments.map { |_,t| t[:treatment] } Hash[keys.zip(treats)] end - def get_treatments_by_flag_sets(key, flag_sets, attributes = {}, options = nil) + def get_treatments_by_flag_sets(key, flag_sets, attributes = {}, evaluation_options = nil) valid_flag_set = @split_validator.valid_flag_sets(GET_TREATMENTS_BY_FLAG_SETS, flag_sets) split_names = @splits_repository.get_feature_flags_by_sets(valid_flag_set) - treatments = treatments(key, split_names, attributes, options, GET_TREATMENTS_BY_FLAG_SETS) + treatments = treatments(key, split_names, attributes, evaluation_options, GET_TREATMENTS_BY_FLAG_SETS) return treatments if treatments.nil? keys = treatments.keys treats = treatments.map { |_,t| t[:treatment] } Hash[keys.zip(treats)] end - def get_treatments_with_config_by_flag_set(key, flag_set, attributes = {}, options = nil) + def get_treatments_with_config_by_flag_set(key, flag_set, attributes = {}, evaluation_options = nil) valid_flag_set = @split_validator.valid_flag_sets(GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET, [flag_set]) split_names = @splits_repository.get_feature_flags_by_sets(valid_flag_set) - treatments(key, split_names, attributes, options, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET) + treatments(key, split_names, attributes, evaluation_options, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET) end - def get_treatments_with_config_by_flag_sets(key, flag_sets, attributes = {}, options = nil) + def get_treatments_with_config_by_flag_sets(key, flag_sets, attributes = {}, evaluation_options = nil) valid_flag_set = @split_validator.valid_flag_sets(GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS, flag_sets) split_names = @splits_repository.get_feature_flags_by_sets(valid_flag_set) - treatments(key, split_names, attributes, options, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS) + treatments(key, split_names, attributes, evaluation_options, GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS) end def destroy @@ -235,18 +235,14 @@ def validate_properties(properties, method = 'Event') return fixed_properties, size end - def validate_options(options) - if !options.is_a?(Hash) - @config.logger.warn("Option #{options} should be a hash type. Setting value to nil") + def validate_evaluation_options(evaluation_options) + if !evaluation_options.is_a?(SplitIoClient::Engine::Models::EvaluationOptions) + @config.logger.warn("Option #{evaluation_options} should be a EvaluationOptions type. Setting value to nil") return nil, 0 end - options = options.transform_keys(&:to_sym) - if !options.key?(:properties) - @config.logger.warn("Option #{options} hash does not contain properties key. Setting value to nil") - return nil, 0 - end - options[:properties], size = validate_properties(options[:properties], method = 'Treatment') - return options, size + evaluation_options.properties = evaluation_options.properties.transform_keys(&:to_sym) + evaluation_options.properties, size = validate_properties(evaluation_options.properties, method = 'Treatment') + return evaluation_options, size end def valid_client @@ -257,7 +253,7 @@ def valid_client @config.valid_mode end - def treatments(key, feature_flag_names, attributes = {}, options = nil, calling_method = 'get_treatments') + def treatments(key, feature_flag_names, attributes = {}, evaluation_options = nil, calling_method = 'get_treatments') sanitized_feature_flag_names = sanitize_split_names(calling_method, feature_flag_names) if sanitized_feature_flag_names.nil? @@ -290,7 +286,7 @@ def treatments(key, feature_flag_names, attributes = {}, options = nil, calling_ to_return[name.to_sym] = control_treatment_with_config 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 }, - options), :disabled => false } + evaluation_options), :disabled => false } } @impressions_manager.track(impressions) return to_return @@ -312,7 +308,7 @@ def treatments(key, feature_flag_names, attributes = {}, options = nil, calling_ invalid_treatments[key] = control_treatment_with_config next end - treatments_labels_change_numbers, impressions = evaluate_treatment(feature_flag, key, bucketing_key, matching_key, attributes, calling_method, false, options) + treatments_labels_change_numbers, impressions = evaluate_treatment(feature_flag, key, bucketing_key, matching_key, attributes, calling_method, false, evaluation_options) treatments[key] = { treatment: treatments_labels_change_numbers[:treatment], @@ -335,7 +331,7 @@ def treatments(key, feature_flag_names, attributes = {}, options = nil, calling_ # @param store_impressions [Boolean] impressions aren't stored if this flag is false # @return [String/Hash] Treatment as String or Hash of treatments in case of array of features def treatment(key, feature_flag_name, attributes = {}, split_data = nil, store_impressions = true, - calling_method = 'get_treatment', multiple = false, options = nil) + calling_method = 'get_treatment', multiple = false, evaluation_options = nil) impressions = [] bucketing_key, matching_key = keys_from_key(key) @@ -353,13 +349,13 @@ def treatment(key, feature_flag_name, attributes = {}, split_data = nil, store_i end feature_flag = @splits_repository.get_split(feature_flag_name) - treatments, impressions_decorator = evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_key, attributes, calling_method, multiple, options) + treatments, impressions_decorator = evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_key, attributes, calling_method, multiple, evaluation_options) @impressions_manager.track(impressions_decorator) unless impressions_decorator.nil? treatments end - def evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_key, attributes, calling_method, multiple = false, options = nil) + def evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_key, attributes, calling_method, multiple = false, evaluation_options = nil) impressions_decorator = [] begin start = Time.now @@ -380,16 +376,16 @@ def evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_ impressions_disabled = false end - options, size = validate_options(options) unless options.nil? - options[:properties] = nil unless options.nil? or check_properties_size((EVENT_AVERAGE_SIZE + size), "Properties are ignored") + evaluation_options, size = validate_evaluation_options(evaluation_options) unless evaluation_options.nil? + evaluation_options.properties = nil unless evaluation_options.nil? or check_properties_size((EVENT_AVERAGE_SIZE + size), "Properties are ignored") record_latency(calling_method, start) - impression_decorator = { :impression => @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, treatment_data, impressions_disabled, { attributes: attributes, time: nil }, options), :disabled => impressions_disabled } + impression_decorator = { :impression => @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, treatment_data, impressions_disabled, { attributes: attributes, time: nil }, evaluation_options), :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, false, { attributes: attributes, time: nil }, options), :disabled => false } + impression_decorator = { :impression => @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, control_treatment, false, { attributes: attributes, time: nil }, evaluation_options), :disabled => false } impressions_decorator << impression_decorator unless impression_decorator.nil? return parsed_treatment(control_treatment.merge({ :label => Engine::Models::Label::EXCEPTION }), multiple), impressions_decorator diff --git a/lib/splitclient-rb/engine/common/impressions_manager.rb b/lib/splitclient-rb/engine/common/impressions_manager.rb index a2b196cc..d685dd7b 100644 --- a/lib/splitclient-rb/engine/common/impressions_manager.rb +++ b/lib/splitclient-rb/engine/common/impressions_manager.rb @@ -19,22 +19,23 @@ def initialize(config, end def build_impression(matching_key, bucketing_key, split_name, treatment_data, impressions_disabled, params = {}, - options = nil) - properties = options.nil? ? nil : options[:properties] + evaluation_options = nil) + properties = get_properties(evaluation_options) impression_data = impression_data(matching_key, bucketing_key, split_name, treatment_data, params[:time], properties) + return impression(impression_data, params[:attributes]) if check_return_conditions(properties) + begin - if @config.impressions_mode == :none || impressions_disabled + if check_none_mode(impressions_disabled) @impression_counter.inc(split_name, impression_data[:m]) @unique_keys_tracker.track(split_name, matching_key) - elsif @config.impressions_mode == :debug # In DEBUG mode we should calculate the pt only. - return impression(impression_data, params[:attributes]) unless properties.nil? - - impression_data[:pt] = @impression_observer.test_and_set(impression_data) - else # In OPTIMIZED mode we should track the total amount of evaluations and deduplicate the impressions. - return impression(impression_data, params[:attributes]) unless properties.nil? - + end + if check_observe_impressions + # In DEBUG mode we should calculate the pt only. impression_data[:pt] = @impression_observer.test_and_set(impression_data) - @impression_counter.inc(split_name, impression_data[:m]) unless impression_data[:pt].nil? + end + if check_impression_counter(impression_data) + # In OPTIMIZED mode we should track the total amount of evaluations and deduplicate the impressions. + @impression_counter.inc(split_name, impression_data[:m]) end rescue StandardError => e @config.log_found_exception(__method__.to_s, e) @@ -67,6 +68,26 @@ def track(impressions_decorator) private + def check_return_conditions(properties) + return (@config.impressions_mode == :debug || @config.impressions_mode == :optimized) && !properties.nil? + end + + def check_none_mode(impressions_disabled) + return @config.impressions_mode == :none || impressions_disabled + end + + def check_observe_impressions + return @config.impressions_mode == :debug || @config.impressions_mode == :optimized + end + + def check_impression_counter(impression_data) + return @config.impressions_mode == :optimized && !impression_data[:pt].nil? + end + + def get_properties(evaluation_options) + return evaluation_options.nil? ? nil : evaluation_options.properties + end + def impression_router @impression_router ||= SplitIoClient::ImpressionRouter.new(@config) rescue StandardError => e diff --git a/lib/splitclient-rb/engine/models/evaluation_options.rb b/lib/splitclient-rb/engine/models/evaluation_options.rb new file mode 100644 index 00000000..c79c710a --- /dev/null +++ b/lib/splitclient-rb/engine/models/evaluation_options.rb @@ -0,0 +1,9 @@ +module SplitIoClient::Engine::Models + class EvaluationOptions + attr_accessor :properties + + def initialize(properties) + @properties = properties + end + end +end diff --git a/spec/cache/senders/impressions_formatter_spec.rb b/spec/cache/senders/impressions_formatter_spec.rb index 4e85dfeb..6c649a48 100644 --- a/spec/cache/senders/impressions_formatter_spec.rb +++ b/spec/cache/senders/impressions_formatter_spec.rb @@ -75,7 +75,7 @@ it 'formats multiple impressions for one key' do params = { attributes: {}, time: 1_478_113_518_900 } impressions = [] - impressions << { :impression => impressions_manager.build_impression('matching_key3', nil, 'foo2', treatment3, false, params, {:properties => {:prop => "val"}}), :disabled => false } + impressions << { :impression => impressions_manager.build_impression('matching_key3', nil, 'foo2', treatment3, false, params, SplitIoClient::Engine::Models::EvaluationOptions.new({:prop => "val"})), :disabled => false } impressions_manager.track(impressions) expect(formatted_impressions.find { |i| i[:f] == :foo1 }[:i]).to match_array( diff --git a/spec/cache/senders/impressions_sender_spec.rb b/spec/cache/senders/impressions_sender_spec.rb index 975d1a8b..ccb5b885 100644 --- a/spec/cache/senders/impressions_sender_spec.rb +++ b/spec/cache/senders/impressions_sender_spec.rb @@ -46,7 +46,7 @@ params2 = { attributes: {}, time: 1_478_113_518_285 } impressions = [] 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, {:properties => {:prop => "val"}}), :disabled => false } + impressions << { :impression => impressions_manager.build_impression('matching_key2', 'foo2', 'foo2', treatment2, false, params2, SplitIoClient::Engine::Models::EvaluationOptions.new({:prop => "val"})), :disabled => false } impressions_manager.track(impressions) end diff --git a/spec/engine/common/impression_manager_spec.rb b/spec/engine/common/impression_manager_spec.rb index c23167c5..02aab198 100644 --- a/spec/engine/common/impression_manager_spec.rb +++ b/spec/engine/common/impression_manager_spec.rb @@ -72,7 +72,7 @@ 'split_name_test', treatment, false, - params, {"properties": {"prop":"val"}}) + params, SplitIoClient::Engine::Models::EvaluationOptions.new({"prop":"val"})) expect(impression).to match(expected) result_count = impression_counter.pop_all diff --git a/spec/splitclient/split_client_spec.rb b/spec/splitclient/split_client_spec.rb index 9ff88c25..6c49fffd 100644 --- a/spec/splitclient/split_client_spec.rb +++ b/spec/splitclient/split_client_spec.rb @@ -37,21 +37,21 @@ expect(split_client.get_treatments_with_config_by_flag_sets('key', ['set_2'])).to eq({:testing222 => {:treatment => 'off', :config => nil}}) imps = impressions_repository.batch - expect(split_client.get_treatment('key', 'testing222', {}, {:properties => {:prop => "value"}})).to eq('off') + expect(split_client.get_treatment('key', 'testing222', {}, SplitIoClient::Engine::Models::EvaluationOptions.new({:prop => "value"}))).to eq('off') check_properties(impressions_repository.batch) - expect(split_client.get_treatments('key_prop', ['testing222'], {}, {:properties => {:prop => "value"}})).to eq({:testing222 => 'off'}) + expect(split_client.get_treatments('key_prop', ['testing222'], {}, SplitIoClient::Engine::Models::EvaluationOptions.new({:prop => "value"}))).to eq({:testing222 => 'off'}) check_properties(impressions_repository.batch) - expect(split_client.get_treatment_with_config('key', 'testing222', {}, {:properties => {:prop => "value"}})).to eq({:treatment => 'off', :config => nil}) + expect(split_client.get_treatment_with_config('key', 'testing222', {}, SplitIoClient::Engine::Models::EvaluationOptions.new({:prop => "value"}))).to eq({:treatment => 'off', :config => nil}) check_properties(impressions_repository.batch) - expect(split_client.get_treatments_with_config('key', ['testing222'], {}, {:properties => {:prop => "value"}})).to eq({:testing222 => {:treatment => 'off', :config => nil}}) + expect(split_client.get_treatments_with_config('key', ['testing222'], {}, SplitIoClient::Engine::Models::EvaluationOptions.new({:prop => "value"}))).to eq({:testing222 => {:treatment => 'off', :config => nil}}) check_properties(impressions_repository.batch) - expect(split_client.get_treatments_by_flag_set('key', 'set_1', {}, {:properties => {:prop => "value"}})).to eq({:testing222 => 'off'}) + expect(split_client.get_treatments_by_flag_set('key', 'set_1', {}, SplitIoClient::Engine::Models::EvaluationOptions.new({:prop => "value"}))).to eq({:testing222 => 'off'}) check_properties(impressions_repository.batch) - expect(split_client.get_treatments_by_flag_sets('key', ['set_2'], {}, {:properties => {:prop => "value"}})).to eq({:testing222 => 'off'}) + expect(split_client.get_treatments_by_flag_sets('key', ['set_2'], {}, SplitIoClient::Engine::Models::EvaluationOptions.new({:prop => "value"}))).to eq({:testing222 => 'off'}) check_properties(impressions_repository.batch) - expect(split_client.get_treatments_with_config_by_flag_set('key', 'set_1', {}, {:properties => {:prop => "value"}})).to eq({:testing222 => {:treatment => 'off', :config => nil}}) + expect(split_client.get_treatments_with_config_by_flag_set('key', 'set_1', {}, SplitIoClient::Engine::Models::EvaluationOptions.new({:prop => "value"}))).to eq({:testing222 => {:treatment => 'off', :config => nil}}) check_properties(impressions_repository.batch) - expect(split_client.get_treatments_with_config_by_flag_sets('key', ['set_2'], {}, {:properties => {:prop => "value"}})).to eq({:testing222 => {:treatment => 'off', :config => nil}}) + expect(split_client.get_treatments_with_config_by_flag_sets('key', ['set_2'], {}, SplitIoClient::Engine::Models::EvaluationOptions.new({:prop => "value"}))).to eq({:testing222 => {:treatment => 'off', :config => nil}}) check_properties(impressions_repository.batch) end