From a4d717ddc5ec8a8c22e3882105fadf17fa38590c Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Mon, 16 Dec 2024 10:33:23 -0800 Subject: [PATCH 01/18] cleared bloom filter by deleting and recreating the object --- lib/splitclient-rb/cache/filter/bloom_filter.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/splitclient-rb/cache/filter/bloom_filter.rb b/lib/splitclient-rb/cache/filter/bloom_filter.rb index b06a94ab..70548566 100644 --- a/lib/splitclient-rb/cache/filter/bloom_filter.rb +++ b/lib/splitclient-rb/cache/filter/bloom_filter.rb @@ -8,8 +8,8 @@ module Filter class BloomFilter def initialize(capacity, false_positive_probability = 0.001) @capacity = capacity.round - m = best_m(capacity, false_positive_probability) - @ba = BitArray.new(m.round) + @m = best_m(capacity, false_positive_probability) + reset_filter @k = best_k(capacity) end @@ -17,22 +17,26 @@ def add(string) return false if contains?(string) positions = hashes(string) - positions.each { |position| @ba[position] = 1 } true end - + def contains?(string) !hashes(string).any? { |ea| @ba[ea] == 0 } end def clear - @ba.size.times { |i| @ba[i] = 0 } + @ba.delete() + reset_filter end - + private + def reset_filter + @ba = BitArray.new(@m.round) + end + # m is the required number of bits in the array def best_m(capacity, false_positive_probability) -(capacity * Math.log(false_positive_probability)) / (Math.log(2) ** 2) From eb7509bf2a8477d936c9d21cadf9f728510b0802 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Mon, 16 Dec 2024 10:50:33 -0800 Subject: [PATCH 02/18] setting object to nil instead --- lib/splitclient-rb/cache/filter/bloom_filter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/splitclient-rb/cache/filter/bloom_filter.rb b/lib/splitclient-rb/cache/filter/bloom_filter.rb index 70548566..aa602807 100644 --- a/lib/splitclient-rb/cache/filter/bloom_filter.rb +++ b/lib/splitclient-rb/cache/filter/bloom_filter.rb @@ -27,7 +27,7 @@ def contains?(string) end def clear - @ba.delete() + @ba = nil reset_filter end From 554d84842d96265b88d5c315fcf4180ce490bc1e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 1 Jan 2025 03:08:59 +0000 Subject: [PATCH 03/18] Updated License Year --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index c022e920..df08de3f 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright © 2024 Split Software, Inc. +Copyright © 2025 Split Software, Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From 2df38ac8e667e28bb81335f779bd66369209d72b Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Mon, 6 Jan 2025 12:31:18 -0800 Subject: [PATCH 04/18] updated engine classes and repo helper --- .../engine/common/impressions_manager.rb | 49 +++-- lib/splitclient-rb/engine/synchronizer.rb | 8 +- .../helpers/repository_helper.rb | 5 + spec/engine/common/impression_manager_spec.rb | 171 +++++++++++++----- spec/engine/synchronizer_spec.rb | 7 + spec/repository_helper.rb | 21 +++ 6 files changed, 182 insertions(+), 79 deletions(-) diff --git a/lib/splitclient-rb/engine/common/impressions_manager.rb b/lib/splitclient-rb/engine/common/impressions_manager.rb index a4cfb208..0fee4033 100644 --- a/lib/splitclient-rb/engine/common/impressions_manager.rb +++ b/lib/splitclient-rb/engine/common/impressions_manager.rb @@ -18,19 +18,16 @@ def initialize(config, @unique_keys_tracker = unique_keys_tracker end - def build_impression(matching_key, bucketing_key, split_name, treatment, params = {}) + 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]) - begin - case @config.impressions_mode - when :debug # In DEBUG mode we should calculate the pt only. - impression_data[:pt] = @impression_observer.test_and_set(impression_data) - when :none # In NONE mode we should track the total amount of evaluations and the unique keys. + 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]) @unique_keys_tracker.track(split_name, matching_key) + elsif @config.impressions_mode == :debug # In DEBUG mode we should calculate the pt only. + 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. impression_data[:pt] = @impression_observer.test_and_set(impression_data) - @impression_counter.inc(split_name, impression_data[:m]) unless impression_data[:pt].nil? end rescue StandardError => e @@ -40,24 +37,25 @@ def build_impression(matching_key, bucketing_key, split_name, treatment, params impression(impression_data, params[:attributes]) end - def track(impressions) - return if impressions.empty? - - stats = { dropped: 0, queued: 0, dedupe: 0 } - begin - case @config.impressions_mode - when :none - return - when :debug - track_debug_mode(impressions, stats) - when :optimized - track_optimized_mode(impressions, stats) + def track(impressions_decorator) + return if impressions_decorator.empty? + + impressions_decorator.each do |impression_decorator| + stats = { dropped: 0, queued: 0, dedupe: 0 } + begin + if @config.impressions_mode == :none || impression_decorator[:disabled] + return + elsif @config.impressions_mode == :debug + track_debug_mode([impression_decorator[:impression]], stats) + elsif @config.impressions_mode == :optimized + track_optimized_mode([impression_decorator[:impression]], stats) + end + rescue StandardError => e + @config.log_found_exception(__method__.to_s, e) + ensure + record_stats(stats) + impression_router.add_bulk([impression_decorator[:impression]]) end - rescue StandardError => e - @config.log_found_exception(__method__.to_s, e) - ensure - record_stats(stats) - impression_router.add_bulk(impressions) end end @@ -126,11 +124,10 @@ def track_debug_mode(impressions, stats) def track_optimized_mode(impressions, stats) optimized_impressions = impressions.select { |imp| should_queue_impression?(imp[:i]) } - + stats[:dedupe] = impressions.length - optimized_impressions.length return if optimized_impressions.empty? stats[:dropped] = @impressions_repository.add_bulk(optimized_impressions) - stats[:dedupe] = impressions.length - optimized_impressions.length stats[:queued] = optimized_impressions.length - stats[:dropped] end end diff --git a/lib/splitclient-rb/engine/synchronizer.rb b/lib/splitclient-rb/engine/synchronizer.rb index a1a91843..d2e0b319 100644 --- a/lib/splitclient-rb/engine/synchronizer.rb +++ b/lib/splitclient-rb/engine/synchronizer.rb @@ -48,7 +48,7 @@ def start_periodic_data_recording events_sender start_telemetry_sync_task end - + impressions_count_sender start_unique_keys_tracker_task end @@ -175,7 +175,7 @@ def attempt_splits_sync(target_cn, fetch_options, max_retries, retry_delay_secon # Starts thread which loops constantly and sends impressions to the Split API def impressions_sender - ImpressionsSender.new(@impressions_repository, @config, @impressions_api).call unless @config.impressions_mode == :none + ImpressionsSender.new(@impressions_repository, @config, @impressions_api).call end # Starts thread which loops constantly and sends events to the Split API @@ -185,7 +185,7 @@ def events_sender # Starts thread which loops constantly and sends impressions count to the Split API def impressions_count_sender - ImpressionsCountSender.new(@config, @impression_counter, @impressions_sender_adapter).call unless @config.impressions_mode == :debug + ImpressionsCountSender.new(@config, @impression_counter, @impressions_sender_adapter).call end def start_telemetry_sync_task @@ -203,7 +203,7 @@ def sync_result(success, remaining_attempts, segment_names = nil) def sync_splits_and_segments @config.logger.debug('Synchronizing feature flags and segments ...') if @config.debug_enabled splits_result = @split_fetcher.fetch_splits - + splits_result[:success] && @segment_fetcher.fetch_segments end end diff --git a/lib/splitclient-rb/helpers/repository_helper.rb b/lib/splitclient-rb/helpers/repository_helper.rb index c53e53ae..c4cea4b7 100644 --- a/lib/splitclient-rb/helpers/repository_helper.rb +++ b/lib/splitclient-rb/helpers/repository_helper.rb @@ -13,6 +13,11 @@ def self.update_feature_flag_repository(feature_flag_repository, feature_flags, next end + unless feature_flag.key?(:impressionsDisabled) + feature_flag[:impressionsDisabled] = false + config.logger.debug("feature flag (#{feature_flag[:name]}) does not have `impressionsDisabled` field, setting it to `false`") if config.debug_enabled + end + config.logger.debug("storing feature flag (#{feature_flag[:name]})") if config.debug_enabled to_add.push(feature_flag) end diff --git a/spec/engine/common/impression_manager_spec.rb b/spec/engine/common/impression_manager_spec.rb index 7044183a..21875236 100644 --- a/spec/engine/common/impression_manager_spec.rb +++ b/spec/engine/common/impression_manager_spec.rb @@ -47,8 +47,7 @@ unique_keys_tracker) end - # TODO: remove skip test when the sdk support :none mode. - xit 'build & track impression' do + it 'build & track impression' do expected = { m: { s: version, i: ip, n: machine_name }, @@ -71,6 +70,7 @@ 'bucketing_key_test', 'split_name_test', treatment, + false, params) expect(impression).to match(expected) @@ -78,12 +78,50 @@ expect(result_count['split_name_test::1478113200000']).to eq(1) expect(unique_cache['split_name_test'].size).to eq(1) - impression_manager.track([impression]) + impression_manager.track([{impression: impression, disabled: false}]) sleep 0.5 expect(impression_repository.batch.size).to eq(0) expect(impression_listener.size).to eq(1) end + + it 'impression toggle' do + expected = + { + m: { s: version, i: ip, n: machine_name }, + i: { + k: 'matching_key_test', + b: 'bucketing_key_test', + f: 'split_name_test', + t: 'off', + r: 'default label', + c: 1_478_113_516_002, + m: 1_478_113_516_222, + pt: nil + }, + attributes: {} + } + treatment = { treatment: 'off', label: 'default label', change_number: 1_478_113_516_002 } + params = { attributes: {}, time: 1_478_113_516_222 } + + impression = impression_manager.build_impression('matching_key_test', + 'bucketing_key_test', + 'split_name_test', + treatment, + true, + params) + expect(impression).to match(expected) + + result_count = impression_counter.pop_all + expect(result_count['split_name_test::1478113200000']).to eq(1) + expect(unique_cache['split_name_test'].size).to eq(1) + + impression_manager.track([{impression: impression, disabled: true}]) + + sleep 0.5 + expect(impression_repository.batch.size).to eq(0) + expect(impression_listener.size).to eq(1) + end end context 'impressions in optimized mode' do @@ -127,14 +165,14 @@ treatment = { treatment: 'off', label: 'default label', change_number: 1_478_113_516_002 } params = { attributes: {}, time: 1_478_113_516_222 } - res = impression_manager.build_impression('matching_key_test', 'bucketing_key_test', 'split_name_test', treatment, params) + res = impression_manager.build_impression('matching_key_test', 'bucketing_key_test', 'split_name_test', treatment, true, params) expect(res).to match(expected) end it 'track' do impressions = [] - impressions << expected + impressions << {impression: expected, disabled: false} impression_manager.track(impressions) @@ -150,16 +188,16 @@ params = { attributes: {}, time: expected[:i][:m] } imp = expected[:i] - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} - impressions << impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, params) + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, false, params), disabled: false} - impressions << impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, params) + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} impression_manager.track(impressions) @@ -175,31 +213,30 @@ treatment_data = { treatment: 'off', label: 'default label', change_number: 1_478_113_516_002 } params = { attributes: {}, time: expected[:i][:m] } imp = expected[:i] - - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) - - impressions << impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, params) - - impressions << impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, params) - - impressions << impression_manager.build_impression('other_key', imp[:b], 'test_split', treatment_data, params) - - impressions << impression_manager.build_impression('other_key-1', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('other_key-2', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('other_key-3', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('other_key-4', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('other_key-5', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('other_key-6', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('other_key-7', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('other_key-8', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('other_key-9', imp[:b], 'test_split', treatment_data, params) + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} + + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, false, params), disabled: false} + + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + + impressions << {impression: impression_manager.build_impression('other_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + + impressions << {impression: impression_manager.build_impression('other_key-1', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('other_key-2', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('other_key-3', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('other_key-4', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('other_key-5', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('other_key-6', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('other_key-7', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('other_key-8', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('other_key-9', imp[:b], 'test_split', treatment_data, false, params), disabled: false} impression_manager.track(impressions) @@ -208,6 +245,24 @@ expect(runtime_consumer.impressions_stats(SplitIoClient::Telemetry::Domain::Constants::IMPRESSIONS_QUEUED)).to be(10) expect(runtime_consumer.impressions_stats(SplitIoClient::Telemetry::Domain::Constants::IMPRESSIONS_DEDUPE)).to be(7) end + + it 'impressions toggle' do + impressions = [] + + treatment_data = { treatment: 'off', label: 'default label', change_number: 1_478_113_516_002 } + params = { attributes: {}, time: expected[:i][:m] } + imp = expected[:i] + + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, true, params), disabled: true} + impression_manager.track(impressions) + sleep(0.5) + + expect(impression_repository.batch.size).to eq(0) + expect(impression_listener.size).to eq(1) + result_count = impression_counter.pop_all + expect(result_count['split_name_test::1478113200000']).to eq(1) + expect(unique_cache['split_name_test'].size).to eq(1) + end end context 'impressions in debug mode' do @@ -255,20 +310,20 @@ params = { attributes: {}, time: expected[:i][:m] } imp = expected[:i] - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, params) + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, false, params), disabled: false} - impressions << impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, params) - impressions << impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, params) + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], imp[:f], treatment_data, false, params), disabled: false} - impressions << impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, params) - impressions << impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, params) + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} + impressions << {impression: impression_manager.build_impression('second_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} - impressions << impression_manager.build_impression('other_key', imp[:b], 'test_split', treatment_data, params) + impressions << {impression: impression_manager.build_impression('other_key', imp[:b], 'test_split', treatment_data, false, params), disabled: false} impression_manager.track(impressions) @@ -277,5 +332,23 @@ expect(runtime_consumer.impressions_stats(SplitIoClient::Telemetry::Domain::Constants::IMPRESSIONS_QUEUED)).to be(10) expect(runtime_consumer.impressions_stats(SplitIoClient::Telemetry::Domain::Constants::IMPRESSIONS_DEDUPE)).to be(0) end + + it 'impressions toggle' do + impressions = [] + + treatment_data = { treatment: 'off', label: 'default label', change_number: 1_478_113_516_002 } + params = { attributes: {}, time: expected[:i][:m] } + imp = expected[:i] + + impressions << {impression: impression_manager.build_impression(imp[:k], imp[:b], imp[:f], treatment_data, true, params), disabled: true} + impression_manager.track(impressions) + sleep(0.5) + + expect(impression_repository.batch.size).to eq(0) + expect(impression_listener.size).to eq(1) + result_count = impression_counter.pop_all + expect(result_count['split_name_test::1478113200000']).to eq(1) + expect(unique_cache['split_name_test'].size).to eq(1) + end end end diff --git a/spec/engine/synchronizer_spec.rb b/spec/engine/synchronizer_spec.rb index 74599346..4a920b55 100644 --- a/spec/engine/synchronizer_spec.rb +++ b/spec/engine/synchronizer_spec.rb @@ -47,6 +47,13 @@ mock_segment_changes('segment3', segment3, '1470947453879') end + it 'senders should start always' do + synchronizer.start_periodic_data_recording + sleep (2) + expect(config.threads.key?(:impressions_sender)).to eq(true) + expect(config.threads.key?(:impressions_count_sender)).to eq(true) + end + it 'sync_all asynchronous - should return true' do result = synchronizer.sync_all diff --git a/spec/repository_helper.rb b/spec/repository_helper.rb index f4a72ab6..0f091f78 100644 --- a/spec/repository_helper.rb +++ b/spec/repository_helper.rb @@ -47,5 +47,26 @@ SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ARCHIVED', conditions: [], :sets => ['set_1']}], -1, config) expect(feature_flag_repository.get_split('split1').nil?).to eq(true) end + + it 'test impressions toggle' do + config = SplitIoClient::SplitConfig.new(cache_adapter: :memory) + flag_set_filter = SplitIoClient::Cache::Filter::FlagSetsFilter.new([]) + flag_sets_repository = SplitIoClient::Cache::Repositories::MemoryFlagSetsRepository.new([]) + feature_flag_repository = SplitIoClient::Cache::Repositories::SplitsRepository.new( + config, + flag_sets_repository, + flag_set_filter) + + SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split1', :status => 'ACTIVE', conditions: [], :sets => []}], -1, config) + expect(feature_flag_repository.get_split('split1')[:impressionsDisabled]).to eq(false) + + SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split2', :status => 'ACTIVE', conditions: [], :impressionsDisabled => false, :sets => []}], -1, config) + expect(feature_flag_repository.get_split('split2')[:impressionsDisabled]).to eq(false) + + SplitIoClient::Helpers::RepositoryHelper.update_feature_flag_repository(feature_flag_repository, [{:name => 'split2', :status => 'ACTIVE', conditions: [], :impressionsDisabled => true, :sets => []}], -1, config) + expect(feature_flag_repository.get_split('split2')[:impressionsDisabled]).to eq(true) + + + end end end From 647a43fbb3b84ca24e6e4358a33898c5c65c60e9 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Tue, 7 Jan 2025 09:41:27 -0800 Subject: [PATCH 05/18] polish --- .../engine/common/impressions_manager.rb | 10 +++++----- lib/splitclient-rb/helpers/repository_helper.rb | 4 +++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/splitclient-rb/engine/common/impressions_manager.rb b/lib/splitclient-rb/engine/common/impressions_manager.rb index 0fee4033..68ae13a3 100644 --- a/lib/splitclient-rb/engine/common/impressions_manager.rb +++ b/lib/splitclient-rb/engine/common/impressions_manager.rb @@ -21,7 +21,7 @@ def initialize(config, 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]) begin - if @config.impressions_mode == :none || impressions_disabled # In NONE mode we should track the total amount of evaluations and the unique keys. + if @config.impressions_mode == :none || 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. @@ -43,11 +43,11 @@ def track(impressions_decorator) impressions_decorator.each do |impression_decorator| stats = { dropped: 0, queued: 0, dedupe: 0 } begin - if @config.impressions_mode == :none || impression_decorator[:disabled] - return - elsif @config.impressions_mode == :debug + next if @config.impressions_mode == :none || impression_decorator[:disabled] + + if @config.impressions_mode == :debug track_debug_mode([impression_decorator[:impression]], stats) - elsif @config.impressions_mode == :optimized + else track_optimized_mode([impression_decorator[:impression]], stats) end rescue StandardError => e diff --git a/lib/splitclient-rb/helpers/repository_helper.rb b/lib/splitclient-rb/helpers/repository_helper.rb index c4cea4b7..11f42416 100644 --- a/lib/splitclient-rb/helpers/repository_helper.rb +++ b/lib/splitclient-rb/helpers/repository_helper.rb @@ -15,7 +15,9 @@ def self.update_feature_flag_repository(feature_flag_repository, feature_flags, unless feature_flag.key?(:impressionsDisabled) feature_flag[:impressionsDisabled] = false - config.logger.debug("feature flag (#{feature_flag[:name]}) does not have `impressionsDisabled` field, setting it to `false`") if config.debug_enabled + if config.debug_enabled + config.logger.debug("feature flag (#{feature_flag[:name]}) does not have impressionsDisabled field, setting it to false") + end end config.logger.debug("storing feature flag (#{feature_flag[:name]})") if config.debug_enabled From aa20a81fbcb1b8d8943d64c64e275711435a90a2 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Tue, 7 Jan 2025 11:12:12 -0800 Subject: [PATCH 06/18] updated client and factory classes --- lib/splitclient-rb/clients/split_client.rb | 18 +-- lib/splitclient-rb/split_factory.rb | 15 +- spec/splitclient/split_client_spec.rb | 121 ++++++++++++++++ spec/test_data/splits/imp-toggle.json | 155 +++++++++++++++++++++ 4 files changed, 287 insertions(+), 22 deletions(-) create mode 100644 spec/test_data/splits/imp-toggle.json diff --git a/lib/splitclient-rb/clients/split_client.rb b/lib/splitclient-rb/clients/split_client.rb index 2b2eed6c..db33a9f9 100644 --- a/lib/splitclient-rb/clients/split_client.rb +++ b/lib/splitclient-rb/clients/split_client.rb @@ -332,14 +332,14 @@ def treatment(key, feature_flag_name, attributes = {}, split_data = nil, store_i end feature_flag = @splits_repository.get_split(feature_flag_name) - treatments, impressions = evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_key, attributes, calling_method, multiple) + treatments, impressions_decorator = evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_key, attributes, calling_method, multiple) - @impressions_manager.track(impressions) unless impressions.nil? + @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) - impressions = [] + impressions_decorator = [] begin start = Time.now if feature_flag.nil? && ready? @@ -358,20 +358,20 @@ def evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_ end record_latency(calling_method, start) - impression = @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, treatment_data, { attributes: attributes, time: nil }) - impressions << impression unless impression.nil? + 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] } + impressions_decorator << impression_decorator unless impression_decorator.nil? rescue StandardError => e @config.log_found_exception(__method__.to_s, e) record_exception(calling_method) - impression = @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, control_treatment, { attributes: attributes, time: nil }) - impressions << impression unless impression.nil? + impression_decorator = { impression: @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, control_treatment, { 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 + return parsed_treatment(control_treatment.merge({ label: Engine::Models::Label::EXCEPTION }), multiple), impressions_decorator end - return parsed_treatment(treatment_data, multiple), impressions + return parsed_treatment(treatment_data, multiple), impressions_decorator end def control_treatment diff --git a/lib/splitclient-rb/split_factory.rb b/lib/splitclient-rb/split_factory.rb index 10b9b46e..442298e7 100644 --- a/lib/splitclient-rb/split_factory.rb +++ b/lib/splitclient-rb/split_factory.rb @@ -230,11 +230,6 @@ def build_telemetry_synchronizer(flag_sets, flag_sets_invalid) end def build_unique_keys_tracker - if @config.impressions_mode != :none - @unique_keys_tracker = Engine::Impressions::NoopUniqueKeysTracker.new - return - end - bf = Cache::Filter::BloomFilter.new(30_000_000) filter_adapter = Cache::Filter::FilterAdapter.new(@config, bf) cache = Concurrent::Hash.new @@ -242,8 +237,7 @@ def build_unique_keys_tracker end def build_impressions_observer - if (@config.cache_adapter == :redis && @config.impressions_mode != :optimized) || - (@config.cache_adapter == :memory && @config.impressions_mode == :none) + if (@config.cache_adapter == :redis && @config.impressions_mode != :optimized) @impression_observer = Observers::NoopImpressionObserver.new else @impression_observer = Observers::ImpressionObserver.new @@ -251,12 +245,7 @@ def build_impressions_observer end def build_impression_counter - case @config.impressions_mode - when :debug - @impression_counter = Engine::Common::NoopImpressionCounter.new - else - @impression_counter = Engine::Common::ImpressionCounter.new - end + @impression_counter = Engine::Common::ImpressionCounter.new end def build_impressions_sender_adapter diff --git a/spec/splitclient/split_client_spec.rb b/spec/splitclient/split_client_spec.rb index 55b4447d..d8d7123d 100644 --- a/spec/splitclient/split_client_spec.rb +++ b/spec/splitclient/split_client_spec.rb @@ -92,6 +92,127 @@ end end +context 'impressions toggle' do + it 'optimized mode' do + config = SplitIoClient::SplitConfig.new(cache_adapter: :memory, impressions_mode: :optimized) + segments_repository = SplitIoClient::Cache::Repositories::SegmentsRepository.new(config) + flag_sets_repository = SplitIoClient::Cache::Repositories::MemoryFlagSetsRepository.new([]) + flag_set_filter = SplitIoClient::Cache::Filter::FlagSetsFilter.new([]) + splits_repository = SplitIoClient::Cache::Repositories::SplitsRepository.new(config, flag_sets_repository, flag_set_filter) + impressions_repository = SplitIoClient::Cache::Repositories::ImpressionsRepository.new(config) + runtime_producer = SplitIoClient::Telemetry::RuntimeProducer.new(config) + events_repository = SplitIoClient::Cache::Repositories::EventsRepository.new(config, 'sdk_key', runtime_producer) + impressions_counter = SplitIoClient::Engine::Common::ImpressionCounter.new + filter_adapter = SplitIoClient::Cache::Filter::FilterAdapter.new(config, SplitIoClient::Cache::Filter::BloomFilter.new(1_000)) + unique_keys_tracker = SplitIoClient::Engine::Impressions::UniqueKeysTracker.new(config, filter_adapter, nil, Concurrent::Hash.new) + impression_manager = SplitIoClient::Engine::Common::ImpressionManager.new(config, impressions_repository, impressions_counter, runtime_producer, SplitIoClient::Observers::ImpressionObserver.new, unique_keys_tracker) + evaluation_producer = SplitIoClient::Telemetry::EvaluationProducer.new(config) + evaluator = SplitIoClient::Engine::Parser::Evaluator.new(segments_repository, splits_repository, config) + split_client = SplitIoClient::SplitClient.new('sdk_key', {:splits => splits_repository, :segments => segments_repository, :impressions => impressions_repository, :events => events_repository}, nil, config, impression_manager, evaluation_producer, evaluator, SplitIoClient::Validators.new(config)) + + splits = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/imp-toggle.json')) + splits_repository.update([JSON.parse(splits,:symbolize_names => true)[:splits][0]], [], -1) + splits_repository.update([JSON.parse(splits,:symbolize_names => true)[:splits][1]], [], -1) + splits_repository.update([JSON.parse(splits,:symbolize_names => true)[:splits][2]], [], -1) + + expect(split_client.get_treatment('key1', 'with_track_disabled')).to eq('off') + expect(split_client.get_treatment('key2', 'with_track_enabled')).to eq('off') + expect(split_client.get_treatment('key3', 'without_track')).to eq('off') + + imps = impressions_repository.batch + expect(imps.length()).to eq(2) + expect(imps[0][:i][:f]).to eq('with_track_enabled') + expect(imps[1][:i][:f]).to eq('without_track') + + unique_keys = unique_keys_tracker.instance_variable_get(:@cache) + expect(unique_keys.key?('with_track_disabled')).to eq(true) + expect(unique_keys.length).to eq(1) + imp_count = impressions_counter.pop_all + expect(imp_count.keys()[0].include? ('with_track_disabled')).to eq(true) + expect(imp_count.length).to eq(1) + end + + it 'debug mode' do + config = SplitIoClient::SplitConfig.new(cache_adapter: :memory, impressions_mode: :debug) + segments_repository = SplitIoClient::Cache::Repositories::SegmentsRepository.new(config) + flag_sets_repository = SplitIoClient::Cache::Repositories::MemoryFlagSetsRepository.new([]) + flag_set_filter = SplitIoClient::Cache::Filter::FlagSetsFilter.new([]) + splits_repository = SplitIoClient::Cache::Repositories::SplitsRepository.new(config, flag_sets_repository, flag_set_filter) + impressions_repository = SplitIoClient::Cache::Repositories::ImpressionsRepository.new(config) + runtime_producer = SplitIoClient::Telemetry::RuntimeProducer.new(config) + events_repository = SplitIoClient::Cache::Repositories::EventsRepository.new(config, 'sdk_key', runtime_producer) + impressions_counter = SplitIoClient::Engine::Common::ImpressionCounter.new + filter_adapter = SplitIoClient::Cache::Filter::FilterAdapter.new(config, SplitIoClient::Cache::Filter::BloomFilter.new(1_000)) + unique_keys_tracker = SplitIoClient::Engine::Impressions::UniqueKeysTracker.new(config, filter_adapter, nil, Concurrent::Hash.new) + impression_manager = SplitIoClient::Engine::Common::ImpressionManager.new(config, impressions_repository, impressions_counter, runtime_producer, SplitIoClient::Observers::ImpressionObserver.new, unique_keys_tracker) + evaluation_producer = SplitIoClient::Telemetry::EvaluationProducer.new(config) + evaluator = SplitIoClient::Engine::Parser::Evaluator.new(segments_repository, splits_repository, config) + split_client = SplitIoClient::SplitClient.new('sdk_key', {:splits => splits_repository, :segments => segments_repository, :impressions => impressions_repository, :events => events_repository}, nil, config, impression_manager, evaluation_producer, evaluator, SplitIoClient::Validators.new(config)) + + splits = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/imp-toggle.json')) + splits_repository.update([JSON.parse(splits,:symbolize_names => true)[:splits][0]], [], -1) + splits_repository.update([JSON.parse(splits,:symbolize_names => true)[:splits][1]], [], -1) + splits_repository.update([JSON.parse(splits,:symbolize_names => true)[:splits][2]], [], -1) + + expect(split_client.get_treatment('key1', 'with_track_disabled')).to eq('off') + expect(split_client.get_treatment('key2', 'with_track_enabled')).to eq('off') + expect(split_client.get_treatment('key3', 'without_track')).to eq('off') + + imps = impressions_repository.batch + expect(imps.length()).to eq(2) + expect(imps[0][:i][:f]).to eq('with_track_enabled') + expect(imps[1][:i][:f]).to eq('without_track') + + unique_keys = unique_keys_tracker.instance_variable_get(:@cache) + expect(unique_keys.key?('with_track_disabled')).to eq(true) + expect(unique_keys.length).to eq(1) + imp_count = impressions_counter.pop_all + expect(imp_count.keys()[0].include? ('with_track_disabled')).to eq(true) + expect(imp_count.length).to eq(1) + end + + it 'none mode' do + config = SplitIoClient::SplitConfig.new(cache_adapter: :memory, impressions_mode: :none) + segments_repository = SplitIoClient::Cache::Repositories::SegmentsRepository.new(config) + flag_sets_repository = SplitIoClient::Cache::Repositories::MemoryFlagSetsRepository.new([]) + flag_set_filter = SplitIoClient::Cache::Filter::FlagSetsFilter.new([]) + splits_repository = SplitIoClient::Cache::Repositories::SplitsRepository.new(config, flag_sets_repository, flag_set_filter) + impressions_repository = SplitIoClient::Cache::Repositories::ImpressionsRepository.new(config) + runtime_producer = SplitIoClient::Telemetry::RuntimeProducer.new(config) + events_repository = SplitIoClient::Cache::Repositories::EventsRepository.new(config, 'sdk_key', runtime_producer) + impressions_counter = SplitIoClient::Engine::Common::ImpressionCounter.new + filter_adapter = SplitIoClient::Cache::Filter::FilterAdapter.new(config, SplitIoClient::Cache::Filter::BloomFilter.new(1_000)) + unique_keys_tracker = SplitIoClient::Engine::Impressions::UniqueKeysTracker.new(config, filter_adapter, nil, Concurrent::Hash.new) + impression_manager = SplitIoClient::Engine::Common::ImpressionManager.new(config, impressions_repository, impressions_counter, runtime_producer, SplitIoClient::Observers::ImpressionObserver.new, unique_keys_tracker) + evaluation_producer = SplitIoClient::Telemetry::EvaluationProducer.new(config) + evaluator = SplitIoClient::Engine::Parser::Evaluator.new(segments_repository, splits_repository, config) + split_client = SplitIoClient::SplitClient.new('sdk_key', {:splits => splits_repository, :segments => segments_repository, :impressions => impressions_repository, :events => events_repository}, nil, config, impression_manager, evaluation_producer, evaluator, SplitIoClient::Validators.new(config)) + + splits = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/imp-toggle.json')) + splits_repository.update([JSON.parse(splits,:symbolize_names => true)[:splits][0]], [], -1) + splits_repository.update([JSON.parse(splits,:symbolize_names => true)[:splits][1]], [], -1) + splits_repository.update([JSON.parse(splits,:symbolize_names => true)[:splits][2]], [], -1) + + expect(split_client.get_treatment('key1', 'with_track_disabled')).to eq('off') + expect(split_client.get_treatment('key2', 'with_track_enabled')).to eq('off') + expect(split_client.get_treatment('key3', 'without_track')).to eq('off') + + imps = impressions_repository.batch + expect(imps.length()).to eq(0) + + unique_keys = unique_keys_tracker.instance_variable_get(:@cache) + expect(unique_keys.key?('with_track_disabled')).to eq(true) + expect(unique_keys.key?('with_track_enabled')).to eq(true) + expect(unique_keys.key?('without_track')).to eq(true) + expect(unique_keys.length).to eq(3) + imp_count = impressions_counter.pop_all + expect(imp_count.keys()[0].include? ('with_track_disabled')).to eq(true) + expect(imp_count.keys()[1].include? ('with_track_enabled')).to eq(true) + expect(imp_count.keys()[2].include? ('without_track')).to eq(true) + expect(imp_count.length).to eq(3) + end +end + def mock_segment_changes(segment_name, segment_json, since) stub_request(:get, "https://sdk.split.io/api/segmentChanges/#{segment_name}?since=#{since}") .to_return(status: 200, body: segment_json) diff --git a/spec/test_data/splits/imp-toggle.json b/spec/test_data/splits/imp-toggle.json new file mode 100644 index 00000000..77d7a4e9 --- /dev/null +++ b/spec/test_data/splits/imp-toggle.json @@ -0,0 +1,155 @@ +{ + "splits": [ + { + "trafficTypeName": "user", + "name": "with_track_disabled", + "impressionsDisabled": true, + "trafficAllocation": 100, + "trafficAllocationSeed": 1068038034, + "seed": -1053389887, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1675259356568, + "algo": 2, + "configurations": {}, + "conditions": [ + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 0 + }, + { + "treatment": "off", + "size": 100 + } + ], + "label": "default rule" + } + ] + }, + { + "trafficTypeName": "user", + "name": "with_track_enabled", + "impressionsDisabled": false, + "trafficAllocation": 100, + "trafficAllocationSeed": 1068038034, + "seed": -1053389887, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1675259356568, + "algo": 2, + "configurations": {}, + "conditions": [ + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 0 + }, + { + "treatment": "off", + "size": 100 + } + ], + "label": "default rule" + } + ] + }, + { + "trafficTypeName": "user", + "name": "without_track", + "trafficAllocation": 100, + "trafficAllocationSeed": 1068038034, + "seed": -1053389887, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1675259356568, + "algo": 2, + "configurations": {}, + "conditions": [ + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 0 + }, + { + "treatment": "off", + "size": 100 + } + ], + "label": "default rule" + } + ] + } + ], + "since": -1, + "till": 1675259356568 +} From bc499a6ecb18069ddd821ee707097ce1d297bd1c Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Thu, 9 Jan 2025 16:07:51 -0800 Subject: [PATCH 07/18] updated tests and fixes --- lib/splitclient-rb/clients/split_client.rb | 34 +++++++++------- .../engine/common/impressions_manager.rb | 8 ++-- .../impressions_repository_spec.rb | 32 +++++++-------- .../senders/impressions_formatter_spec.rb | 6 +-- spec/cache/senders/impressions_sender_spec.rb | 4 +- .../senders/localhost_repo_cleaner_spec.rb | 5 ++- spec/engine_spec.rb | 19 ++++++--- spec/integrations/in_memory_client_spec.rb | 39 ++++++++++--------- spec/test_data/integrations/splits.json | 30 ++++++++++++++ spec/test_data/integrations/splits_push.json | 4 ++ spec/test_data/integrations/splits_push3.json | 1 + 11 files changed, 117 insertions(+), 65 deletions(-) diff --git a/lib/splitclient-rb/clients/split_client.rb b/lib/splitclient-rb/clients/split_client.rb index db33a9f9..ac345eb8 100644 --- a/lib/splitclient-rb/clients/split_client.rb +++ b/lib/splitclient-rb/clients/split_client.rb @@ -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 @@ -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 @@ -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 @@ -345,37 +343,45 @@ 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] } + if !feature_flag.nil? + impressions_disabled = feature_flag[:impressionsDisabled] + else + impressions_disabled = false + end + + 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) - + puts e.to_s 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 } + puts "split-client exception: #{treatment_data}" + 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 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 diff --git a/lib/splitclient-rb/engine/common/impressions_manager.rb b/lib/splitclient-rb/engine/common/impressions_manager.rb index 0fee4033..81873347 100644 --- a/lib/splitclient-rb/engine/common/impressions_manager.rb +++ b/lib/splitclient-rb/engine/common/impressions_manager.rb @@ -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]) @@ -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 @@ -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 diff --git a/spec/cache/repositories/impressions_repository_spec.rb b/spec/cache/repositories/impressions_repository_spec.rb index 7a4e0c3a..c422d34c 100644 --- a/spec/cache/repositories/impressions_repository_spec.rb +++ b/spec/cache/repositories/impressions_repository_spec.rb @@ -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) @@ -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) @@ -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 @@ -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 @@ -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) @@ -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 @@ -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([]) @@ -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| @@ -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| diff --git a/spec/cache/senders/impressions_formatter_spec.rb b/spec/cache/senders/impressions_formatter_spec.rb index 621ffdd7..e55228ff 100644 --- a/spec/cache/senders/impressions_formatter_spec.rb +++ b/spec/cache/senders/impressions_formatter_spec.rb @@ -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 @@ -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( diff --git a/spec/cache/senders/impressions_sender_spec.rb b/spec/cache/senders/impressions_sender_spec.rb index 9976a1de..96da8aa9 100644 --- a/spec/cache/senders/impressions_sender_spec.rb +++ b/spec/cache/senders/impressions_sender_spec.rb @@ -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 diff --git a/spec/cache/senders/localhost_repo_cleaner_spec.rb b/spec/cache/senders/localhost_repo_cleaner_spec.rb index d03d8220..ee0e21ef 100644 --- a/spec/cache/senders/localhost_repo_cleaner_spec.rb +++ b/spec/cache/senders/localhost_repo_cleaner_spec.rb @@ -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) diff --git a/spec/engine_spec.rb b/spec/engine_spec.rb index 8d09ae25..3631da63 100644 --- a/spec/engine_spec.rb +++ b/spec/engine_spec.rb @@ -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')) @@ -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 @@ -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 @@ -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 @@ -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) diff --git a/spec/integrations/in_memory_client_spec.rb b/spec/integrations/in_memory_client_spec.rb index 1120b57a..8de25df7 100644 --- a/spec/integrations/in_memory_client_spec.rb +++ b/spec/integrations/in_memory_client_spec.rb @@ -45,7 +45,27 @@ # sleep 1 end + # after(:each) do + # client.destroy + # sleep 0.5 + #end + context '#get_treatment' do + it 'returns CONTROL when server return 500' do + # stub_request(:get, "https://sdk.split.io/api/splitChanges?s=1.1&since=1506703262916").to_return(status: 200, body: 'ok') + mock_split_changes_error + expect(client.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'control' + + sleep 0.5 + impressions = custom_impression_listener.queue + expect(impressions.size).to eq 1 + expect(impressions[0][:matching_key]).to eq('nico_test') + expect(impressions[0][:split_name]).to eq('FACUNDO_TEST') + expect(impressions[0][:treatment][:treatment]).to eq('control') + expect(impressions[0][:treatment][:label]).to eq('not ready') + expect(impressions[0][:treatment][:change_number]).to eq(nil) + end + it 'returns treatments with FACUNDO_TEST feature and check impressions' do stub_request(:get, "https://sdk.split.io/api/splitChanges?s=1.1&since=1506703262916").to_return(status: 200, body: 'ok') client.block_until_ready @@ -130,23 +150,6 @@ expect(impressions.size).to eq 0 end - it 'returns CONTROL when server return 500' do - stub_request(:get, "https://sdk.split.io/api/splitChanges?s=1.1&since=1506703262916").to_return(status: 200, body: 'ok') - mock_split_changes_error - - expect(client.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'control' - - sleep 0.5 - impressions = custom_impression_listener.queue - - expect(impressions.size).to eq 1 - expect(impressions[0][:matching_key]).to eq('nico_test') - expect(impressions[0][:split_name]).to eq('FACUNDO_TEST') - expect(impressions[0][:treatment][:treatment]).to eq('control') - expect(impressions[0][:treatment][:label]).to eq('not ready') - expect(impressions[0][:treatment][:change_number]).to eq(nil) - end - it 'with multiple factories returns on' do # stub_request(:get, "https://sdk.split.io/api/splitChanges?s=1.1&since=1506703262916").to_return(status: 200, body: 'ok') local_log = StringIO.new @@ -450,7 +453,7 @@ it 'returns CONTROL when server return 500' do mock_split_changes_error - + sleep 1 result = client.get_treatments('nico_test', %w[FACUNDO_TEST random_treatment]) expect(result[:FACUNDO_TEST]).to eq 'control' diff --git a/spec/test_data/integrations/splits.json b/spec/test_data/integrations/splits.json index dd94bd8f..4361b6bb 100644 --- a/spec/test_data/integrations/splits.json +++ b/spec/test_data/integrations/splits.json @@ -1,6 +1,7 @@ { "splits": [ { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "FACUNDO_TEST", "trafficAllocation": 59, @@ -120,6 +121,7 @@ "sets": ["set_3"] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "testing", "trafficAllocation": 100, @@ -233,6 +235,7 @@ "sets": ["set_2"] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "testing222", "trafficAllocation": 100, @@ -286,6 +289,7 @@ "sets": ["set_1", "set_2"] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "a_new_split_2", "trafficAllocation": 99, @@ -551,6 +555,7 @@ "sets": [] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "test_string_without_attr", "trafficAllocation": 100, @@ -620,6 +625,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "Test", "trafficAllocation": 100, @@ -745,6 +751,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "Test_Save_1", "trafficAllocation": 100, @@ -891,6 +898,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "TEST", "trafficAllocation": 100, @@ -939,6 +947,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "benchmark_jw_1", "trafficAllocation": 100, @@ -1025,6 +1034,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "nico_tests", "trafficAllocation": 100, @@ -1070,6 +1080,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "testo2222", "trafficAllocation": 100, @@ -1282,6 +1293,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "Tagging", "trafficAllocation": 100, @@ -1330,6 +1342,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "Welcome_Page_UI", "trafficAllocation": 100, @@ -1381,6 +1394,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "test", "name": "pato_test_3", "trafficAllocation": 100, @@ -1429,6 +1443,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "testo23", "trafficAllocation": 100, @@ -1477,6 +1492,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "testo909090", "trafficAllocation": 100, @@ -1643,6 +1659,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "testo22", "trafficAllocation": 100, @@ -1691,6 +1708,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "test-net", "trafficAllocation": 100, @@ -1739,6 +1757,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "test_dep_2", "trafficAllocation": 100, @@ -1831,6 +1850,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "Definition_As_Of_Clickable_UI", "trafficAllocation": 100, @@ -1911,6 +1931,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "Identify_UI", "trafficAllocation": 100, @@ -1959,6 +1980,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "test_definition_as_of", "trafficAllocation": 100, @@ -2007,6 +2029,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "Test-jw-go", "trafficAllocation": 100, @@ -2085,6 +2108,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "benchmark_jw_7", "trafficAllocation": 100, @@ -2133,6 +2157,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "benchmark_jw_6", "trafficAllocation": 100, @@ -2181,6 +2206,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "benchmark_jw_5", "trafficAllocation": 100, @@ -2229,6 +2255,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "benchmark_jw_4", "trafficAllocation": 100, @@ -2277,6 +2304,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "benchmark_jw_3", "trafficAllocation": 100, @@ -2325,6 +2353,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "benchmark_jw_2", "trafficAllocation": 100, @@ -2373,6 +2402,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "account", "name": "MAURO_TEST", "trafficAllocation": 59, diff --git a/spec/test_data/integrations/splits_push.json b/spec/test_data/integrations/splits_push.json index a6f08de3..833a41a9 100644 --- a/spec/test_data/integrations/splits_push.json +++ b/spec/test_data/integrations/splits_push.json @@ -1,6 +1,7 @@ { "splits": [ { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "push_test", "trafficAllocation": 44, @@ -86,6 +87,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "tinchotest", "trafficAllocation": 24, @@ -135,6 +137,7 @@ ] }, { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "test_split", "trafficAllocation": 100, @@ -184,6 +187,7 @@ ] }, { + "impressionsDisabled": false, "name":"feature_segment", "trafficTypeId":"u", "trafficTypeName":"User", diff --git a/spec/test_data/integrations/splits_push3.json b/spec/test_data/integrations/splits_push3.json index a0c43a50..8984c5ff 100644 --- a/spec/test_data/integrations/splits_push3.json +++ b/spec/test_data/integrations/splits_push3.json @@ -1,6 +1,7 @@ { "splits": [ { + "impressionsDisabled": false, "trafficTypeName": "user", "name": "push_test", "trafficAllocation": 44, From 2530b2a4a86410e7795f795b6d08f4819feef356 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Thu, 9 Jan 2025 16:13:50 -0800 Subject: [PATCH 08/18] polish --- lib/splitclient-rb/clients/split_client.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/splitclient-rb/clients/split_client.rb b/lib/splitclient-rb/clients/split_client.rb index ac345eb8..7e38fe2e 100644 --- a/lib/splitclient-rb/clients/split_client.rb +++ b/lib/splitclient-rb/clients/split_client.rb @@ -358,12 +358,6 @@ def evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_ end record_latency(calling_method, start) - if !feature_flag.nil? - impressions_disabled = feature_flag[:impressionsDisabled] - else - impressions_disabled = false - end - 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 From 16643eac82da364ec2cf4b63878a58940472fd2e Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Fri, 10 Jan 2025 08:30:51 -0800 Subject: [PATCH 09/18] polish --- lib/splitclient-rb/clients/split_client.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/splitclient-rb/clients/split_client.rb b/lib/splitclient-rb/clients/split_client.rb index 7e38fe2e..1e12271a 100644 --- a/lib/splitclient-rb/clients/split_client.rb +++ b/lib/splitclient-rb/clients/split_client.rb @@ -362,9 +362,7 @@ def evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_ impressions_decorator << impression_decorator unless impression_decorator.nil? rescue StandardError => e @config.log_found_exception(__method__.to_s, e) - puts e.to_s record_exception(calling_method) - puts "split-client exception: #{treatment_data}" 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? From 4b99fc9c2537c61a9c2950838f132c2250ec6040 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Fri, 10 Jan 2025 08:35:28 -0800 Subject: [PATCH 10/18] polish --- lib/splitclient-rb/engine/common/impressions_manager.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/splitclient-rb/engine/common/impressions_manager.rb b/lib/splitclient-rb/engine/common/impressions_manager.rb index 3eda5d9a..adb71c6f 100644 --- a/lib/splitclient-rb/engine/common/impressions_manager.rb +++ b/lib/splitclient-rb/engine/common/impressions_manager.rb @@ -45,6 +45,7 @@ def track(impressions_decorator) stats = { dropped: 0, queued: 0, dedupe: 0 } begin next if @config.impressions_mode == :none || impression_decorator[:disabled] + if @config.impressions_mode == :debug track_debug_mode([impression_decorator[:impression]], stats) else From 21152526b0ca566c91679c74bce607c4551de92f Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Fri, 10 Jan 2025 09:47:32 -0800 Subject: [PATCH 11/18] fixed test --- spec/engine_spec.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/engine_spec.rb b/spec/engine_spec.rb index 3631da63..b9999044 100644 --- a/spec/engine_spec.rb +++ b/spec/engine_spec.rb @@ -56,10 +56,6 @@ 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')) From 7f33c4678f595e0f7c5f906e57fbc7dfe8c473c9 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Fri, 10 Jan 2025 09:58:15 -0800 Subject: [PATCH 12/18] polish --- lib/splitclient-rb/clients/split_client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/splitclient-rb/clients/split_client.rb b/lib/splitclient-rb/clients/split_client.rb index 1e12271a..b81fbfb2 100644 --- a/lib/splitclient-rb/clients/split_client.rb +++ b/lib/splitclient-rb/clients/split_client.rb @@ -358,7 +358,7 @@ def evaluate_treatment(feature_flag, feature_flag_name, bucketing_key, matching_ end record_latency(calling_method, start) - 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 } + impression_decorator = { :impression => @impressions_manager.build_impression(matching_key, bucketing_key, feature_flag_name, treatment_data, impressions_disabled, { 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) From 8606c0f86301ba6ed94be77e2b034ad12d5764a7 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Fri, 10 Jan 2025 10:32:24 -0800 Subject: [PATCH 13/18] added integration test for imp toggle --- spec/integrations/in_memory_client_spec.rb | 111 +++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/spec/integrations/in_memory_client_spec.rb b/spec/integrations/in_memory_client_spec.rb index 8de25df7..ff231179 100644 --- a/spec/integrations/in_memory_client_spec.rb +++ b/spec/integrations/in_memory_client_spec.rb @@ -1199,7 +1199,118 @@ result = client1.get_treatments_with_config_by_flag_sets('nico_test', ['set_2', 'set_3']) expect(result[:FACUNDO_TEST]).to eq({:config=>"{\"color\":\"green\"}", :treatment=>"on"}) end + end + context 'impressions toggle' do + it 'optimized mode' do + splits_imp_toggle = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/imp-toggle.json')) + stub_request(:get, 'https://sdk.split.io/api/splitChanges?s=1.1&since=-1') + .to_return(status: 200, body: splits_imp_toggle) + factory_imp_toggle = + SplitIoClient::SplitFactory.new('test_api_key', + impressions_mode: :optimized, + features_refresh_rate: 9999, + telemetry_refresh_rate: 99999, + impressions_refresh_rate: 99999, + streaming_enabled: false) + + client_imp_toggle = factory_imp_toggle.client + client_imp_toggle.block_until_ready + + expect(client_imp_toggle.get_treatment('key1', 'with_track_disabled')).to eq('off') + expect(client_imp_toggle.get_treatment('key2', 'with_track_enabled')).to eq('off') + expect(client_imp_toggle.get_treatment('key3', 'without_track')).to eq('off') + + impressions_repository = client_imp_toggle.instance_variable_get(:@impressions_repository) + imps = impressions_repository.batch + expect(imps.length()).to eq(2) + expect(imps[0][:i][:f]).to eq('with_track_enabled') + expect(imps[1][:i][:f]).to eq('without_track') + + unique_keys_tracker = factory_imp_toggle.instance_variable_get(:@unique_keys_tracker) + unique_keys = unique_keys_tracker.instance_variable_get(:@cache) + expect(unique_keys.key?('with_track_disabled')).to eq(true) + expect(unique_keys.length).to eq(1) + impression_counter = factory_imp_toggle.instance_variable_get(:@impression_counter) + imp_count = impression_counter.pop_all + expect(imp_count.keys()[0].include? ('with_track_disabled')).to eq(true) + expect(imp_count.length).to eq(1) + client_imp_toggle.destroy + sleep 1 + end + it 'debug mode' do + splits_imp_toggle = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/imp-toggle.json')) + stub_request(:get, 'https://sdk.split.io/api/splitChanges?s=1.1&since=-1') + .to_return(status: 200, body: splits_imp_toggle) + factory_imp_toggle = + SplitIoClient::SplitFactory.new('test_api_key', + impressions_mode: :debug, + features_refresh_rate: 9999, + telemetry_refresh_rate: 99999, + impressions_refresh_rate: 99999, + streaming_enabled: false) + + client_imp_toggle = factory_imp_toggle.client + client_imp_toggle.block_until_ready + + expect(client_imp_toggle.get_treatment('key1', 'with_track_disabled')).to eq('off') + expect(client_imp_toggle.get_treatment('key2', 'with_track_enabled')).to eq('off') + expect(client_imp_toggle.get_treatment('key3', 'without_track')).to eq('off') + + impressions_repository = client_imp_toggle.instance_variable_get(:@impressions_repository) + imps = impressions_repository.batch + expect(imps.length()).to eq(2) + expect(imps[0][:i][:f]).to eq('with_track_enabled') + expect(imps[1][:i][:f]).to eq('without_track') + + unique_keys_tracker = factory_imp_toggle.instance_variable_get(:@unique_keys_tracker) + unique_keys = unique_keys_tracker.instance_variable_get(:@cache) + expect(unique_keys.key?('with_track_disabled')).to eq(true) + expect(unique_keys.length).to eq(1) + impression_counter = factory_imp_toggle.instance_variable_get(:@impression_counter) + imp_count = impression_counter.pop_all + expect(imp_count.keys()[0].include? ('with_track_disabled')).to eq(true) + expect(imp_count.length).to eq(1) + client_imp_toggle.destroy + sleep 1 + end + it 'debug mode' do + splits_imp_toggle = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/imp-toggle.json')) + stub_request(:get, 'https://sdk.split.io/api/splitChanges?s=1.1&since=-1') + .to_return(status: 200, body: splits_imp_toggle) + factory_imp_toggle = + SplitIoClient::SplitFactory.new('test_api_key', + impressions_mode: :none, + features_refresh_rate: 9999, + telemetry_refresh_rate: 99999, + impressions_refresh_rate: 99999, + streaming_enabled: false) + + client_imp_toggle = factory_imp_toggle.client + client_imp_toggle.block_until_ready + expect(client_imp_toggle.get_treatment('key1', 'with_track_disabled')).to eq('off') + expect(client_imp_toggle.get_treatment('key2', 'with_track_enabled')).to eq('off') + expect(client_imp_toggle.get_treatment('key3', 'without_track')).to eq('off') + + impressions_repository = client_imp_toggle.instance_variable_get(:@impressions_repository) + imps = impressions_repository.batch + expect(imps.length()).to eq(0) + + unique_keys_tracker = factory_imp_toggle.instance_variable_get(:@unique_keys_tracker) + unique_keys = unique_keys_tracker.instance_variable_get(:@cache) + expect(unique_keys.key?('with_track_disabled')).to eq(true) + expect(unique_keys.key?('with_track_enabled')).to eq(true) + expect(unique_keys.key?('without_track')).to eq(true) + expect(unique_keys.length).to eq(3) + impression_counter = factory_imp_toggle.instance_variable_get(:@impression_counter) + imp_count = impression_counter.pop_all + expect(imp_count.keys()[0].include? ('with_track_disabled')).to eq(true) + expect(imp_count.keys()[1].include? ('with_track_enabled')).to eq(true) + expect(imp_count.keys()[2].include? ('without_track')).to eq(true) + expect(imp_count.length).to eq(3) + client_imp_toggle.destroy + sleep 1 + end end end From f185466ce3131f90b2d1b2b13dd72f601b6d46b9 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Fri, 10 Jan 2025 11:10:02 -0800 Subject: [PATCH 14/18] fixed test --- spec/integrations/in_memory_client_spec.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/spec/integrations/in_memory_client_spec.rb b/spec/integrations/in_memory_client_spec.rb index ff231179..d70e6c76 100644 --- a/spec/integrations/in_memory_client_spec.rb +++ b/spec/integrations/in_memory_client_spec.rb @@ -1235,8 +1235,6 @@ imp_count = impression_counter.pop_all expect(imp_count.keys()[0].include? ('with_track_disabled')).to eq(true) expect(imp_count.length).to eq(1) - client_imp_toggle.destroy - sleep 1 end it 'debug mode' do splits_imp_toggle = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/imp-toggle.json')) @@ -1271,10 +1269,8 @@ imp_count = impression_counter.pop_all expect(imp_count.keys()[0].include? ('with_track_disabled')).to eq(true) expect(imp_count.length).to eq(1) - client_imp_toggle.destroy - sleep 1 end - it 'debug mode' do + it 'none mode' do splits_imp_toggle = File.read(File.join(SplitIoClient.root, 'spec/test_data/splits/imp-toggle.json')) stub_request(:get, 'https://sdk.split.io/api/splitChanges?s=1.1&since=-1') .to_return(status: 200, body: splits_imp_toggle) @@ -1308,8 +1304,6 @@ expect(imp_count.keys()[1].include? ('with_track_enabled')).to eq(true) expect(imp_count.keys()[2].include? ('without_track')).to eq(true) expect(imp_count.length).to eq(3) - client_imp_toggle.destroy - sleep 1 end end end From 234cf309496acca8dbe9b8c41a0c48c9ff2a7de0 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Mon, 13 Jan 2025 11:48:58 -0800 Subject: [PATCH 15/18] added impressions_disable field to manager --- lib/splitclient-rb/managers/split_manager.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/splitclient-rb/managers/split_manager.rb b/lib/splitclient-rb/managers/split_manager.rb index 41e6a7f0..9d401558 100644 --- a/lib/splitclient-rb/managers/split_manager.rb +++ b/lib/splitclient-rb/managers/split_manager.rb @@ -107,7 +107,8 @@ def build_split_view(name, split) change_number: split[:changeNumber], configs: split[:configurations] || {}, sets: split[:sets] || [], - default_treatment: split[:defaultTreatment] + default_treatment: split[:defaultTreatment], + impressions_disabled: split[:impressionsDisabled] } end From 8373a0ac74c28a29d21731bb268c7bff9b1b6e6c Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Mon, 13 Jan 2025 12:03:20 -0800 Subject: [PATCH 16/18] fix test --- spec/splitclient/manager_localhost_spec.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/spec/splitclient/manager_localhost_spec.rb b/spec/splitclient/manager_localhost_spec.rb index 08067a4b..d6a4db86 100644 --- a/spec/splitclient/manager_localhost_spec.rb +++ b/spec/splitclient/manager_localhost_spec.rb @@ -21,7 +21,8 @@ name: 'local_feature', traffic_type_name: nil, treatments: ['local_treatment'], - sets: [] } + sets: [], + impressions_disabled: nil } end let(:split_views) do @@ -32,7 +33,8 @@ name: 'local_feature', traffic_type_name: nil, treatments: ['local_treatment'], - sets: [] }, + sets: [], + impressions_disabled: nil }, { change_number: nil, configs: { local_treatment2: nil }, default_treatment: "control_treatment", @@ -40,7 +42,8 @@ name: 'local_feature2', traffic_type_name: nil, treatments: ['local_treatment2'], - sets: [] }] + sets: [], + impressions_disabled: nil }] end it 'validates the calling manager.splits returns the offline data' do @@ -70,7 +73,8 @@ name: 'multiple_keys_feature', traffic_type_name: nil, treatments: %w[off on], - sets: [] }, + sets: [], + impressions_disabled: nil }, { change_number: nil, configs: { on: '{"desc":"this applies only to ON and only for john_doe. The rest will receive OFF"}', @@ -81,7 +85,8 @@ name: 'single_key_feature', traffic_type_name: nil, treatments: %w[on off], - sets: [] }, + sets: [], + impressions_disabled: nil }, { change_number: nil, configs: { off: '{"desc":"this applies only to OFF treatment"}' @@ -91,7 +96,8 @@ name: 'no_keys_feature', traffic_type_name: nil, treatments: %w[off], - sets: [] }] + sets: [], + impressions_disabled: nil }] end it 'returns split_names' do From 22a89b19724ffd37d44751bab47fbdde74f0134b Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Tue, 14 Jan 2025 12:04:11 -0800 Subject: [PATCH 17/18] updated changes and version --- CHANGES.txt | 4 ++++ lib/splitclient-rb/version.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index a1b3e034..a4772a85 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,9 @@ CHANGES +8.5.0 (Jan xx, 2025) +- Fixed high cpu usage when unique keys are cleared every 24 hours. +- Added support for the new impressions tracking toggle available on feature flags, both respecting the setting and including the new field being returned on SplitView type objects. Read more in our docs. + 8.4.0 (May 3, 2024) - Fixed issue preventing Impressopns and Events posting if client.destroy is called before the post threads started - Added support for targeting rules based on semantic versions (https://semver.org/). diff --git a/lib/splitclient-rb/version.rb b/lib/splitclient-rb/version.rb index 1cd7e252..bc0e4bd5 100644 --- a/lib/splitclient-rb/version.rb +++ b/lib/splitclient-rb/version.rb @@ -1,3 +1,3 @@ module SplitIoClient - VERSION = '8.4.0' + VERSION = '8.5.0' end From 0e065e3585403b1dd752f9a90ea2f7a27eeff39f Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Fri, 17 Jan 2025 08:48:49 -0800 Subject: [PATCH 18/18] updated changes --- CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index a4772a85..2ad00a32 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,6 @@ CHANGES -8.5.0 (Jan xx, 2025) +8.5.0 (Jan 17, 2025) - Fixed high cpu usage when unique keys are cleared every 24 hours. - Added support for the new impressions tracking toggle available on feature flags, both respecting the setting and including the new field being returned on SplitView type objects. Read more in our docs.