From ddac87a44ce4d15389948dfb32e001a78a99a742 Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Tue, 19 Apr 2022 16:46:42 -0500 Subject: [PATCH 1/6] using hset instead of rpush --- .../telemetry/redis/redis_init_producer.rb | 10 +++++----- spec/integrations/push_client_spec.rb | 2 ++ spec/telemetry/synchronizer_spec.rb | 13 ++++++------- spec/telemetry/telemetry_init_spec.rb | 15 ++++++++------- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb b/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb index 841a3c48..cbc91d34 100644 --- a/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb +++ b/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb @@ -11,12 +11,12 @@ def initialize(config) def record_config(config_data) return if config_data.nil? - data = { m: { i: @config.machine_ip, n: @config.machine_name, s: "#{@config.language}-#{@config.version}" }, - t: { oM: config_data.om, st: config_data.st, aF: config_data.af, rF: config_data.rf, t: config_data.t } } + data = { t: { oM: config_data.om, st: config_data.st, aF: config_data.af, rF: config_data.rf, t: config_data.t } } + field = "#{@config.language}-#{@config.version}/#{@config.machine_name}/#{@config.machine_ip}" - @adapter.add_to_queue(config_key, data.to_json) - rescue StandardError => error - @config.log_found_exception(__method__.to_s, error) + @adapter.add_to_map(config_key, field, data.to_json) + rescue StandardError => e + @config.log_found_exception(__method__.to_s, e) end def record_bur_timeout diff --git a/spec/integrations/push_client_spec.rb b/spec/integrations/push_client_spec.rb index 9cfeb197..13f657a1 100644 --- a/spec/integrations/push_client_spec.rb +++ b/spec/integrations/push_client_spec.rb @@ -171,6 +171,7 @@ mock_splits_request(splits, -1) mock_splits_request(splits2, 1_585_948_850_109) stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=-1&till=1470947453879').to_return(status: 200, body: '') stub_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=-1') .to_return({ status: 200, body: segment3 }, { status: 200, body: segment3 }, { status: 200, body: segment3_updated }) @@ -352,6 +353,7 @@ mock_splits_request(splits3, 1_585_948_850_110) mock_segment_changes('segment3', segment3, '-1') stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=1585948850111').to_return(status: 200, body: '') mock_server do |server| server.setup_response('/') do |_, res| diff --git a/spec/telemetry/synchronizer_spec.rb b/spec/telemetry/synchronizer_spec.rb index ce9542f5..03b44500 100644 --- a/spec/telemetry/synchronizer_spec.rb +++ b/spec/telemetry/synchronizer_spec.rb @@ -6,27 +6,26 @@ let(:log) { StringIO.new } context 'Redis' do - let(:config) { SplitIoClient::SplitConfig.new(logger: Logger.new(log), cache_adapter: :redis, mode: :consumer) } + let(:config) { SplitIoClient::SplitConfig.new(logger: Logger.new(log), cache_adapter: :redis, mode: :consumer, redis_namespace: 'synch-test') } let(:adapter) { config.telemetry_adapter } let(:init_producer) { SplitIoClient::Telemetry::InitProducer.new(config) } let(:synchronizer) { SplitIoClient::Telemetry::Synchronizer.new(config, nil, init_producer, nil, nil) } - let(:config_key) { 'SPLITIO.telemetry.config' } + let(:config_key) { 'synch-test.SPLITIO.telemetry.config' } it 'synchronize_config with data' do adapter.redis.del(config_key) synchronizer.synchronize_config(5, 1, ['tag-1']) - result = JSON.parse(adapter.redis.lrange(config_key, 0, -1)[0], symbolize_names: true) - - expect(result[:m][:i]).to eq(config.machine_ip) - expect(result[:m][:n]).to eq(config.machine_name) - expect(result[:m][:s]).to eq("#{config.language}-#{config.version}") + field = "#{config.language}-#{config.version}/#{config.machine_name}/#{config.machine_ip}" + result = JSON.parse(adapter.find_in_map(config_key, field), symbolize_names: true) expect(result[:t][:oM]).to eq('consumer') expect(result[:t][:st]).to eq('redis') expect(result[:t][:aF]).to eq(5) expect(result[:t][:rF]).to eq(1) expect(result[:t][:t]).to eq(%w[tag-1]) + + adapter.redis.del(config_key) end end diff --git a/spec/telemetry/telemetry_init_spec.rb b/spec/telemetry/telemetry_init_spec.rb index da9ccf6b..9bacc866 100644 --- a/spec/telemetry/telemetry_init_spec.rb +++ b/spec/telemetry/telemetry_init_spec.rb @@ -53,10 +53,10 @@ end context 'Redis' do - let(:config) { SplitIoClient::SplitConfig.new(logger: Logger.new(log), cache_adapter: :redis) } + let(:config) { SplitIoClient::SplitConfig.new(logger: Logger.new(log), cache_adapter: :redis, redis_namespace: 'telemetry-test') } let(:adapter) { config.telemetry_adapter } let(:init_producer) { SplitIoClient::Telemetry::InitProducer.new(config) } - let(:telemetry_config_key) { 'SPLITIO.telemetry.config' } + let(:telemetry_config_key) { 'telemetry-test.SPLITIO.telemetry.config' } it 'record config_init' do adapter.redis.del(telemetry_config_key) @@ -65,17 +65,16 @@ init_producer.record_config(config_init) - result = JSON.parse(adapter.redis.lrange(telemetry_config_key, 0, -1)[0], symbolize_names: true) - - expect(result[:m][:i]).to eq(config.machine_ip) - expect(result[:m][:n]).to eq(config.machine_name) - expect(result[:m][:s]).to eq("#{config.language}-#{config.version}") + field = "#{config.language}-#{config.version}/#{config.machine_name}/#{config.machine_ip}" + result = JSON.parse(adapter.find_in_map(telemetry_config_key, field), symbolize_names: true) expect(result[:t][:oM]).to eq('CONSUMER') expect(result[:t][:st]).to eq('REDIS') expect(result[:t][:aF]).to eq(1) expect(result[:t][:rF]).to eq(0) expect(result[:t][:t]).to eq(%w[t1 t2]) + + adapter.redis.del(telemetry_config_key) end it 'record config_init when data is nil' do @@ -86,6 +85,8 @@ result = adapter.redis.lrange(telemetry_config_key, 0, -1) expect(result.empty?).to be true + + adapter.redis.del(telemetry_config_key) end end end From 675b08c2f0d3551bb4130569f4b4c7f907129058 Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Wed, 20 Apr 2022 10:29:28 -0500 Subject: [PATCH 2/6] added expire time --- lib/splitclient-rb/telemetry/redis/redis_init_producer.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb b/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb index cbc91d34..250b99f7 100644 --- a/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb +++ b/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb @@ -3,6 +3,8 @@ module SplitIoClient module Telemetry class RedisInitProducer < InitProducer + EXPIRE_SECONDS = 3600 + def initialize(config) @config = config @adapter = config.telemetry_adapter @@ -14,7 +16,9 @@ def record_config(config_data) data = { t: { oM: config_data.om, st: config_data.st, aF: config_data.af, rF: config_data.rf, t: config_data.t } } field = "#{@config.language}-#{@config.version}/#{@config.machine_name}/#{@config.machine_ip}" - @adapter.add_to_map(config_key, field, data.to_json) + result = @adapter.add_to_map(config_key, field, data.to_json) + + @adapter.expire(config_key, EXPIRE_SECONDS) if result == 1 rescue StandardError => e @config.log_found_exception(__method__.to_s, e) end From 83dc0bc4b5ead1353dfbefcce35cc1c806bd6d5c Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Wed, 20 Apr 2022 10:33:40 -0500 Subject: [PATCH 3/6] Revert "added expire time" This reverts commit 675b08c2f0d3551bb4130569f4b4c7f907129058. --- lib/splitclient-rb/telemetry/redis/redis_init_producer.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb b/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb index 250b99f7..cbc91d34 100644 --- a/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb +++ b/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb @@ -3,8 +3,6 @@ module SplitIoClient module Telemetry class RedisInitProducer < InitProducer - EXPIRE_SECONDS = 3600 - def initialize(config) @config = config @adapter = config.telemetry_adapter @@ -16,9 +14,7 @@ def record_config(config_data) data = { t: { oM: config_data.om, st: config_data.st, aF: config_data.af, rF: config_data.rf, t: config_data.t } } field = "#{@config.language}-#{@config.version}/#{@config.machine_name}/#{@config.machine_ip}" - result = @adapter.add_to_map(config_key, field, data.to_json) - - @adapter.expire(config_key, EXPIRE_SECONDS) if result == 1 + @adapter.add_to_map(config_key, field, data.to_json) rescue StandardError => e @config.log_found_exception(__method__.to_s, e) end From f253f6ee36f85bba84d4fee7464dd8f5d284ce1a Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Tue, 19 Apr 2022 17:29:09 -0500 Subject: [PATCH 4/6] redis: force to debug mode. inMemory: accept only debug or optimized mode. --- lib/splitclient-rb/split_config.rb | 10 ++++++---- spec/engine/common/impression_manager_spec.rb | 3 ++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/splitclient-rb/split_config.rb b/lib/splitclient-rb/split_config.rb index 6c3573ca..22508cbd 100644 --- a/lib/splitclient-rb/split_config.rb +++ b/lib/splitclient-rb/split_config.rb @@ -51,7 +51,7 @@ def initialize(opts = {}) @segments_refresh_rate = opts[:segments_refresh_rate] || SplitConfig.default_segments_refresh_rate - @impressions_mode = init_impressions_mode(opts[:impressions_mode]) + @impressions_mode = init_impressions_mode(opts[:impressions_mode], opts[:cache_adapter]) @impressions_refresh_rate = SplitConfig.init_impressions_refresh_rate(@impressions_mode, opts[:impressions_refresh_rate], SplitConfig.default_impressions_refresh_rate) @impressions_queue_size = opts[:impressions_queue_size] || SplitConfig.default_impressions_queue_size @@ -314,14 +314,16 @@ def self.default_impressions_mode :optimized end - def init_impressions_mode(impressions_mode) + def init_impressions_mode(impressions_mode, adapter) impressions_mode ||= SplitConfig.default_impressions_mode + return :debug if adapter == :redis + case impressions_mode when :debug return :debug - when :none - return :none + # when :none // we not support :none impression mode yet. Defaulting to :optimized mode + # return :none else @logger.error('You passed an invalid impressions_mode, impressions_mode should be one of the following values: :debug or :optimized. Defaulting to :optimized mode') unless impressions_mode == :optimized return :optimized diff --git a/spec/engine/common/impression_manager_spec.rb b/spec/engine/common/impression_manager_spec.rb index 38346e71..7044183a 100644 --- a/spec/engine/common/impression_manager_spec.rb +++ b/spec/engine/common/impression_manager_spec.rb @@ -47,7 +47,8 @@ unique_keys_tracker) end - it 'build & track impression' do + # TODO: remove skip test when the sdk support :none mode. + xit 'build & track impression' do expected = { m: { s: version, i: ip, n: machine_name }, From 331cbbbc6b0eb64e1c19a918a5faaa68d4736a62 Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Wed, 20 Apr 2022 12:46:30 -0500 Subject: [PATCH 5/6] updated ci to run always --- .github/workflows/ci.yml | 8 +------- lib/splitclient-rb/version.rb | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 39d813a2..2a648c81 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,10 +1,4 @@ -on: - push: - branches: - - do-not-send-impressions - pull_request: - branches: - - do-not-send-impressions +on: [push, pull_request] jobs: test: diff --git a/lib/splitclient-rb/version.rb b/lib/splitclient-rb/version.rb index f7708069..5697e653 100644 --- a/lib/splitclient-rb/version.rb +++ b/lib/splitclient-rb/version.rb @@ -1,3 +1,3 @@ module SplitIoClient - VERSION = '7.3.5.pre.rc4' + VERSION = '7.3.5.pre.rc5' end From f8d705f8725030b1615662d98b0d30069bef6a22 Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Wed, 20 Apr 2022 16:00:00 -0500 Subject: [PATCH 6/6] pr feedback --- .../cache/senders/impressions_adapter/redis_sender.rb | 2 -- lib/splitclient-rb/telemetry/redis/redis_init_producer.rb | 2 +- spec/telemetry/synchronizer_spec.rb | 2 +- spec/telemetry/telemetry_init_spec.rb | 2 +- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/splitclient-rb/cache/senders/impressions_adapter/redis_sender.rb b/lib/splitclient-rb/cache/senders/impressions_adapter/redis_sender.rb index 3fe1b360..98872dd1 100644 --- a/lib/splitclient-rb/cache/senders/impressions_adapter/redis_sender.rb +++ b/lib/splitclient-rb/cache/senders/impressions_adapter/redis_sender.rb @@ -29,8 +29,6 @@ def record_impressions_count(impressions_count) impressions_count.each do |key, value| pipeline.hincrby(impressions_count_key, key, value) end - - @future = pipeline.hlen(impressions_count_key) end expire_impressions_count_key(impressions_count, result) diff --git a/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb b/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb index cbc91d34..050415a4 100644 --- a/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb +++ b/lib/splitclient-rb/telemetry/redis/redis_init_producer.rb @@ -30,7 +30,7 @@ def record_non_ready_usages private def config_key - "#{@config.redis_namespace}.telemetry.config" + "#{@config.redis_namespace}.telemetry.init" end end end diff --git a/spec/telemetry/synchronizer_spec.rb b/spec/telemetry/synchronizer_spec.rb index 03b44500..31c14281 100644 --- a/spec/telemetry/synchronizer_spec.rb +++ b/spec/telemetry/synchronizer_spec.rb @@ -10,7 +10,7 @@ let(:adapter) { config.telemetry_adapter } let(:init_producer) { SplitIoClient::Telemetry::InitProducer.new(config) } let(:synchronizer) { SplitIoClient::Telemetry::Synchronizer.new(config, nil, init_producer, nil, nil) } - let(:config_key) { 'synch-test.SPLITIO.telemetry.config' } + let(:config_key) { 'synch-test.SPLITIO.telemetry.init' } it 'synchronize_config with data' do adapter.redis.del(config_key) diff --git a/spec/telemetry/telemetry_init_spec.rb b/spec/telemetry/telemetry_init_spec.rb index 9bacc866..95181439 100644 --- a/spec/telemetry/telemetry_init_spec.rb +++ b/spec/telemetry/telemetry_init_spec.rb @@ -56,7 +56,7 @@ let(:config) { SplitIoClient::SplitConfig.new(logger: Logger.new(log), cache_adapter: :redis, redis_namespace: 'telemetry-test') } let(:adapter) { config.telemetry_adapter } let(:init_producer) { SplitIoClient::Telemetry::InitProducer.new(config) } - let(:telemetry_config_key) { 'telemetry-test.SPLITIO.telemetry.config' } + let(:telemetry_config_key) { 'telemetry-test.SPLITIO.telemetry.init' } it 'record config_init' do adapter.redis.del(telemetry_config_key)