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..ce1ee02 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,26 @@ 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: nil, + assignment_config: nil, cohort_sync_config: nil) - @debug = debug || false + @logger = logger + 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 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..f273e66 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,21 @@ 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: nil, + 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 + 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 diff --git a/spec/experiment/local/client_spec.rb b/spec/experiment/local/client_spec.rb index 45a314e..f454c60 100644 --- a/spec/experiment/local/client_spec.rb +++ b/spec/experiment/local/client_spec.rb @@ -27,6 +27,30 @@ 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 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 provided a custom logger' 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::WARN) + end end describe '#evaluation' do diff --git a/spec/experiment/remote/client_spec.rb b/spec/experiment/remote/client_spec.rb index 005066b..0d272d7 100644 --- a/spec/experiment/remote/client_spec.rb +++ b/spec/experiment/remote/client_spec.rb @@ -11,6 +11,30 @@ 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 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 provided a custom logger' 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::WARN) + end end response_with_key = '{"sdk-ci-test":{"key":"on","payload":"payload"}}'