Skip to content

Commit

Permalink
Do not use trace on internal logger
Browse files Browse the repository at this point in the history
The internal logger does not support `trace`. Use `debug` instead.

Since the heartbeat implementation intentionally swallows all errors
and logs them, issues such as this would go unnoticed by the test
suite. Add assertions on the messages that are expected to be
logged to ensure that the implementation is actually working as
expected.
  • Loading branch information
unflxw committed May 14, 2024
1 parent 0012912 commit 30bb675
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 3 deletions.
6 changes: 6 additions & 0 deletions .changesets/fix-issue-when-sending-heartbeats.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: patch
type: fix
---

Fix an issue where an error about the AppSignal internal logger is raised when sending a heartbeat.
2 changes: 1 addition & 1 deletion lib/appsignal/heartbeat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def transmit_event(kind)
response = self.class.transmitter.transmit(event(kind))

if response.code.to_i >= 200 && response.code.to_i < 300
Appsignal.internal_logger.trace("Transmitted heartbeat `#{name}` (#{id}) #{kind} event")
Appsignal.internal_logger.debug("Transmitted heartbeat `#{name}` (#{id}) #{kind} event")
else
Appsignal.internal_logger.error(
"Failed to transmit heartbeat event: status code was #{response.code}"
Expand Down
42 changes: 40 additions & 2 deletions spec/lib/appsignal/heartbeat_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,26 @@
expect(transmitter).to receive(:transmit).with(hash_including(
:name => "heartbeat-name",
:kind => "start"
)).and_return(nil)
)).and_return(Net::HTTPResponse.new(nil, "200", nil))

expect(Appsignal.internal_logger).to receive(:debug).with(
"Transmitted heartbeat `heartbeat-name` (#{heartbeat.id}) start event"
)
expect(Appsignal.internal_logger).not_to receive(:error)

heartbeat.start
end

it "should log an error if it fails" do
expect(transmitter).to receive(:transmit).with(hash_including(
:name => "heartbeat-name",
:kind => "start"
)).and_return(Net::HTTPResponse.new(nil, "499", nil))

expect(Appsignal.internal_logger).not_to receive(:debug)
expect(Appsignal.internal_logger).to receive(:error).with(
"Failed to transmit heartbeat event: status code was 499"
)

heartbeat.start
end
Expand All @@ -35,7 +54,26 @@
expect(transmitter).to receive(:transmit).with(hash_including(
:name => "heartbeat-name",
:kind => "finish"
)).and_return(nil)
)).and_return(Net::HTTPResponse.new(nil, "200", nil))

expect(Appsignal.internal_logger).to receive(:debug).with(
"Transmitted heartbeat `heartbeat-name` (#{heartbeat.id}) finish event"
)
expect(Appsignal.internal_logger).not_to receive(:error)

heartbeat.finish
end

it "should log an error if it fails" do
expect(transmitter).to receive(:transmit).with(hash_including(
:name => "heartbeat-name",
:kind => "finish"
)).and_return(Net::HTTPResponse.new(nil, "499", nil))

expect(Appsignal.internal_logger).not_to receive(:debug)
expect(Appsignal.internal_logger).to receive(:error).with(
"Failed to transmit heartbeat event: status code was 499"
)

heartbeat.finish
end
Expand Down

0 comments on commit 30bb675

Please sign in to comment.