Skip to content

Commit

Permalink
Add config options to disable instrumentations
Browse files Browse the repository at this point in the history
Add the `instrument_ecto`, `instrument_finch` and `instrument_oban`
config options to disable the automatic Telemetry instrumentations.
  • Loading branch information
unflxw committed Feb 1, 2023
1 parent 2fa9e19 commit 837e028
Show file tree
Hide file tree
Showing 9 changed files with 206 additions and 6 deletions.
8 changes: 8 additions & 0 deletions .changesets/disable-instrumentations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
bump: "patch"
type: "add"
---

Add config options to disable automatic Ecto, Finch and Oban instrumentations.
Set `instrument_ecto`, `instrument_finch` or `instrument_oban` to `true` in
order to disable that instrumentation.
7 changes: 7 additions & 0 deletions .changesets/fix-enable-error-backend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: "patch"
type: "fix"
---

Fix the default value of `enable_error_backend` so it defaults to `true` when
the config option is not set.
14 changes: 11 additions & 3 deletions lib/appsignal.ex
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,17 @@ defmodule Appsignal do
Appsignal.Error.Backend.attach()
end

Appsignal.Ecto.attach()
Appsignal.Finch.attach()
Appsignal.Oban.attach()
if Config.instrument_ecto?() do
Appsignal.Ecto.attach()
end

if Config.instrument_finch?() do
Appsignal.Finch.attach()
end

if Config.instrument_oban?() do
Appsignal.Oban.attach()
end

children = [
{Appsignal.Tracer, []},
Expand Down
32 changes: 30 additions & 2 deletions lib/appsignal/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ defmodule Appsignal.Config do
ignore_actions: [],
ignore_errors: [],
ignore_namespaces: [],
instrument_ecto: true,
instrument_finch: true,
instrument_oban: true,
log: "file",
logging_endpoint: "https://appsignal-endpoint.net",
request_headers: ~w(
Expand Down Expand Up @@ -175,14 +178,35 @@ defmodule Appsignal.Config do

def minutely_probes_enabled? do
case Application.fetch_env(:appsignal, :config) do
{:ok, value} -> !!value[:enable_minutely_probes]
{:ok, value} -> !!Map.get(value, :enable_minutely_probes, false)
_ -> false
end
end

def error_backend_enabled? do
case Application.fetch_env(:appsignal, :config) do
{:ok, value} -> !!value[:enable_error_backend]
{:ok, value} -> !!Map.get(value, :enable_error_backend, true)
_ -> true
end
end

def instrument_ecto? do
case Application.fetch_env(:appsignal, :config) do
{:ok, value} -> !!Map.get(value, :instrument_ecto, true)
_ -> true
end
end

def instrument_finch? do
case Application.fetch_env(:appsignal, :config) do
{:ok, value} -> !!Map.get(value, :instrument_finch, true)
_ -> true
end
end

def instrument_oban? do
case Application.fetch_env(:appsignal, :config) do
{:ok, value} -> !!Map.get(value, :instrument_oban, true)
_ -> true
end
end
Expand Down Expand Up @@ -242,6 +266,9 @@ defmodule Appsignal.Config do
"APPSIGNAL_IGNORE_ACTIONS" => :ignore_actions,
"APPSIGNAL_IGNORE_ERRORS" => :ignore_errors,
"APPSIGNAL_IGNORE_NAMESPACES" => :ignore_namespaces,
"APPSIGNAL_INSTRUMENT_ECTO" => :instrument_ecto,
"APPSIGNAL_INSTRUMENT_FINCH" => :instrument_finch,
"APPSIGNAL_INSTRUMENT_OBAN" => :instrument_oban,
"APPSIGNAL_LOG" => :log,
"APPSIGNAL_LOG_LEVEL" => :log_level,
"APPSIGNAL_LOG_PATH" => :log_path,
Expand Down Expand Up @@ -274,6 +301,7 @@ defmodule Appsignal.Config do
APPSIGNAL_TRANSACTION_DEBUG_MODE APPSIGNAL_FILES_WORLD_ACCESSIBLE APPSIGNAL_SEND_PARAMS
APPSIGNAL_ENABLE_MINUTELY_PROBES APPSIGNAL_ENABLE_STATSD APPSIGNAL_ENABLE_NGINX_METRICS
APPSIGNAL_ENABLE_ERROR_BACKEND APPSIGNAL_SEND_ENVIRONMENT_METADATA
APPSIGNAL_INSTRUMENT_ECTO APPSIGNAL_INSTRUMENT_FINCH APPSIGNAL_INSTRUMENT_OBAN
)
@atom_keys ~w(APPSIGNAL_APP_ENV APPSIGNAL_OTP_APP)
@string_list_keys ~w(
Expand Down
4 changes: 4 additions & 0 deletions lib/appsignal/ecto.ex
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@ defmodule Appsignal.Ecto do
:ok ->
Appsignal.IntegrationLogger.debug("Appsignal.Ecto attached to #{inspect(event)}")

:ok

{:error, _} = error ->
Logger.warn("Appsignal.Ecto not attached to #{inspect(event)}: #{inspect(error)}")

error
end
end

Expand Down
101 changes: 100 additions & 1 deletion test/appsignal/config_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,102 @@ defmodule Appsignal.ConfigTest do
end
end

describe "error_backend_enabled?" do
test "when true" do
assert with_config(
%{enable_error_backend: true},
&Config.error_backend_enabled?/0
)
end

test "when false" do
refute with_config(
%{enable_error_backend: false},
&Config.error_backend_enabled?/0
)
end

test "when unset" do
assert with_config(%{}, &Config.error_backend_enabled?/0)
end

test "without an appsignal config" do
assert without_config(&Config.error_backend_enabled?/0)
end
end

describe "instrument_oban?" do
test "when true" do
assert with_config(
%{instrument_oban: true},
&Config.instrument_oban?/0
)
end

test "when false" do
refute with_config(
%{instrument_oban: false},
&Config.instrument_oban?/0
)
end

test "when unset" do
assert with_config(%{}, &Config.instrument_oban?/0)
end

test "without an appsignal config" do
assert without_config(&Config.instrument_oban?/0)
end
end

describe "instrument_ecto?" do
test "when true" do
assert with_config(
%{instrument_ecto: true},
&Config.instrument_ecto?/0
)
end

test "when false" do
refute with_config(
%{instrument_ecto: false},
&Config.instrument_ecto?/0
)
end

test "when unset" do
assert with_config(%{}, &Config.instrument_ecto?/0)
end

test "without an appsignal config" do
assert without_config(&Config.instrument_ecto?/0)
end
end

describe "instrument_finch?" do
test "when discard" do
assert with_config(
%{instrument_finch: true},
&Config.instrument_finch?/0
)
end

test "when false" do
refute with_config(
%{instrument_finch: false},
&Config.instrument_finch?/0
)
end

test "when unset" do
assert with_config(%{}, &Config.instrument_finch?/0)
end

test "without an appsignal config" do
assert without_config(&Config.instrument_finch?/0)
end
end

describe "log_level" do
test "without an appsignal config" do
assert without_config(&Config.log_level/0) == :info
Expand Down Expand Up @@ -1171,7 +1267,10 @@ defmodule Appsignal.ConfigTest do
send_params: true,
send_session_data: true,
skip_session_data: false,
transaction_debug_mode: false
transaction_debug_mode: false,
instrument_ecto: true,
instrument_finch: true,
instrument_oban: true
}
end

Expand Down
10 changes: 10 additions & 0 deletions test/appsignal/ecto_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ defmodule Appsignal.EctoTest do
assert attached?([:appsignal, :test, :repo, :query])
end

test "is not attached automatically when :instrument_ecto is set to false" do
:telemetry.detach({Ecto, [:appsignal, :test, :repo, :query]})

with_config(%{instrument_ecto: false}, fn -> Appsignal.start([], []) end)

assert !attached?([:appsignal, :test, :repo, :query])

:ok = Ecto.attach()
end

test "attach/0 attaches to configured repos" do
assert with_config(
%{ecto_repos: [Appsignal.Test.RepoOne, Appsignal.Test.RepoTwo]},
Expand Down
15 changes: 15 additions & 0 deletions test/appsignal/finch_test.exs
Original file line number Diff line number Diff line change
@@ -1,13 +1,28 @@
defmodule Appsignal.FinchTest do
use ExUnit.Case
alias Appsignal.{Span, Test}
import AppsignalTest.Utils, only: [with_config: 2]

test "attaches to Finch events automatically" do
assert attached?([:finch, :request, :start])
assert attached?([:finch, :request, :stop])
assert attached?([:finch, :request, :exception])
end

test "does not attach to Finch events when :instrument_finch is set to false" do
:telemetry.detach({Appsignal.Finch, [:finch, :request, :start]})
:telemetry.detach({Appsignal.Finch, [:finch, :request, :stop]})
:telemetry.detach({Appsignal.Finch, [:finch, :request, :exception]})

with_config(%{instrument_finch: false}, fn -> Appsignal.start([], []) end)

assert !attached?([:finch, :request, :start])
assert !attached?([:finch, :request, :stop])
assert !attached?([:finch, :request, :exception])

[:ok, :ok, :ok] = Appsignal.Finch.attach()
end

describe "finch_request_start/4, without a root span" do
setup do
start_supervised!(Test.Nif)
Expand Down
21 changes: 21 additions & 0 deletions test/appsignal/oban_test.exs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
defmodule Appsignal.ObanTest do
use ExUnit.Case
alias Appsignal.{FakeAppsignal, Span, Test}
import AppsignalTest.Utils, only: [with_config: 2]

test "attaches to Oban events automatically" do
assert attached?([:oban, :job, :start])
Expand All @@ -11,6 +12,26 @@ defmodule Appsignal.ObanTest do
assert attached?([:oban, :engine, :insert_job, :exception])
end

test "does not attach to Oban events when :instrument_oban is set to false" do
:telemetry.detach({Appsignal.Oban, [:oban, :job, :start]})
:telemetry.detach({Appsignal.Oban, [:oban, :job, :stop]})
:telemetry.detach({Appsignal.Oban, [:oban, :job, :exception]})
:telemetry.detach({Appsignal.Oban, [:oban, :engine, :insert_job, :start]})
:telemetry.detach({Appsignal.Oban, [:oban, :engine, :insert_job, :stop]})
:telemetry.detach({Appsignal.Oban, [:oban, :engine, :insert_job, :exception]})

with_config(%{instrument_oban: false}, fn -> Appsignal.start([], []) end)

assert !attached?([:oban, :job, :start])
assert !attached?([:oban, :job, :stop])
assert !attached?([:oban, :job, :exception])
assert !attached?([:oban, :engine, :insert_job, :start])
assert !attached?([:oban, :engine, :insert_job, :stop])
assert !attached?([:oban, :engine, :insert_job, :exception])

[:ok, :ok, :ok, :ok, :ok, :ok] = Appsignal.Oban.attach()
end

describe "oban_job_start/4" do
setup do
start_supervised!(Test.Nif)
Expand Down

0 comments on commit 837e028

Please sign in to comment.