diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 6fe8feba..1894ed18 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -50,10 +50,6 @@ module Optimizely class Project include Optimizely::Decide - # CMAB Constants - DEFAULT_CMAB_CACHE_TIMEOUT = (30 * 60 * 1000) - DEFAULT_CMAB_CACHE_SIZE = 1000 - attr_reader :notification_center # @api no-doc attr_reader :config_manager, :decision_service, :error_handler, :event_dispatcher, @@ -90,7 +86,8 @@ def initialize( event_processor: nil, default_decide_options: [], event_processor_options: {}, - settings: nil + settings: nil, + cmab_service: nil ) @logger = logger || NoOpLogger.new @error_handler = error_handler || NoOpErrorHandler.new @@ -137,18 +134,22 @@ def initialize( setup_odp!(@config_manager.sdk_key) - # Initialize CMAB components - @cmab_client = DefaultCmabClient.new( - nil, - CmabRetryConfig.new, - @logger - ) - @cmab_cache = LRUCache.new(DEFAULT_CMAB_CACHE_SIZE, DEFAULT_CMAB_CACHE_TIMEOUT) - @cmab_service = DefaultCmabService.new( - @cmab_cache, - @cmab_client, - @logger - ) + # Initialize CMAB components if cmab service is nil + if cmab_service.nil? + @cmab_client = DefaultCmabClient.new( + nil, + CmabRetryConfig.new, + @logger + ) + @cmab_cache = LRUCache.new(Optimizely::DefaultCmabCacheOptions::DEFAULT_CMAB_CACHE_SIZE, Optimizely::DefaultCmabCacheOptions::DEFAULT_CMAB_CACHE_TIMEOUT) + @cmab_service = DefaultCmabService.new( + @cmab_cache, + @cmab_client, + @logger + ) + else + @cmab_service = cmab_service + end @decision_service = DecisionService.new(@logger, @cmab_service, @user_profile_service) diff --git a/lib/optimizely/cmab/cmab_client.rb b/lib/optimizely/cmab/cmab_client.rb index f9a21cff..8b6b7fa1 100644 --- a/lib/optimizely/cmab/cmab_client.rb +++ b/lib/optimizely/cmab/cmab_client.rb @@ -20,7 +20,7 @@ module Optimizely # Default constants for CMAB requests - DEFAULT_MAX_RETRIES = 3 + DEFAULT_MAX_RETRIES = 1 DEFAULT_INITIAL_BACKOFF = 0.1 # in seconds (100 ms) DEFAULT_MAX_BACKOFF = 10 # in seconds DEFAULT_BACKOFF_MULTIPLIER = 2.0 diff --git a/lib/optimizely/cmab/cmab_service.rb b/lib/optimizely/cmab/cmab_service.rb index 7bd60c0f..e432d672 100644 --- a/lib/optimizely/cmab/cmab_service.rb +++ b/lib/optimizely/cmab/cmab_service.rb @@ -17,6 +17,7 @@ # require 'optimizely/odp/lru_cache' require 'optimizely/decide/optimizely_decide_option' +require 'optimizely/logger' require 'digest' require 'json' require 'securerandom' @@ -26,6 +27,12 @@ module Optimizely CmabDecision = Struct.new(:variation_id, :cmab_uuid, keyword_init: true) CmabCacheValue = Struct.new(:attributes_hash, :variation_id, :cmab_uuid, keyword_init: true) + class DefaultCmabCacheOptions + # CMAB Constants + DEFAULT_CMAB_CACHE_TIMEOUT = (30 * 60) # in seconds + DEFAULT_CMAB_CACHE_SIZE = 1000 + end + # Default CMAB service implementation class DefaultCmabService # Initializes a new instance of the CmabService. @@ -42,7 +49,7 @@ class DefaultCmabService def initialize(cmab_cache, cmab_client, logger = nil) @cmab_cache = cmab_cache @cmab_client = cmab_client - @logger = logger + @logger = logger || NoOpLogger.new @locks = Array.new(NUM_LOCK_STRIPES) { Mutex.new } end @@ -81,30 +88,65 @@ def get_decision_impl(project_config, user_context, rule_id, options) # @return [CmabDecision] The decision object containing variation_id and cmab_uuid. filtered_attributes = filter_attributes(project_config, user_context, rule_id) + reasons = [] + + if options&.include?(Decide::OptimizelyDecideOption::IGNORE_CMAB_CACHE) + reason = "Ignoring CMAB cache for user '#{user_context.user_id}' and rule '#{rule_id}'" + @logger.log(Logger::DEBUG, reason) + reasons << reason + cmab_decision = fetch_decision(rule_id, user_context.user_id, filtered_attributes) + return [cmab_decision, reasons] + end - return fetch_decision(rule_id, user_context.user_id, filtered_attributes) if options&.include?(Decide::OptimizelyDecideOption::IGNORE_CMAB_CACHE) - - @cmab_cache.reset if options&.include?(Decide::OptimizelyDecideOption::RESET_CMAB_CACHE) + if options&.include?(Decide::OptimizelyDecideOption::RESET_CMAB_CACHE) + reason = "Resetting CMAB cache for user '#{user_context.user_id}' and rule '#{rule_id}'" + @logger.log(Logger::DEBUG, reason) + reasons << reason + @cmab_cache.reset + end cache_key = get_cache_key(user_context.user_id, rule_id) - @cmab_cache.remove(cache_key) if options&.include?(Decide::OptimizelyDecideOption::INVALIDATE_USER_CMAB_CACHE) + if options&.include?(Decide::OptimizelyDecideOption::INVALIDATE_USER_CMAB_CACHE) + reason = "Invalidating CMAB cache for user '#{user_context.user_id}' and rule '#{rule_id}'" + @logger.log(Logger::DEBUG, reason) + reasons << reason + @cmab_cache.remove(cache_key) + end + cached_value = @cmab_cache.lookup(cache_key) attributes_hash = hash_attributes(filtered_attributes) if cached_value - return CmabDecision.new(variation_id: cached_value.variation_id, cmab_uuid: cached_value.cmab_uuid) if cached_value.attributes_hash == attributes_hash - - @cmab_cache.remove(cache_key) + if cached_value.attributes_hash == attributes_hash + reason = "CMAB cache hit for user '#{user_context.user_id}' and rule '#{rule_id}'" + @logger.log(Logger::DEBUG, reason) + reasons << reason + return [CmabDecision.new(variation_id: cached_value.variation_id, cmab_uuid: cached_value.cmab_uuid), reasons] + else + reason = "CMAB cache attributes mismatch for user '#{user_context.user_id}' and rule '#{rule_id}', fetching new decision." + @logger.log(Logger::DEBUG, reason) + reasons << reason + @cmab_cache.remove(cache_key) + end + else + reason = "CMAB cache miss for user '#{user_context.user_id}' and rule '#{rule_id}'" + @logger.log(Logger::DEBUG, reason) + reasons << reason end + cmab_decision = fetch_decision(rule_id, user_context.user_id, filtered_attributes) + reason = "CMAB decision is #{cmab_decision.to_h}" + @logger.log(Logger::DEBUG, reason) + reasons << reason + @cmab_cache.save(cache_key, CmabCacheValue.new( attributes_hash: attributes_hash, variation_id: cmab_decision.variation_id, cmab_uuid: cmab_decision.cmab_uuid )) - cmab_decision + [cmab_decision, reasons] end def fetch_decision(rule_id, user_id, attributes) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index afc21f67..f1bc92e2 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -649,9 +649,10 @@ def get_decision_for_cmab_experiment(project_config, experiment, user_context, b # User is in CMAB allocation, proceed to CMAB decision begin - cmab_decision = @cmab_service.get_decision( + cmab_decision, reasons = @cmab_service.get_decision( project_config, user_context, experiment['id'], decide_options ) + decide_reasons.push(*reasons) CmabDecisionResult.new(false, cmab_decision, decide_reasons) rescue StandardError => e error_message = "Failed to fetch CMAB data for experiment #{experiment['key']}." diff --git a/lib/optimizely/optimizely_factory.rb b/lib/optimizely/optimizely_factory.rb index 717e43d9..f2d5ad72 100644 --- a/lib/optimizely/optimizely_factory.rb +++ b/lib/optimizely/optimizely_factory.rb @@ -22,6 +22,8 @@ require 'optimizely/event/batch_event_processor' require 'optimizely/logger' require 'optimizely/notification_center' +require 'optimizely/cmab/cmab_client' +require 'optimizely/cmab/cmab_service' module Optimizely class OptimizelyFactory @@ -83,6 +85,40 @@ def self.blocking_timeout(blocking_timeout) @blocking_timeout = blocking_timeout end + # Convenience method for setting CMAB cache size. + # @param cache_size Integer - Maximum number of items in CMAB cache. + # @param logger - Optional LoggerInterface Provides a log method to log messages. + def self.cmab_cache_size(cache_size, logger = NoOpLogger.new) + unless cache_size.is_a?(Integer) && cache_size.positive? + logger.log( + Logger::ERROR, + "CMAB cache size is invalid, setting to default size #{Optimizely::DefaultCmabCacheOptions::DEFAULT_CMAB_CACHE_SIZE}." + ) + return + end + @cmab_cache_size = cache_size + end + + # Convenience method for setting CMAB cache TTL. + # @param cache_ttl Numeric - Time in seconds for cache entries to live. + # @param logger - Optional LoggerInterface Provides a log method to log messages. + def self.cmab_cache_ttl(cache_ttl, logger = NoOpLogger.new) + unless cache_ttl.is_a?(Numeric) && cache_ttl.positive? + logger.log( + Logger::ERROR, + "CMAB cache TTL is invalid, setting to default TTL #{Optimizely::DefaultCmabCacheOptions::DEFAULT_CMAB_CACHE_TIMEOUT}." + ) + return + end + @cmab_cache_ttl = cache_ttl + end + + # Convenience method for setting custom CMAB cache. + # @param custom_cache - Cache implementation responding to lookup, save, remove, and reset methods. + def self.cmab_custom_cache(custom_cache) + @cmab_custom_cache = custom_cache + end + # Returns a new optimizely instance. # # @params sdk_key - Required String uniquely identifying the fallback datafile corresponding to project. @@ -165,6 +201,14 @@ def self.custom_instance( # rubocop:disable Metrics/ParameterLists notification_center: notification_center ) + # Initialize CMAB components + cmab_client = DefaultCmabClient.new(logger: logger) + cmab_cache = @cmab_custom_cache || LRUCache.new( + @cmab_cache_size || Optimizely::DefaultCmabCacheOptions::DEFAULT_CMAB_CACHE_SIZE, + @cmab_cache_ttl || Optimizely::DefaultCmabCacheOptions::DEFAULT_CMAB_CACHE_TIMEOUT + ) + cmab_service = DefaultCmabService.new(cmab_cache, cmab_client, logger) + Optimizely::Project.new( datafile: datafile, event_dispatcher: event_dispatcher, @@ -176,7 +220,8 @@ def self.custom_instance( # rubocop:disable Metrics/ParameterLists config_manager: config_manager, notification_center: notification_center, event_processor: event_processor, - settings: settings + settings: settings, + cmab_service: cmab_service ) end end diff --git a/spec/cmab/cmab_service_spec.rb b/spec/cmab/cmab_service_spec.rb index 7ac38147..b4b40086 100644 --- a/spec/cmab/cmab_service_spec.rb +++ b/spec/cmab/cmab_service_spec.rb @@ -5,11 +5,12 @@ require 'optimizely/odp/lru_cache' require 'optimizely/cmab/cmab_client' require 'optimizely/decide/optimizely_decide_option' +require 'optimizely/logger' describe Optimizely::DefaultCmabService do let(:mock_cmab_cache) { instance_double(Optimizely::LRUCache) } let(:mock_cmab_client) { instance_double(Optimizely::DefaultCmabClient) } - let(:mock_logger) { double('logger') } + let(:mock_logger) { Optimizely::NoOpLogger.new } let(:cmab_service) { described_class.new(mock_cmab_cache, mock_cmab_client, mock_logger) } let(:mock_project_config) { double('project_config') } @@ -47,18 +48,19 @@ allow(mock_cmab_cache).to receive(:lookup).with(expected_key).and_return(cached_value) - decision = cmab_service.get_decision(mock_project_config, mock_user_context, rule_id, []) + decision, reasons = cmab_service.get_decision(mock_project_config, mock_user_context, rule_id, []) expect(mock_cmab_cache).to have_received(:lookup).with(expected_key) expect(decision.variation_id).to eq('varA') expect(decision.cmab_uuid).to eq('uuid-123') + expect(reasons).to include(match(/CMAB cache hit for user '#{user_id}' and rule '#{rule_id}'/)) end it 'ignores cache when option given' do allow(mock_cmab_client).to receive(:fetch_decision).and_return('varB') expected_attributes = {'age' => 25, 'location' => 'USA'} - decision = cmab_service.get_decision( + decision, reasons = cmab_service.get_decision( mock_project_config, mock_user_context, rule_id, @@ -73,6 +75,7 @@ expected_attributes, decision.cmab_uuid ) + expect(reasons).to include(match(/Ignoring CMAB cache for user '#{user_id}' and rule '#{rule_id}'/)) end it 'invalidates user cache when option given' do @@ -98,7 +101,7 @@ allow(mock_cmab_cache).to receive(:lookup).and_return(nil) allow(mock_cmab_cache).to receive(:save) - decision = cmab_service.get_decision( + decision, reasons = cmab_service.get_decision( mock_project_config, mock_user_context, rule_id, @@ -108,6 +111,7 @@ expect(mock_cmab_cache).to have_received(:reset) expect(decision.variation_id).to eq('varD') expect(decision.cmab_uuid).to be_a(String) + expect(reasons).to include(match(/Resetting CMAB cache for user '#{user_id}' and rule '#{rule_id}'/)) end it 'fetches new decision when hash changes' do @@ -126,7 +130,7 @@ cmab_service.send(:hash_attributes, expected_attributes) expected_key = cmab_service.send(:get_cache_key, user_id, rule_id) - decision = cmab_service.get_decision(mock_project_config, mock_user_context, rule_id, []) + decision, reasons = cmab_service.get_decision(mock_project_config, mock_user_context, rule_id, []) expect(mock_cmab_cache).to have_received(:remove).with(expected_key) expect(mock_cmab_cache).to have_received(:save).with( @@ -140,6 +144,7 @@ expected_attributes, decision.cmab_uuid ) + expect(reasons).to include(match(/CMAB cache attributes mismatch for user '#{user_id}' and rule '#{rule_id}', fetching new decision./)) end it 'only passes cmab attributes to client' do @@ -151,7 +156,7 @@ }) allow(mock_cmab_client).to receive(:fetch_decision).and_return('varF') - decision = cmab_service.get_decision( + decision, reasons = cmab_service.get_decision( mock_project_config, mock_user_context, rule_id, @@ -165,6 +170,7 @@ {'age' => 25, 'location' => 'USA'}, decision.cmab_uuid ) + expect(reasons).to include(match(/Ignoring CMAB cache for user '#{user_id}' and rule '#{rule_id}'/)) end end