From 4c4ff47517c7ca6a3941342225d5470dc7bdeb8e Mon Sep 17 00:00:00 2001 From: crbelaus Date: Sat, 8 Jun 2024 13:27:23 +0200 Subject: [PATCH 01/13] Track error context --- lib/error_tracker.ex | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/error_tracker.ex b/lib/error_tracker.ex index 44cf145..afae1da 100644 --- a/lib/error_tracker.ex +++ b/lib/error_tracker.ex @@ -3,10 +3,17 @@ defmodule ErrorTracker do En Elixir based built-in error tracking solution. """ - def report(exception, stacktrace, context \\ %{}) do + @typedoc """ + A map containing the relvant context for a particular error. + """ + @type context :: map() + + def report(exception, stacktrace, given_context \\ %{}) do {:ok, stacktrace} = ErrorTracker.Stacktrace.new(stacktrace) {:ok, error} = ErrorTracker.Error.new(exception, stacktrace) + context = Map.merge(get_context(), given_context) + error = repo().insert!(error, on_conflict: [set: [status: :unresolved]], @@ -26,4 +33,18 @@ defmodule ErrorTracker do def prefix do Application.get_env(:error_tracker, :prefix, "public") end + + @spec set_context(context()) :: context() + def set_context(params) when is_map(params) do + current_context = Process.get(:error_tracker_context, %{}) + + Process.put(:error_tracker_context, Map.merge(current_context, params)) + + params + end + + @spec get_context() :: context() + def get_context do + Process.get(:error_tracker_context, %{}) + end end From 2e8deaa27b508c98112db6af1095ad4ead37366d Mon Sep 17 00:00:00 2001 From: crbelaus Date: Sat, 8 Jun 2024 13:27:38 +0200 Subject: [PATCH 02/13] Plug for automatic request context --- dev.exs | 1 + lib/error_tracker/integrations/phoenix.ex | 16 ++++++---- lib/error_tracker/integrations/plug.ex | 38 +++++++++++++++++++++-- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/dev.exs b/dev.exs index 437f027..4aacafd 100644 --- a/dev.exs +++ b/dev.exs @@ -125,6 +125,7 @@ defmodule ErrorTrackerDevWeb.Endpoint do plug Plug.RequestId plug Plug.Telemetry, event_prefix: [:phoenix, :endpoint] plug :maybe_exception + plug ErrorTrackerDevWeb.Router def maybe_exception(%Plug.Conn{path_info: ["plug-exception"]}, _), do: raise("Plug exception") diff --git a/lib/error_tracker/integrations/phoenix.ex b/lib/error_tracker/integrations/phoenix.ex index bfa4a43..117c209 100644 --- a/lib/error_tracker/integrations/phoenix.ex +++ b/lib/error_tracker/integrations/phoenix.ex @@ -10,14 +10,14 @@ defmodule ErrorTracker.Integrations.Phoenix do alias ErrorTracker.Integrations.Plug, as: PlugIntegration + @events %{ + [:phoenix, :router_dispatch, :exception] => &__MODULE__.handle_exception/4, + [:phoenix, :router_dispatch, :start] => &__MODULE__.add_context/4 + } + def attach do if Application.spec(:phoenix) do - :telemetry.attach( - __MODULE__, - [:phoenix, :router_dispatch, :exception], - &__MODULE__.handle_exception/4, - [] - ) + for {event, handler} <- @events, do: :telemetry.attach(__MODULE__, event, handler, []) end end @@ -38,4 +38,8 @@ defmodule ErrorTracker.Integrations.Phoenix do ) do PlugIntegration.report_error(conn, reason, stack) end + + def add_context([:phoenix, :router, :dispatch], _measurements, %{conn: conn}, _opts) do + PlugIntegration.set_context(conn) + end end diff --git a/lib/error_tracker/integrations/plug.ex b/lib/error_tracker/integrations/plug.ex index d5f42ab..cb6c0d3 100644 --- a/lib/error_tracker/integrations/plug.ex +++ b/lib/error_tracker/integrations/plug.ex @@ -68,13 +68,47 @@ defmodule ErrorTracker.Integrations.Plug do end end - def report_error(_conn, reason, stack) do + def report_error(conn, reason, stack) do unless Process.get(:error_tracker_router_exception_reported) do try do - ErrorTracker.report(reason, stack) + ErrorTracker.report(reason, stack, set_context(conn)) after Process.put(:error_tracker_router_exception_reported, true) end end end + + def set_context(conn = %Plug.Conn{}) do + ErrorTracker.set_context(%{ + "request.host" => conn.host, + "request.path" => conn.request_path, + "request.query" => conn.query_string, + "request.method" => conn.method, + "request.ip" => remote_ip(conn), + "request.headers" => Map.new(conn.req_headers), + "response.headers" => Map.new(conn.resp_headers), + # TODO most likey this will be a %Plug.Conn.Unfetched{} + # Should we fetch it here? + "request.params" => %{}, + "request.session" => %{} + }) + + conn + end + + defp remote_ip(conn = %Plug.Conn{}) do + remote_ip = + case Plug.Conn.get_req_header(conn, "x-forwarded-for") do + [x_forwarded_for | _] -> + x_forwarded_for |> String.split(",", parts: 2) |> List.first() + + [] -> + case :inet.ntoa(conn.remote_ip) do + {:error, _} -> "" + address -> to_string(address) + end + end + + String.trim(remote_ip) + end end From b105eaca9ee0205cf64d733a4cab87966f71542e Mon Sep 17 00:00:00 2001 From: crbelaus Date: Sat, 8 Jun 2024 16:09:34 +0200 Subject: [PATCH 03/13] Oban context --- dev.exs | 2 +- lib/error_tracker/integrations/oban.ex | 22 +++++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/dev.exs b/dev.exs index 4aacafd..a6003a2 100644 --- a/dev.exs +++ b/dev.exs @@ -125,7 +125,6 @@ defmodule ErrorTrackerDevWeb.Endpoint do plug Plug.RequestId plug Plug.Telemetry, event_prefix: [:phoenix, :endpoint] plug :maybe_exception - plug ErrorTrackerDevWeb.Router def maybe_exception(%Plug.Conn{path_info: ["plug-exception"]}, _), do: raise("Plug exception") @@ -158,3 +157,4 @@ Task.async(fn -> Process.sleep(:infinity) end) +|> Task.await(:infinity) diff --git a/lib/error_tracker/integrations/oban.ex b/lib/error_tracker/integrations/oban.ex index 3f5758b..f6c50a0 100644 --- a/lib/error_tracker/integrations/oban.ex +++ b/lib/error_tracker/integrations/oban.ex @@ -14,14 +14,22 @@ defmodule ErrorTracker.Integrations.Oban do end end - def handle_event([:oban, :job, :exception], _measurements, metadata, :no_config) do - %{job: job, reason: exception, stacktrace: stacktrace} = metadata + def handle_event([:oban, :job, :start], _measurements, metadata, :no_config) do + %{job: job} = metadata - ErrorTracker.report(exception, stacktrace, %{ - job_id: job.id, - job_attempt: job.attempt, - job_queue: job.queue, - job_worker: job.worker + ErrorTracker.set_context(%{ + "job.args" => job.args, + "job.attempt" => job.attempt, + "job.id" => job.id, + "job.priority" => job.priority, + "job.queue" => job.queue, + "job.worker" => job.worker }) end + + def handle_event([:oban, :job, :exception], _measurements, metadata, :no_config) do + %{reason: exception, stacktrace: stacktrace} = metadata + + ErrorTracker.report(exception, stacktrace) + end end From 5fbfa4fb8a542bfe926df9607d5c374705ec43f9 Mon Sep 17 00:00:00 2001 From: crbelaus Date: Wed, 12 Jun 2024 16:04:50 +0200 Subject: [PATCH 04/13] Set context on Plug integration --- lib/error_tracker/integrations/phoenix.ex | 8 ++++---- lib/error_tracker/integrations/plug.ex | 11 ++++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/error_tracker/integrations/phoenix.ex b/lib/error_tracker/integrations/phoenix.ex index 117c209..7e8a407 100644 --- a/lib/error_tracker/integrations/phoenix.ex +++ b/lib/error_tracker/integrations/phoenix.ex @@ -24,19 +24,19 @@ defmodule ErrorTracker.Integrations.Phoenix do def handle_exception( [:phoenix, :router_dispatch, :exception], _measurements, - %{reason: %Plug.Conn.WrapperError{conn: conn, reason: reason, stack: stack}}, + %{reason: %Plug.Conn.WrapperError{reason: reason, stack: stack}}, _opts ) do - PlugIntegration.report_error(conn, reason, stack) + PlugIntegration.report_error(reason, stack) end def handle_exception( [:phoenix, :router_dispatch, :exception], _measurements, - %{reason: reason, stacktrace: stack, conn: conn}, + %{reason: reason, stacktrace: stack}, _opts ) do - PlugIntegration.report_error(conn, reason, stack) + PlugIntegration.report_error(reason, stack) end def add_context([:phoenix, :router, :dispatch], _measurements, %{conn: conn}, _opts) do diff --git a/lib/error_tracker/integrations/plug.ex b/lib/error_tracker/integrations/plug.ex index cb6c0d3..bfef4fc 100644 --- a/lib/error_tracker/integrations/plug.ex +++ b/lib/error_tracker/integrations/plug.ex @@ -46,32 +46,33 @@ defmodule ErrorTracker.Integrations.Plug do defoverridable call: 2 def call(conn, opts) do + unquote(__MODULE__).set_context(conn) super(conn, opts) rescue e in Plug.Conn.WrapperError -> - unquote(__MODULE__).report_error(conn, e, e.stack) + unquote(__MODULE__).report_error(e, e.stack) Plug.Conn.WrapperError.reraise(e) e -> stack = __STACKTRACE__ - unquote(__MODULE__).report_error(conn, e, stack) + unquote(__MODULE__).report_error(e, stack) :erlang.raise(:error, e, stack) catch kind, reason -> stack = __STACKTRACE__ - unquote(__MODULE__).report_error(conn, reason, stack) + unquote(__MODULE__).report_error(reason, stack) :erlang.raise(kind, reason, stack) end end end - def report_error(conn, reason, stack) do + def report_error(reason, stack) do unless Process.get(:error_tracker_router_exception_reported) do try do - ErrorTracker.report(reason, stack, set_context(conn)) + ErrorTracker.report(reason, stack) after Process.put(:error_tracker_router_exception_reported, true) end From 3066dbbf4758f761e067aba5a947c36ac9435fed Mon Sep 17 00:00:00 2001 From: crbelaus Date: Wed, 12 Jun 2024 16:07:33 +0200 Subject: [PATCH 05/13] Remove line --- dev.exs | 1 - 1 file changed, 1 deletion(-) diff --git a/dev.exs b/dev.exs index a6003a2..437f027 100644 --- a/dev.exs +++ b/dev.exs @@ -157,4 +157,3 @@ Task.async(fn -> Process.sleep(:infinity) end) -|> Task.await(:infinity) From f4bf44318789d3a2bc990ab8b17391558c0b9bc3 Mon Sep 17 00:00:00 2001 From: crbelaus Date: Wed, 12 Jun 2024 16:09:17 +0200 Subject: [PATCH 06/13] Typespecs --- lib/error_tracker.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/error_tracker.ex b/lib/error_tracker.ex index afae1da..f661275 100644 --- a/lib/error_tracker.ex +++ b/lib/error_tracker.ex @@ -6,7 +6,7 @@ defmodule ErrorTracker do @typedoc """ A map containing the relvant context for a particular error. """ - @type context :: map() + @type context :: %{String.t() => any()} def report(exception, stacktrace, given_context \\ %{}) do {:ok, stacktrace} = ErrorTracker.Stacktrace.new(stacktrace) From 92dca7da3fdd5b224cf66641879c128b8cb05457 Mon Sep 17 00:00:00 2001 From: crbelaus Date: Wed, 12 Jun 2024 17:56:55 +0200 Subject: [PATCH 07/13] Fix telemetry --- lib/error_tracker/integrations/oban.ex | 8 ++++- lib/error_tracker/integrations/phoenix.ex | 40 ++++++++++------------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/lib/error_tracker/integrations/oban.ex b/lib/error_tracker/integrations/oban.ex index f6c50a0..cc8e815 100644 --- a/lib/error_tracker/integrations/oban.ex +++ b/lib/error_tracker/integrations/oban.ex @@ -8,9 +8,15 @@ defmodule ErrorTracker.Integrations.Oban do modify anything on your application. """ + # https://hexdocs.pm/oban/Oban.Telemetry.html + @events [ + [:oban, :job, :start], + [:oban, :job, :exception] + ] + def attach do if Application.spec(:oban) do - :telemetry.attach(__MODULE__, [:oban, :job, :exception], &handle_event/4, :no_config) + :telemetry.attach_many(__MODULE__, @events, &__MODULE__.handle_event/4, :no_config) end end diff --git a/lib/error_tracker/integrations/phoenix.ex b/lib/error_tracker/integrations/phoenix.ex index 7e8a407..463dc42 100644 --- a/lib/error_tracker/integrations/phoenix.ex +++ b/lib/error_tracker/integrations/phoenix.ex @@ -10,36 +10,32 @@ defmodule ErrorTracker.Integrations.Phoenix do alias ErrorTracker.Integrations.Plug, as: PlugIntegration - @events %{ - [:phoenix, :router_dispatch, :exception] => &__MODULE__.handle_exception/4, - [:phoenix, :router_dispatch, :start] => &__MODULE__.add_context/4 - } + # https://hexdocs.pm/phoenix/Phoenix.Logger.html#module-instrumentation + @events [ + [:phoenix, :router_dispatch, :start], + [:phoenix, :router_dispatch, :exception] + ] def attach do if Application.spec(:phoenix) do - for {event, handler} <- @events, do: :telemetry.attach(__MODULE__, event, handler, []) + :telemetry.attach_many(__MODULE__, @events, &__MODULE__.handle_event/4, :no_config) end end - def handle_exception( - [:phoenix, :router_dispatch, :exception], - _measurements, - %{reason: %Plug.Conn.WrapperError{reason: reason, stack: stack}}, - _opts - ) do - PlugIntegration.report_error(reason, stack) + def handle_event([:phoenix, :router_dispatch, :start], _measurements, metadata, :no_config) do + PlugIntegration.set_context(metadata.conn) end - def handle_exception( - [:phoenix, :router_dispatch, :exception], - _measurements, - %{reason: reason, stacktrace: stack}, - _opts - ) do - PlugIntegration.report_error(reason, stack) - end + def handle_event([:phoenix, :router_dispatch, :exception], _measurements, metadata, :no_config) do + {reason, stack} = + case metadata do + %{reason: %Plug.Conn.WrapperError{reason: reason, stack: stack}} -> + {reason, stack} - def add_context([:phoenix, :router, :dispatch], _measurements, %{conn: conn}, _opts) do - PlugIntegration.set_context(conn) + %{reason: reason, stacktrace: stack} -> + {reason, stack} + end + + PlugIntegration.report_error(reason, stack) end end From 01e50df3942081dee2f0f0168e11107abc90079d Mon Sep 17 00:00:00 2001 From: crbelaus Date: Wed, 12 Jun 2024 18:11:51 +0200 Subject: [PATCH 08/13] Don't record session or params --- lib/error_tracker/integrations/plug.ex | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/error_tracker/integrations/plug.ex b/lib/error_tracker/integrations/plug.ex index bfef4fc..41d1b0c 100644 --- a/lib/error_tracker/integrations/plug.ex +++ b/lib/error_tracker/integrations/plug.ex @@ -86,15 +86,8 @@ defmodule ErrorTracker.Integrations.Plug do "request.query" => conn.query_string, "request.method" => conn.method, "request.ip" => remote_ip(conn), - "request.headers" => Map.new(conn.req_headers), - "response.headers" => Map.new(conn.resp_headers), - # TODO most likey this will be a %Plug.Conn.Unfetched{} - # Should we fetch it here? - "request.params" => %{}, - "request.session" => %{} + "request.headers" => Map.new(conn.req_headers) }) - - conn end defp remote_ip(conn = %Plug.Conn{}) do From d81ebab74d76366cce96be4e970e89343f8f3696 Mon Sep 17 00:00:00 2001 From: crbelaus Date: Wed, 12 Jun 2024 18:12:11 +0200 Subject: [PATCH 09/13] Update context for wrapped conn errors --- lib/error_tracker/integrations/plug.ex | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/error_tracker/integrations/plug.ex b/lib/error_tracker/integrations/plug.ex index 41d1b0c..f80fa21 100644 --- a/lib/error_tracker/integrations/plug.ex +++ b/lib/error_tracker/integrations/plug.ex @@ -50,7 +50,10 @@ defmodule ErrorTracker.Integrations.Plug do super(conn, opts) rescue e in Plug.Conn.WrapperError -> - unquote(__MODULE__).report_error(e, e.stack) + # This error wraps the failed connection so it may contain newer + # information for the context. + unquote(__MODULE__).set_context(e.conn) + unquote(__MODULE__).report_error(e.reason, e.stack) Plug.Conn.WrapperError.reraise(e) From a57e011c27fb36562a0f32383e55b0f2ce330b8d Mon Sep 17 00:00:00 2001 From: crbelaus Date: Wed, 12 Jun 2024 18:31:06 +0200 Subject: [PATCH 10/13] Include session and params when reporting errors --- lib/error_tracker/integrations/phoenix.ex | 2 +- lib/error_tracker/integrations/plug.ex | 23 ++++++++++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/error_tracker/integrations/phoenix.ex b/lib/error_tracker/integrations/phoenix.ex index 463dc42..1bb577d 100644 --- a/lib/error_tracker/integrations/phoenix.ex +++ b/lib/error_tracker/integrations/phoenix.ex @@ -36,6 +36,6 @@ defmodule ErrorTracker.Integrations.Phoenix do {reason, stack} end - PlugIntegration.report_error(reason, stack) + PlugIntegration.report_error(metadata.conn, reason, stack) end end diff --git a/lib/error_tracker/integrations/plug.ex b/lib/error_tracker/integrations/plug.ex index f80fa21..bff486f 100644 --- a/lib/error_tracker/integrations/plug.ex +++ b/lib/error_tracker/integrations/plug.ex @@ -53,29 +53,42 @@ defmodule ErrorTracker.Integrations.Plug do # This error wraps the failed connection so it may contain newer # information for the context. unquote(__MODULE__).set_context(e.conn) - unquote(__MODULE__).report_error(e.reason, e.stack) + unquote(__MODULE__).report_error(e.conn, e.reason, e.stack) Plug.Conn.WrapperError.reraise(e) e -> stack = __STACKTRACE__ - unquote(__MODULE__).report_error(e, stack) + unquote(__MODULE__).report_error(conn, e, stack) :erlang.raise(:error, e, stack) catch kind, reason -> stack = __STACKTRACE__ - unquote(__MODULE__).report_error(reason, stack) + unquote(__MODULE__).report_error(conn, reason, stack) :erlang.raise(kind, reason, stack) end end end - def report_error(reason, stack) do + def report_error(conn, reason, stack) do + context = + try do + %{"request.session" => Plug.Conn.get_session(conn)} + rescue + ArgumentError -> %{} + end + + context = + case conn.params do + %Plug.Conn.Unfetched{} -> context + fetched_params -> Map.put(context, "request.params", fetched_params) + end + unless Process.get(:error_tracker_router_exception_reported) do try do - ErrorTracker.report(reason, stack) + ErrorTracker.report(reason, stack, context) after Process.put(:error_tracker_router_exception_reported, true) end From 177334d64441e752674d6829b1d8cd08b3fbdb2c Mon Sep 17 00:00:00 2001 From: crbelaus Date: Wed, 12 Jun 2024 18:33:05 +0200 Subject: [PATCH 11/13] Fix typo --- lib/error_tracker.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/error_tracker.ex b/lib/error_tracker.ex index f661275..7f45232 100644 --- a/lib/error_tracker.ex +++ b/lib/error_tracker.ex @@ -4,7 +4,7 @@ defmodule ErrorTracker do """ @typedoc """ - A map containing the relvant context for a particular error. + A map containing the relevant context for a particular error. """ @type context :: %{String.t() => any()} From 49b15b38096a707cdc01b7cc9ddb1cf8bca88147 Mon Sep 17 00:00:00 2001 From: crbelaus Date: Wed, 12 Jun 2024 18:36:06 +0200 Subject: [PATCH 12/13] Ensure refreshed context when reporting Plug errors --- lib/error_tracker/integrations/plug.ex | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/error_tracker/integrations/plug.ex b/lib/error_tracker/integrations/plug.ex index bff486f..4f088d7 100644 --- a/lib/error_tracker/integrations/plug.ex +++ b/lib/error_tracker/integrations/plug.ex @@ -50,9 +50,6 @@ defmodule ErrorTracker.Integrations.Plug do super(conn, opts) rescue e in Plug.Conn.WrapperError -> - # This error wraps the failed connection so it may contain newer - # information for the context. - unquote(__MODULE__).set_context(e.conn) unquote(__MODULE__).report_error(e.conn, e.reason, e.stack) Plug.Conn.WrapperError.reraise(e) @@ -73,9 +70,11 @@ defmodule ErrorTracker.Integrations.Plug do end def report_error(conn, reason, stack) do + context = build_context(conn) + context = try do - %{"request.session" => Plug.Conn.get_session(conn)} + Map.put(context, "request.session", Plug.Conn.get_session(conn)) rescue ArgumentError -> %{} end @@ -96,14 +95,18 @@ defmodule ErrorTracker.Integrations.Plug do end def set_context(conn = %Plug.Conn{}) do - ErrorTracker.set_context(%{ + conn |> build_context |> ErrorTracker.set_context() + end + + defp build_context(conn = %Plug.Conn{}) do + %{ "request.host" => conn.host, "request.path" => conn.request_path, "request.query" => conn.query_string, "request.method" => conn.method, "request.ip" => remote_ip(conn), "request.headers" => Map.new(conn.req_headers) - }) + } end defp remote_ip(conn = %Plug.Conn{}) do From aaecebce463ec596381bf6e774082957fec45813 Mon Sep 17 00:00:00 2001 From: crbelaus Date: Sat, 15 Jun 2024 11:57:29 +0200 Subject: [PATCH 13/13] Don't track session --- lib/error_tracker/integrations/plug.ex | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/lib/error_tracker/integrations/plug.ex b/lib/error_tracker/integrations/plug.ex index 4f088d7..6c56bbd 100644 --- a/lib/error_tracker/integrations/plug.ex +++ b/lib/error_tracker/integrations/plug.ex @@ -70,24 +70,9 @@ defmodule ErrorTracker.Integrations.Plug do end def report_error(conn, reason, stack) do - context = build_context(conn) - - context = - try do - Map.put(context, "request.session", Plug.Conn.get_session(conn)) - rescue - ArgumentError -> %{} - end - - context = - case conn.params do - %Plug.Conn.Unfetched{} -> context - fetched_params -> Map.put(context, "request.params", fetched_params) - end - unless Process.get(:error_tracker_router_exception_reported) do try do - ErrorTracker.report(reason, stack, context) + ErrorTracker.report(reason, stack, build_context(conn)) after Process.put(:error_tracker_router_exception_reported, true) end @@ -105,7 +90,9 @@ defmodule ErrorTracker.Integrations.Plug do "request.query" => conn.query_string, "request.method" => conn.method, "request.ip" => remote_ip(conn), - "request.headers" => Map.new(conn.req_headers) + "request.headers" => Map.new(conn.req_headers), + # Depending on the error source, the request params may have not been fetched yet + "request.params" => unless(is_struct(conn.params, Plug.Conn.Unfetched), do: conn.params) } end