From 9d6b0900c1af8838d19caf24d74f141f027e98fa Mon Sep 17 00:00:00 2001 From: Kenneth Yeh Date: Wed, 29 Oct 2025 22:54:58 -0700 Subject: [PATCH 1/2] feat: allow configs to take in user provided logger instances for client logging behavior --- lib/amplitude/processor.rb | 2 +- lib/amplitude/timeline.rb | 2 +- lib/experiment/local/client.rb | 7 +----- lib/experiment/local/config.rb | 27 ++++++++++++++++++++--- lib/experiment/remote/client.rb | 7 +----- lib/experiment/remote/config.rb | 31 +++++++++++++++++++++++---- spec/experiment/local/client_spec.rb | 26 ++++++++++++++++++++++ spec/experiment/remote/client_spec.rb | 26 ++++++++++++++++++++++ 8 files changed, 107 insertions(+), 21 deletions(-) diff --git a/lib/amplitude/processor.rb b/lib/amplitude/processor.rb index 038cd02..acaf971 100644 --- a/lib/amplitude/processor.rb +++ b/lib/amplitude/processor.rb @@ -87,7 +87,7 @@ def callback(events, code, message) @configuration.callback.call(event, code, message) if @configuration.callback.respond_to?(:call) event.callback(code, message) rescue StandardError => e - @configuration.logger.exception("Error callback for event #{event}: #{e.message}") + @configuration.logger.error("Error callback for event #{event}: #{e.message}") end end diff --git a/lib/amplitude/timeline.rb b/lib/amplitude/timeline.rb index 8ca562e..428893b 100644 --- a/lib/amplitude/timeline.rb +++ b/lib/amplitude/timeline.rb @@ -49,7 +49,7 @@ def flush @plugins[PluginType::DESTINATION].each do |destination| destination_futures << destination.flush rescue StandardError - logger.exception('Error for flush events') + logger.error('Error for flush events') end end destination_futures diff --git a/lib/experiment/local/client.rb b/lib/experiment/local/client.rb index 26eb2de..2865888 100644 --- a/lib/experiment/local/client.rb +++ b/lib/experiment/local/client.rb @@ -14,14 +14,9 @@ class LocalEvaluationClient def initialize(api_key, config = nil) @api_key = api_key @config = config || LocalEvaluationConfig.new + @logger = @config.logger @flags = nil @flags_mutex = Mutex.new - @logger = Logger.new($stdout) - @logger.level = if @config.debug - Logger::DEBUG - else - Logger::INFO - end raise ArgumentError, 'Experiment API key is empty' if @api_key.nil? || @api_key.empty? @engine = Evaluation::Engine.new diff --git a/lib/experiment/local/config.rb b/lib/experiment/local/config.rb index 194bec1..41d53b4 100644 --- a/lib/experiment/local/config.rb +++ b/lib/experiment/local/config.rb @@ -1,3 +1,5 @@ +require 'logger' + module AmplitudeExperiment module ServerZone US = 'US'.freeze @@ -9,11 +11,17 @@ class LocalEvaluationConfig # Default server url DEFAULT_SERVER_URL = 'https://api.lab.amplitude.com'.freeze EU_SERVER_URL = 'https://flag.lab.eu.amplitude.com'.freeze + DEFAULT_LOGDEV = $stdout + DEFAULT_LOG_LEVEL = Logger::ERROR # Set to true to log some extra information to the console. # @return [Boolean] the value of debug attr_accessor :debug + # Set the client logger to a user defined [Logger] + # @return [Logger] the logger instance of the client + attr_accessor :logger + # The server endpoint from which to request variants. # @return [String] the value of server url attr_accessor :server_url @@ -35,16 +43,23 @@ class LocalEvaluationConfig attr_accessor :cohort_sync_config # @param [Boolean] debug Set to true to log some extra information to the console. + # @param [Logger] logger instance to be used for all client logging behavior # @param [String] server_url The server endpoint from which to request variants. # @param [String] server_zone Location of the Amplitude data center to get flags and cohorts from, US or EU # @param [Hash] bootstrap The value of bootstrap. # @param [long] flag_config_polling_interval_millis The value of flag config polling interval in million seconds. # @param [AssignmentConfig] assignment_config Configuration for automatically tracking assignment events after an evaluation. # @param [CohortSyncConfig] cohort_sync_config Configuration for downloading cohorts required for flag evaluation - def initialize(server_url: DEFAULT_SERVER_URL, server_zone: ServerZone::US, bootstrap: {}, - flag_config_polling_interval_millis: 30_000, debug: false, assignment_config: nil, + def initialize(server_url: DEFAULT_SERVER_URL, + server_zone: ServerZone::US, + bootstrap: {}, + flag_config_polling_interval_millis: 30_000, + debug: false, + logger: LocalEvaluationConfig.new_default_logger, + assignment_config: nil, cohort_sync_config: nil) - @debug = debug || false + @logger = logger + @logger.level = Logger::DEBUG if debug @server_url = server_url @server_zone = server_zone @cohort_sync_config = cohort_sync_config @@ -56,5 +71,11 @@ def initialize(server_url: DEFAULT_SERVER_URL, server_zone: ServerZone::US, boot @flag_config_polling_interval_millis = flag_config_polling_interval_millis @assignment_config = assignment_config end + + def self.new_default_logger + logger = Logger.new(DEFAULT_LOGDEV) + logger.level = DEFAULT_LOG_LEVEL + logger + end end end diff --git a/lib/experiment/remote/client.rb b/lib/experiment/remote/client.rb index 18af547..2ae4af1 100644 --- a/lib/experiment/remote/client.rb +++ b/lib/experiment/remote/client.rb @@ -13,12 +13,7 @@ class RemoteEvaluationClient def initialize(api_key, config = nil) @api_key = api_key @config = config || RemoteEvaluationConfig.new - @logger = Logger.new($stdout) - @logger.level = if @config.debug - Logger::DEBUG - else - Logger::INFO - end + @logger = @config.logger endpoint = "#{@config.server_url}/sdk/v2/vardata?v=0" @uri = URI(endpoint) raise ArgumentError, 'Experiment API key is empty' if @api_key.nil? || @api_key.empty? diff --git a/lib/experiment/remote/config.rb b/lib/experiment/remote/config.rb index 690e155..f58ffcd 100644 --- a/lib/experiment/remote/config.rb +++ b/lib/experiment/remote/config.rb @@ -1,13 +1,21 @@ +require 'logger' + module AmplitudeExperiment # Configuration class RemoteEvaluationConfig # Default server url DEFAULT_SERVER_URL = 'https://api.lab.amplitude.com'.freeze + DEFAULT_LOGDEV = $stdout + DEFAULT_LOG_LEVEL = Logger::ERROR # Set to true to log some extra information to the console. # @return [Boolean] the value of debug attr_accessor :debug + # Set the client logger to a user defined [Logger] + # @return [Logger] the logger instance of the client + attr_accessor :logger + # The server endpoint from which to request variants. # @return [Boolean] the value of server url attr_accessor :server_url @@ -43,6 +51,7 @@ class RemoteEvaluationConfig attr_accessor :fetch_retry_timeout_millis # @param [Boolean] debug Set to true to log some extra information to the console. + # @param [Logger] logger instance to be used for all client logging behavior # @param [String] server_url The server endpoint from which to request variants. # @param [Integer] connect_timeout_millis The request connection open timeout, in milliseconds, used when # fetching variants triggered by calling start() or setUser(). @@ -55,10 +64,18 @@ class RemoteEvaluationConfig # greater than the max, the max is used for all subsequent retries. # @param [Float] fetch_retry_backoff_scalar Scales the minimum backoff exponentially. # @param [Integer] fetch_retry_timeout_millis The request timeout for retrying fetch requests. - def initialize(debug: false, server_url: DEFAULT_SERVER_URL, connect_timeout_millis: 60_000, fetch_timeout_millis: 10_000, fetch_retries: 0, - fetch_retry_backoff_min_millis: 500, fetch_retry_backoff_max_millis: 10_000, - fetch_retry_backoff_scalar: 1.5, fetch_retry_timeout_millis: 10_000) - @debug = debug + def initialize(debug: false, + logger: RemoteEvaluationConfig.new_default_logger, + server_url: DEFAULT_SERVER_URL, + connect_timeout_millis: 60_000, + fetch_timeout_millis: 10_000, + fetch_retries: 0, + fetch_retry_backoff_min_millis: 500, + fetch_retry_backoff_max_millis: 10_000, + fetch_retry_backoff_scalar: 1.5, + fetch_retry_timeout_millis: 10_000) + @logger = logger + @logger.level = Logger::DEBUG if debug @server_url = server_url @connect_timeout_millis = connect_timeout_millis @fetch_timeout_millis = fetch_timeout_millis @@ -68,5 +85,11 @@ def initialize(debug: false, server_url: DEFAULT_SERVER_URL, connect_timeout_mil @fetch_retry_backoff_scalar = fetch_retry_backoff_scalar @fetch_retry_timeout_millis = fetch_retry_timeout_millis end + + def self.new_default_logger + logger = Logger.new(DEFAULT_LOGDEV) + logger.level = DEFAULT_LOG_LEVEL + logger + end end end diff --git a/spec/experiment/local/client_spec.rb b/spec/experiment/local/client_spec.rb index 45a314e..2706715 100644 --- a/spec/experiment/local/client_spec.rb +++ b/spec/experiment/local/client_spec.rb @@ -27,6 +27,32 @@ def setup_stub it 'error if api_key is empty' do expect { LocalEvaluationClient.new('') }.to raise_error(ArgumentError) end + + it 'uses custom logger when provided' do + custom_logger = Logger.new($stdout) + config = LocalEvaluationConfig.new(logger: custom_logger) + client = LocalEvaluationClient.new(api_key, config) + + expect(client.instance_variable_get(:@logger)).to eq(custom_logger) + end + + it 'debug flag overrides logger level to DEBUG when debug is true' do + custom_logger = Logger.new($stdout) + custom_logger.level = Logger::WARN + config = LocalEvaluationConfig.new(logger: custom_logger, debug: true) + client = LocalEvaluationClient.new(api_key, config) + + expect(client.instance_variable_get(:@logger).level).to eq(Logger::DEBUG) + end + + it 'debug flag does not modify logger level when debug is false' do + custom_logger = Logger.new($stdout) + custom_logger.level = Logger::WARN + config = LocalEvaluationConfig.new(logger: custom_logger, debug: false) + client = LocalEvaluationClient.new(api_key, config) + + expect(client.instance_variable_get(:@logger).level).to eq(Logger::WARN) + end end describe '#evaluation' do diff --git a/spec/experiment/remote/client_spec.rb b/spec/experiment/remote/client_spec.rb index 005066b..f488166 100644 --- a/spec/experiment/remote/client_spec.rb +++ b/spec/experiment/remote/client_spec.rb @@ -11,6 +11,32 @@ module AmplitudeExperiment it 'raises an error if api_key is empty' do expect { RemoteEvaluationClient.new('') }.to raise_error(ArgumentError) end + + it 'uses custom logger when provided' do + custom_logger = Logger.new($stdout) + config = RemoteEvaluationConfig.new(logger: custom_logger) + client = RemoteEvaluationClient.new(api_key, config) + + expect(client.instance_variable_get(:@logger)).to eq(custom_logger) + end + + it 'debug flag overrides logger level to DEBUG when debug is true' do + custom_logger = Logger.new($stdout) + custom_logger.level = Logger::WARN + config = RemoteEvaluationConfig.new(logger: custom_logger, debug: true) + client = RemoteEvaluationClient.new(api_key, config) + + expect(client.instance_variable_get(:@logger).level).to eq(Logger::DEBUG) + end + + it 'debug flag does not modify logger level when debug is false' do + custom_logger = Logger.new($stdout) + custom_logger.level = Logger::WARN + config = RemoteEvaluationConfig.new(logger: custom_logger, debug: false) + client = RemoteEvaluationClient.new(api_key, config) + + expect(client.instance_variable_get(:@logger).level).to eq(Logger::WARN) + end end response_with_key = '{"sdk-ci-test":{"key":"on","payload":"payload"}}' From 2eee1ff3254bd7a1b17e79bbe3ae4d13c81d7f4f Mon Sep 17 00:00:00 2001 From: Kenneth Yeh Date: Fri, 31 Oct 2025 16:36:59 -0700 Subject: [PATCH 2/2] fix: update logging configs to ignore debug flag when given given a user provided logger --- lib/experiment/local/config.rb | 13 +++++-------- lib/experiment/remote/config.rb | 13 +++++-------- spec/experiment/local/client_spec.rb | 10 ++++------ spec/experiment/remote/client_spec.rb | 10 ++++------ 4 files changed, 18 insertions(+), 28 deletions(-) diff --git a/lib/experiment/local/config.rb b/lib/experiment/local/config.rb index 41d53b4..ce1ee02 100644 --- a/lib/experiment/local/config.rb +++ b/lib/experiment/local/config.rb @@ -55,11 +55,14 @@ def initialize(server_url: DEFAULT_SERVER_URL, bootstrap: {}, flag_config_polling_interval_millis: 30_000, debug: false, - logger: LocalEvaluationConfig.new_default_logger, + logger: nil, assignment_config: nil, cohort_sync_config: nil) @logger = logger - @logger.level = Logger::DEBUG if debug + if logger.nil? + @logger = Logger.new(DEFAULT_LOGDEV) + @logger.level = debug ? Logger::DEBUG : DEFAULT_LOG_LEVEL + end @server_url = server_url @server_zone = server_zone @cohort_sync_config = cohort_sync_config @@ -71,11 +74,5 @@ def initialize(server_url: DEFAULT_SERVER_URL, @flag_config_polling_interval_millis = flag_config_polling_interval_millis @assignment_config = assignment_config end - - def self.new_default_logger - logger = Logger.new(DEFAULT_LOGDEV) - logger.level = DEFAULT_LOG_LEVEL - logger - end end end diff --git a/lib/experiment/remote/config.rb b/lib/experiment/remote/config.rb index f58ffcd..f273e66 100644 --- a/lib/experiment/remote/config.rb +++ b/lib/experiment/remote/config.rb @@ -65,7 +65,7 @@ class RemoteEvaluationConfig # @param [Float] fetch_retry_backoff_scalar Scales the minimum backoff exponentially. # @param [Integer] fetch_retry_timeout_millis The request timeout for retrying fetch requests. def initialize(debug: false, - logger: RemoteEvaluationConfig.new_default_logger, + logger: nil, server_url: DEFAULT_SERVER_URL, connect_timeout_millis: 60_000, fetch_timeout_millis: 10_000, @@ -75,7 +75,10 @@ def initialize(debug: false, fetch_retry_backoff_scalar: 1.5, fetch_retry_timeout_millis: 10_000) @logger = logger - @logger.level = Logger::DEBUG if debug + if logger.nil? + @logger = Logger.new(DEFAULT_LOGDEV) + @logger.level = debug ? Logger::DEBUG : DEFAULT_LOG_LEVEL + end @server_url = server_url @connect_timeout_millis = connect_timeout_millis @fetch_timeout_millis = fetch_timeout_millis @@ -85,11 +88,5 @@ def initialize(debug: false, @fetch_retry_backoff_scalar = fetch_retry_backoff_scalar @fetch_retry_timeout_millis = fetch_retry_timeout_millis end - - def self.new_default_logger - logger = Logger.new(DEFAULT_LOGDEV) - logger.level = DEFAULT_LOG_LEVEL - logger - end end end diff --git a/spec/experiment/local/client_spec.rb b/spec/experiment/local/client_spec.rb index 2706715..f454c60 100644 --- a/spec/experiment/local/client_spec.rb +++ b/spec/experiment/local/client_spec.rb @@ -36,19 +36,17 @@ def setup_stub expect(client.instance_variable_get(:@logger)).to eq(custom_logger) end - it 'debug flag overrides logger level to DEBUG when debug is true' do - custom_logger = Logger.new($stdout) - custom_logger.level = Logger::WARN - config = LocalEvaluationConfig.new(logger: custom_logger, debug: true) + it 'debug flag overrides logger level to DEBUG when not provided a custom logger ' do + config = LocalEvaluationConfig.new(debug: true) client = LocalEvaluationClient.new(api_key, config) expect(client.instance_variable_get(:@logger).level).to eq(Logger::DEBUG) end - it 'debug flag does not modify logger level when debug is false' do + it 'debug flag does not modify logger level when provided a custom logger' do custom_logger = Logger.new($stdout) custom_logger.level = Logger::WARN - config = LocalEvaluationConfig.new(logger: custom_logger, debug: false) + config = LocalEvaluationConfig.new(logger: custom_logger, debug: true) client = LocalEvaluationClient.new(api_key, config) expect(client.instance_variable_get(:@logger).level).to eq(Logger::WARN) diff --git a/spec/experiment/remote/client_spec.rb b/spec/experiment/remote/client_spec.rb index f488166..0d272d7 100644 --- a/spec/experiment/remote/client_spec.rb +++ b/spec/experiment/remote/client_spec.rb @@ -20,19 +20,17 @@ module AmplitudeExperiment expect(client.instance_variable_get(:@logger)).to eq(custom_logger) end - it 'debug flag overrides logger level to DEBUG when debug is true' do - custom_logger = Logger.new($stdout) - custom_logger.level = Logger::WARN - config = RemoteEvaluationConfig.new(logger: custom_logger, debug: true) + it 'debug flag overrides logger level to DEBUG when not provided a custom logger ' do + config = RemoteEvaluationConfig.new(debug: true) client = RemoteEvaluationClient.new(api_key, config) expect(client.instance_variable_get(:@logger).level).to eq(Logger::DEBUG) end - it 'debug flag does not modify logger level when debug is false' do + it 'debug flag does not modify logger level when provided a custom logger' do custom_logger = Logger.new($stdout) custom_logger.level = Logger::WARN - config = RemoteEvaluationConfig.new(logger: custom_logger, debug: false) + config = RemoteEvaluationConfig.new(logger: custom_logger, debug: true) client = RemoteEvaluationClient.new(api_key, config) expect(client.instance_variable_get(:@logger).level).to eq(Logger::WARN)