From 576d5679c873b6e236b0f3bd3bbcef90a7a7f940 Mon Sep 17 00:00:00 2001 From: Mat Trudel Date: Sat, 4 Jan 2025 13:25:01 -0500 Subject: [PATCH] Add support for sending telemetry events on throws and exits (#443) --- lib/bandit/telemetry.ex | 15 ++++++------- test/bandit/http1/request_test.exs | 36 ++++++++++++++++++++++++++---- test/bandit/http2/plug_test.exs | 34 ++++++++++++++++++++++++---- 3 files changed, 69 insertions(+), 16 deletions(-) diff --git a/lib/bandit/telemetry.ex b/lib/bandit/telemetry.ex index fcb6fced..c9401895 100644 --- a/lib/bandit/telemetry.ex +++ b/lib/bandit/telemetry.ex @@ -80,7 +80,8 @@ defmodule Bandit.Telemetry do the conn * `plug`: The Plug which is being used to serve this request. Specified as `{plug_module, plug_opts}` * `kind`: The kind of unexpected condition, typically `:exit` - * `exception`: The exception which caused this unexpected termination + * `exception`: The exception which caused this unexpected termination. May be an exception + or an arbitrary value when the event was an uncaught throw or an exit * `stacktrace`: The stacktrace of the location which caused this unexpected termination ## `[:bandit, :websocket, *]` @@ -195,11 +196,13 @@ defmodule Bandit.Telemetry do @spec span_exception(t(), Exception.kind(), Exception.t() | term(), Exception.stacktrace()) :: :ok - def span_exception(span, :error, reason, stacktrace) when is_exception(reason) do + def span_exception(span, kind, reason, stacktrace) do + # Using :exit for backwards-compatibility with Bandit =< 1.5.7 + kind = if kind == :error, do: :exit, else: kind + metadata = Map.merge(span.start_metadata, %{ - # Using :exit for backwards-compatibility with Bandit =< 1.5.7 - kind: :exit, + kind: kind, exception: reason, stacktrace: stacktrace }) @@ -207,10 +210,6 @@ defmodule Bandit.Telemetry do span_event(span, :exception, %{}, metadata) end - def span_exception(_span, _kind, _reason, _stacktrace) do - :ok - end - @doc false @spec span_event(t(), span_name(), :telemetry.event_measurements(), :telemetry.event_metadata()) :: :ok diff --git a/test/bandit/http1/request_test.exs b/test/bandit/http1/request_test.exs index eeadab0f..561e952c 100644 --- a/test/bandit/http1/request_test.exs +++ b/test/bandit/http1/request_test.exs @@ -2330,10 +2330,24 @@ defmodule HTTP1RequestTest do end @tag :capture_log - test "it should not send `exception` events for throwing requests", context do + test "it should send `exception` events for throwing requests", context do Req.get!(context.req, url: "/uncaught_throw") - refute_receive {:telemetry, [:bandit, :request, :stop], _, _} + assert_receive {:telemetry, [:bandit, :request, :exception], measurements, metadata}, 500 + + assert measurements + ~> %{monotonic_time: integer()} + + assert metadata + ~> %{ + connection_telemetry_span_context: reference(), + telemetry_span_context: reference(), + conn: struct_like(Plug.Conn, []), + plug: {__MODULE__, []}, + kind: :throw, + exception: "thrown", + stacktrace: list() + } end def uncaught_throw(_conn) do @@ -2341,10 +2355,24 @@ defmodule HTTP1RequestTest do end @tag :capture_log - test "it should not send `exception` events for exiting requests", context do + test "it should send `exception` events for exiting requests", context do Req.get!(context.req, url: "/uncaught_exit") - refute_receive {:telemetry, [:bandit, :request, :exception], _, _} + assert_receive {:telemetry, [:bandit, :request, :exception], measurements, metadata}, 500 + + assert measurements + ~> %{monotonic_time: integer()} + + assert metadata + ~> %{ + connection_telemetry_span_context: reference(), + telemetry_span_context: reference(), + conn: struct_like(Plug.Conn, []), + plug: {__MODULE__, []}, + kind: :exit, + exception: "exited", + stacktrace: list() + } end def uncaught_exit(_conn) do diff --git a/test/bandit/http2/plug_test.exs b/test/bandit/http2/plug_test.exs index f78bd41e..9251a28c 100644 --- a/test/bandit/http2/plug_test.exs +++ b/test/bandit/http2/plug_test.exs @@ -1012,10 +1012,23 @@ defmodule HTTP2PlugTest do end @tag :capture_log - test "it should not send `exception` events for throwing requests", context do + test "it should send `exception` events for throwing requests", context do Req.get!(context.req, url: "/uncaught_throw") - refute_receive {:telemetry, [:bandit, :request, :exception], _, _} + assert_receive {:telemetry, [:bandit, :request, :exception], measurements, metadata}, 500 + + assert measurements ~> %{monotonic_time: integer()} + + assert metadata + ~> %{ + connection_telemetry_span_context: reference(), + telemetry_span_context: reference(), + conn: struct_like(Plug.Conn, []), + plug: {__MODULE__, []}, + kind: :throw, + exception: "thrown", + stacktrace: list() + } end def uncaught_throw(_conn) do @@ -1023,10 +1036,23 @@ defmodule HTTP2PlugTest do end @tag :capture_log - test "it should not send `exception` events for exiting requests", context do + test "it should send `exception` events for exiting requests", context do Req.get!(context.req, url: "/uncaught_exit") - refute_receive {:telemetry, [:bandit, :request, :exception], _, _} + assert_receive {:telemetry, [:bandit, :request, :exception], measurements, metadata}, 500 + + assert measurements ~> %{monotonic_time: integer()} + + assert metadata + ~> %{ + connection_telemetry_span_context: reference(), + telemetry_span_context: reference(), + conn: struct_like(Plug.Conn, []), + plug: {__MODULE__, []}, + kind: :exit, + exception: "exited", + stacktrace: list() + } end def uncaught_exit(_conn) do