Skip to content

Commit

Permalink
Issue/#18/Handle error logging for third party issues
Browse files Browse the repository at this point in the history
  • Loading branch information
praweb authored Apr 3, 2019
2 parents f3fe44a + fd91c86 commit 4dd5740
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 6 deletions.
8 changes: 7 additions & 1 deletion lib/shift/circuit_breaker/circuit_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,13 @@ def handle_exception(exception, fallback)
end

def log_errors(exception)
logger.error(circuit_name: name, state: state, error_message: exception.message) if error_logging_enabled
logger.error(
circuit_name: name,
state: state,
exception: exception,
error_message: exception.message,
remote_logging_enabled: error_logging_enabled
)
end
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/shift/circuit_breaker/circuit_logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ def initialize(logger: ::Logger.new(STDOUT), remote_logger: Shift::CircuitBreake
def error(context)
message = (ERROR_MESSAGE % context)
logger.error(message)
remote_logger.call(message) if remote_logger.respond_to?(:call)
::NewRelic::Agent.notice_error(context[:exception], expected: true) if defined?(NewRelic)
remote_logger.call(message) if context[:remote_logging_enabled] && remote_logger.respond_to?(:call)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/shift/circuit_breaker/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

module Shift
module CircuitBreaker
VERSION = "0.2.2"
VERSION = "0.2.3"
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ module CircuitBreaker
end

context "when it is disabled" do
it "should not log errors" do
it "should log errors to console" do
# Arrange
operation_stub = instance_double("Operation")
fallback_stub = instance_double("Fallback")
Expand All @@ -181,7 +181,7 @@ module CircuitBreaker
aggregate_failures do
expect(operation_result).to eq(fallback_stub)
expect(cb.error_count).to eq(1)
expect(error_logger).not_to have_received(:error)
expect(error_logger).to have_received(:error)
end
end
end
Expand Down
29 changes: 28 additions & 1 deletion spec/shift/circuit_breaker/circuit_logger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

require "spec_helper"

module NewRelic
module Agent
def self.notice_error(*); end
end
end

module Shift
module CircuitBreaker
describe CircuitLogger do
Expand All @@ -28,12 +34,13 @@ module CircuitBreaker

it "logs the given error message using the provided remote_logger" do
# Arrange
context = { circuit_name: :test_circuit_breaker, error_message: "timeout", state: :open }
context = { circuit_name: :test_circuit_breaker, error_message: "timeout", state: :open, remote_logging_enabled: true }
error_message = (described_class::ERROR_MESSAGE % context)
remote_logger = Shift::CircuitBreaker::Adapters::SentryAdapter
logger = described_class.new(remote_logger: remote_logger)

allow(remote_logger).to receive(:call)
allow(::NewRelic::Agent).to receive(:notice_error)

# Act
logger.error(context)
Expand All @@ -44,6 +51,26 @@ module CircuitBreaker
expect(remote_logger).to have_received(:call).with(include(error_message))
end
end

it "should not log error to remote_error if remote_logging_enabled is set to false" do
# Arrange
context = { circuit_name: :test_circuit_breaker, error_message: "timeout", state: :open, remote_logging_enabled: false }
error_message = (described_class::ERROR_MESSAGE % context)
remote_logger = Shift::CircuitBreaker::Adapters::SentryAdapter
logger = described_class.new(remote_logger: remote_logger)

allow(remote_logger).to receive(:call)
allow(::NewRelic::Agent).to receive(:notice_error)

# Act
logger.error(context)

# Assert
aggregate_failures do
expect(logger.remote_logger).to eq(remote_logger)
expect(remote_logger).not_to have_received(:call).with(include(error_message))
end
end
end

context "#info" do
Expand Down

0 comments on commit 4dd5740

Please sign in to comment.