From 39520428eb568919b6caa6656b39a43f415da509 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 25 May 2026 11:58:44 +0200 Subject: [PATCH 1/4] Add requester field and enricher hook to controller metrics --- CHANGELOG.md | 5 ++ lib/zexbox/metrics.ex | 42 +++++++++++-- .../metrics/controller_series_enricher.ex | 60 +++++++++++++++++++ lib/zexbox/metrics/metric_handler.ex | 11 ++++ .../metrics/models/controller_series.ex | 4 ++ test/zexbox/metrics/metric_handler_test.exs | 52 ++++++++++++++++ test/zexbox/metrics_test.exs | 32 ++++++++++ 7 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 lib/zexbox/metrics/controller_series_enricher.ex diff --git a/CHANGELOG.md b/CHANGELOG.md index 72adff8..b01f14f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +- Add `requester` field to `Zexbox.Metrics.ControllerSeries` (not populated by default). +- Add `Zexbox.Metrics.ControllerSeriesEnricher` behaviour and an `:enricher` option on `Zexbox.Metrics.start_link/1` (also configurable via `config :zexbox, :metrics_enricher`) for setting tags/fields on the controller series from the `conn` before it is written. + ## 1.5.1 - 2026-02-05 - Handles cases where `$callers` and `$ancestors` may not be pids to avoid crashing metric handler. diff --git a/lib/zexbox/metrics.ex b/lib/zexbox/metrics.ex index 8006c01..83920f4 100644 --- a/lib/zexbox/metrics.ex +++ b/lib/zexbox/metrics.ex @@ -67,6 +67,17 @@ defmodule Zexbox.Metrics do @doc """ Starts the metrics supervisor and attaches the controller metrics. + Accepts an optional `:enricher` to customise the controller series before it + is written. See `Zexbox.Metrics.ControllerSeriesEnricher`. + + children = [ + {Zexbox.Metrics, enricher: {MyApp.MetricsEnricher, []}} + ] + + An enricher can also be set via application env + (`config :zexbox, :metrics_enricher, {MyApp.MetricsEnricher, []}`); the + `start_link/1` argument wins if both are present. + ## Examples iex> Zexbox.Metrics.start_link(nil) @@ -74,18 +85,41 @@ defmodule Zexbox.Metrics do """ @spec start_link(args :: any()) :: Supervisor.on_start() - def start_link(_args) do + def start_link(args) do on_start = Supervisor.start_link(__MODULE__, nil, name: __MODULE__) - attach_controller_metrics() + attach_controller_metrics(resolve_enricher(args)) on_start end - defp attach_controller_metrics do + defp resolve_enricher(args) do + args + |> enricher_from_args() + |> case do + nil -> Application.get_env(:zexbox, :metrics_enricher) + value -> value + end + |> normalise_enricher() + end + + defp enricher_from_args(args) when is_list(args), do: Keyword.get(args, :enricher) + defp enricher_from_args(_args), do: nil + + defp normalise_enricher(nil), do: nil + + defp normalise_enricher({module, opts}) when is_atom(module) do + {module, module.init(opts)} + end + + defp normalise_enricher(module) when is_atom(module) do + {module, module.init([])} + end + + defp attach_controller_metrics(enricher) do Telemetry.attach( "phoenix_controller_metrics", [:phoenix, :endpoint, :stop], &MetricHandler.handle_event/4, - nil + %{enricher: enricher} ) end end diff --git a/lib/zexbox/metrics/controller_series_enricher.ex b/lib/zexbox/metrics/controller_series_enricher.ex new file mode 100644 index 0000000..879e522 --- /dev/null +++ b/lib/zexbox/metrics/controller_series_enricher.ex @@ -0,0 +1,60 @@ +defmodule Zexbox.Metrics.ControllerSeriesEnricher do + @moduledoc """ + Behaviour for enriching a `Zexbox.Metrics.ControllerSeries` with values + derived from the request before it is written to InfluxDB. + + This is the extension point for setting tags and fields on the controller + series that `Zexbox` does not know how to populate itself — for example, + identifying the caller via an API key description, propagating a tenant + identifier, or attaching anything else that lives on the `conn`. + + ## Configuring an enricher + + Pass the enricher when starting `Zexbox.Metrics`: + + children = [ + {Zexbox.Metrics, enricher: {MyApp.MetricsEnricher, []}} + ] + + Or configure it via application env: + + config :zexbox, :metrics_enricher, {MyApp.MetricsEnricher, []} + + The form `MyApp.MetricsEnricher` (without options) is also accepted and is + equivalent to `{MyApp.MetricsEnricher, []}`. + + ## Implementing an enricher + + Read whatever you need off the `conn` — typically values placed there by an + earlier plug — and return a new series. The recommended pattern is to do any + expensive work (e.g. database lookups) in your auth plug and stash the result + on `conn.assigns`, so the enricher just reads it: + + defmodule MyApp.MetricsEnricher do + @behaviour Zexbox.Metrics.ControllerSeriesEnricher + + alias Zexbox.Metrics.ControllerSeries + + @impl true + def init(opts), do: opts + + @impl true + def call(series, conn, _opts) do + case conn.assigns[:api_key_description] do + nil -> series + description -> ControllerSeries.field(series, :requester, description) + end + end + end + + Enricher exceptions are caught by `Zexbox.Metrics.MetricHandler` and logged; + the un-enriched series is still written. Avoid blocking work inside `call/3` + — it runs in the request process. + """ + + alias Zexbox.Metrics.ControllerSeries + + @callback init(opts :: any()) :: any() + @callback call(series :: ControllerSeries.t(), conn :: map(), opts :: any()) :: + ControllerSeries.t() +end diff --git a/lib/zexbox/metrics/metric_handler.ex b/lib/zexbox/metrics/metric_handler.ex index 88609b3..9d325c5 100644 --- a/lib/zexbox/metrics/metric_handler.ex +++ b/lib/zexbox/metrics/metric_handler.ex @@ -21,6 +21,7 @@ defmodule Zexbox.Metrics.MetricHandler do false -> measurements |> create_controller_series(metadata) + |> enrich(metadata, config) |> write_metric(config) true -> @@ -32,6 +33,16 @@ defmodule Zexbox.Metrics.MetricHandler do Logger.error("Exception creating controller series: #{inspect(exception)}") end + defp enrich(series, %{conn: conn}, %{enricher: {module, opts}}) do + module.call(series, conn, opts) + rescue + exception -> + Logger.error("Exception in controller series enricher: #{inspect(exception)}") + series + end + + defp enrich(series, _metadata, _config), do: series + defp required_fields_missing?(%{conn: %{private: private}}) do format = Map.get(private, :phoenix_format) controller = Map.get(private, :phoenix_controller) diff --git a/lib/zexbox/metrics/models/controller_series.ex b/lib/zexbox/metrics/models/controller_series.ex index fec1f40..f164c6b 100644 --- a/lib/zexbox/metrics/models/controller_series.ex +++ b/lib/zexbox/metrics/models/controller_series.ex @@ -10,6 +10,9 @@ defmodule Zexbox.Metrics.ControllerSeries do * http_referer - The referer of the request * count - The number of requests * request_id - The request ID of the request + * requester - An identifier for the caller (e.g. an API key description or + upstream service name). Not populated by default — set it from a + `Zexbox.Metrics.ControllerSeriesEnricher`. The tags allowed are: @@ -37,6 +40,7 @@ defmodule Zexbox.Metrics.ControllerSeries do field(:trace_id) field(:count) field(:request_id) + field(:requester) end @doc """ diff --git a/test/zexbox/metrics/metric_handler_test.exs b/test/zexbox/metrics/metric_handler_test.exs index dd845d8..f185482 100644 --- a/test/zexbox/metrics/metric_handler_test.exs +++ b/test/zexbox/metrics/metric_handler_test.exs @@ -12,6 +12,28 @@ defmodule Zexbox.Metrics.MetricHandlerTest do end end + defmodule RequesterEnricher do + @behaviour Zexbox.Metrics.ControllerSeriesEnricher + + @impl true + def init(opts), do: opts + + @impl true + def call(series, conn, _opts) do + ControllerSeries.field(series, :requester, conn.assigns[:api_key_description]) + end + end + + defmodule BoomEnricher do + @behaviour Zexbox.Metrics.ControllerSeriesEnricher + + @impl true + def init(opts), do: opts + + @impl true + def call(_series, _conn, _opts), do: raise("boom") + end + describe "handle_event/4" do setup do start_supervised!(ContextRegistry) @@ -106,6 +128,36 @@ defmodule Zexbox.Metrics.MetricHandlerTest do assert log =~ "Exception creating controller series:" end + test "invokes the configured enricher and writes the enriched series", %{ + event: event, + measurements: measurements, + metadata: metadata, + config: config + } do + metadata = put_in(metadata.conn.assigns[:api_key_description], "service-A") + config = Map.put(config, :enricher, {RequesterEnricher, []}) + + assert %ControllerSeries{fields: %ControllerSeries.Fields{requester: "service-A"}} = + MetricHandler.handle_event(event, measurements, metadata, config) + end + + test "logs and falls back to the un-enriched series when the enricher raises", %{ + event: event, + measurements: measurements, + metadata: metadata, + config: config + } do + config = Map.put(config, :enricher, {BoomEnricher, []}) + + log = + capture_log(fn -> + assert %ControllerSeries{fields: %ControllerSeries.Fields{requester: nil}} = + MetricHandler.handle_event(event, measurements, metadata, config) + end) + + assert log =~ "Exception in controller series enricher:" + end + test "does not call Connection.write when process has disabled metrics", %{ event: event, measurements: measurements, diff --git a/test/zexbox/metrics_test.exs b/test/zexbox/metrics_test.exs index 8581c48..eec102c 100644 --- a/test/zexbox/metrics_test.exs +++ b/test/zexbox/metrics_test.exs @@ -2,11 +2,43 @@ defmodule Zexbox.MetricsTest do use ExUnit.Case alias Zexbox.Metrics + defmodule EchoEnricher do + @behaviour Zexbox.Metrics.ControllerSeriesEnricher + @impl true + def init(opts), do: {:initialised, opts} + @impl true + def call(series, _conn, _opts), do: series + end + + setup do + on_exit(fn -> + :telemetry.detach("phoenix_controller_metrics") + + case Process.whereis(Metrics) do + nil -> :ok + pid -> Process.exit(pid, :normal) + end + end) + + :ok + end + test "start_link/1 starts the metrics supervisor" do {:ok, pid} = Metrics.start_link(nil) assert Process.alive?(pid) end + test "start_link/1 normalises the enricher option through its init/1" do + {:ok, _pid} = Metrics.start_link(enricher: {EchoEnricher, [foo: :bar]}) + + handler = + [:phoenix, :endpoint, :stop] + |> :telemetry.list_handlers() + |> Enum.find(&(&1.id == "phoenix_controller_metrics")) + + assert handler.config == %{enricher: {EchoEnricher, {:initialised, [foo: :bar]}}} + end + test "init/1 initializes the metrics supervisor" do assert {:ok, {%{intensity: 3, period: 5, strategy: :one_for_one, auto_shutdown: :never}, From 549897550fefac0d3d8bc6b1cf7797a66218fae3 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 25 May 2026 12:01:43 +0200 Subject: [PATCH 2/4] Avoid piping into case in enricher resolution --- lib/zexbox/metrics.ex | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/zexbox/metrics.ex b/lib/zexbox/metrics.ex index 83920f4..7dbe578 100644 --- a/lib/zexbox/metrics.ex +++ b/lib/zexbox/metrics.ex @@ -92,13 +92,8 @@ defmodule Zexbox.Metrics do end defp resolve_enricher(args) do - args - |> enricher_from_args() - |> case do - nil -> Application.get_env(:zexbox, :metrics_enricher) - value -> value - end - |> normalise_enricher() + enricher = enricher_from_args(args) || Application.get_env(:zexbox, :metrics_enricher) + normalise_enricher(enricher) end defp enricher_from_args(args) when is_list(args), do: Keyword.get(args, :enricher) From b3620a0a7a5c132281858f50b4b465b8f9828fd2 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 25 May 2026 12:07:51 +0200 Subject: [PATCH 3/4] Stabilise metrics test cleanup against supervisor races --- test/zexbox/metrics_test.exs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/zexbox/metrics_test.exs b/test/zexbox/metrics_test.exs index eec102c..fb8de9e 100644 --- a/test/zexbox/metrics_test.exs +++ b/test/zexbox/metrics_test.exs @@ -14,9 +14,10 @@ defmodule Zexbox.MetricsTest do on_exit(fn -> :telemetry.detach("phoenix_controller_metrics") - case Process.whereis(Metrics) do - nil -> :ok - pid -> Process.exit(pid, :normal) + try do + Supervisor.stop(Metrics) + catch + :exit, _ -> :ok end end) From a9059c4d383ca30acb4a5ca6be99e48c373f2242 Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 25 May 2026 12:09:28 +0200 Subject: [PATCH 4/4] Name unused exit reason to satisfy credo --- test/zexbox/metrics_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/zexbox/metrics_test.exs b/test/zexbox/metrics_test.exs index fb8de9e..c75e543 100644 --- a/test/zexbox/metrics_test.exs +++ b/test/zexbox/metrics_test.exs @@ -17,7 +17,7 @@ defmodule Zexbox.MetricsTest do try do Supervisor.stop(Metrics) catch - :exit, _ -> :ok + :exit, _reason -> :ok end end)