Skip to content

Commit

Permalink
Use log_level instead of debug
Browse files Browse the repository at this point in the history
To match the changes implemented in [appsignal-agent#730][agent],
the implementation of `Appsignal.Config.debug?/0`, which is in
turn used by `Appsignal.Logger.debug/1`, is modified to use valid
`log_level` config values as its source of truth, and fall back on
the deprecated `debug` and `transaction_debug_mode` config options
only if no valid `log_level` value is present.

Before this change, this method only checked the `debug`
config option, not `transaction_debug_mode`. In order to be
consistent with the changes to the agent (`transaction_debug_mode`
implies `debug`, because a log level of "trace" implies "debug")
this method now checks `transaction_debug_mode` as well.

Fixes #746.

[agent]: appsignal/appsignal-agent#730
  • Loading branch information
unflxw committed Jan 21, 2022
1 parent 4aa8fe3 commit e4ec8e6
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 8 deletions.
6 changes: 6 additions & 0 deletions .changesets/use-log-level-config-option-instead-of-debug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "fix"
---

Prefer the value of the `log_level` config option, instead of the deprecated `debug` config option, when deciding whether to log a debug message. If `log_level` does not have a value, or its value is invalid, the values of the deprecated `debug` and `transaction_debug_mode` config options are taken into account.
9 changes: 8 additions & 1 deletion lib/appsignal/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,14 @@ defmodule Appsignal.Config do
"""
@spec debug?() :: boolean
def debug? do
Application.get_env(:appsignal, :config, @default_config)[:debug] || false
config = Application.get_env(:appsignal, :config, @default_config)
log_level = config[:log_level]

if Enum.member?(["error", "warn", "info", "debug", "trace"], log_level) do
log_level == "debug" || log_level == "trace"
else
!!(config[:debug] || config[:transaction_debug_mode])
end
end

def request_headers do
Expand Down
56 changes: 53 additions & 3 deletions test/appsignal/config_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,65 @@ defmodule Appsignal.ConfigTest do
end

describe "debug?" do
test "when debug mode is turned on" do
test "when debug is turned on" do
assert with_config(%{debug: true}, &Config.debug?/0) == true
end

test "when debug mode is turned off" do
test "when debug is turned off" do
assert with_config(%{debug: false}, &Config.debug?/0) == false
end

test "when debug mode is not configured" do
test "when transaction debug mode is turned on" do
assert with_config(%{transaction_debug_mode: true}, &Config.debug?/0) == true
end

test "when transaction debug mode is turned off" do
assert with_config(%{transaction_debug_mode: false}, &Config.debug?/0) == false
end

test "when log_level is trace" do
assert with_config(%{log_level: "trace"}, &Config.debug?/0) == true
end

test "when log_level is debug" do
assert with_config(%{log_level: "debug"}, &Config.debug?/0) == true
end

test "when log_level is a logging level other than debug or trace" do
config = %{log_level: "warn"}

assert with_config(config, &Config.debug?/0) == false

# ignores debug and transaction_debug_mode
assert with_config(
Map.put(config, :debug, true),
&Config.debug?/0
) == false

assert with_config(
Map.put(config, :transaction_debug_mode, true),
&Config.debug?/0
) == false
end

test "when log_level is not a logging level" do
config = %{log_level: "foobar"}

assert with_config(config, &Config.debug?/0) == false

# checks debug and transaction_debug_mode
assert with_config(
Map.put(config, :debug, true),
&Config.debug?/0
) == true

assert with_config(
Map.put(config, :transaction_debug_mode, true),
&Config.debug?/0
) == true
end

test "with empty config" do
assert with_config(%{}, &Config.debug?/0) == false
end
end
Expand Down
8 changes: 4 additions & 4 deletions test/appsignal/logger_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ defmodule Appsignal.LoggerTest do
:ok
end

test "logs when debug mode is turned on" do
test "logs when log level is debug or trace" do
assert :ok ==
with_config(
%{debug: true},
%{log_level: "debug"},
fn ->
Appsignal.Logger.debug("debug!")
end
Expand All @@ -19,10 +19,10 @@ defmodule Appsignal.LoggerTest do
assert {:ok, [{"debug!"}]} == Appsignal.Test.Logger.get(:debug)
end

test "logs when debug mode is turned off" do
test "logs when log level is not debug or trace" do
assert :ok ==
with_config(
%{debug: false},
%{log_level: "warn"},
fn ->
Appsignal.Logger.debug("debug!")
end
Expand Down

0 comments on commit e4ec8e6

Please sign in to comment.