From b6dfa207d68fb31a19256f741ad5c2296782ef4e Mon Sep 17 00:00:00 2001 From: Richard Kallos Date: Tue, 24 Feb 2026 14:34:10 -0500 Subject: [PATCH] Remove Peep.get_metric/3 --- lib/peep.ex | 20 ---------- lib/peep/storage.ex | 5 --- lib/peep/storage/ets.ex | 27 ------------- lib/peep/storage/striped.ex | 70 ---------------------------------- mix.exs | 2 +- test/peep_test.exs | 16 ++++---- test/shared/storage_test.exs | 27 ++++++++----- test/shared/test_helpers.ex | 18 +++++++++ test/support/custom_storage.ex | 66 +++----------------------------- 9 files changed, 50 insertions(+), 201 deletions(-) create mode 100644 test/shared/test_helpers.ex diff --git a/lib/peep.ex b/lib/peep.ex index 2c44c43..b50bd87 100644 --- a/lib/peep.ex +++ b/lib/peep.ex @@ -149,26 +149,6 @@ defmodule Peep do end end - @doc """ - Fetches a single metric from storage. Currently only used in tests. - """ - def get_metric(name, metric, tags) when is_list(tags) do - get_metric(name, metric, Map.new(tags)) - end - - def get_metric(name, metric, tags) do - case Peep.Persistent.fetch(name) do - Peep.Persistent.persistent( - storage: {storage_mod, storage}, - metrics_to_ids: %{^metric => id} - ) -> - storage_mod.get_metric(storage, id, metric, tags) - - _ -> - nil - end - end - @doc """ Removes metrics whose metadata contains the specified tag patterns. diff --git a/lib/peep/storage.ex b/lib/peep/storage.ex index c0910d0..af5b27e 100644 --- a/lib/peep/storage.ex +++ b/lib/peep/storage.ex @@ -26,11 +26,6 @@ defmodule Peep.Storage do """ @callback get_all_metrics(term(), Peep.Persistent.t()) :: map() - @doc """ - Retrieves a single stored metric - """ - @callback get_metric(term(), Peep.metric_id(), Metrics.t(), map()) :: any() - @doc """ Removes metrics whose metadata contains a specific tag key and value. This is intended to improve situations where Peep emits metrics whose tags diff --git a/lib/peep/storage/ets.ex b/lib/peep/storage/ets.ex index 58bee82..38fd87f 100644 --- a/lib/peep/storage/ets.ex +++ b/lib/peep/storage/ets.ex @@ -89,33 +89,6 @@ defmodule Peep.Storage.ETS do |> group_metrics(Peep.Persistent.ids_to_metrics(persistent), %{}) end - @impl true - def get_metric(tid, id, %Metrics.Counter{}, tags) do - :ets.select(tid, [{{{id, :"$2", :_}, :"$1"}, [{:==, :"$2", tags}], [:"$1"]}]) - |> Enum.reduce(0, fn count, acc -> count + acc end) - end - - def get_metric(tid, id, %Metrics.Sum{}, tags) do - :ets.select(tid, [{{{id, :"$2", :_}, :"$1"}, [{:==, :"$2", tags}], [:"$1"]}]) - |> Enum.reduce(0, fn count, acc -> count + acc end) - end - - def get_metric(tid, id, %Metrics.LastValue{}, tags) do - case :ets.lookup(tid, {id, tags}) do - [{_key, value}] -> value - _ -> nil - end - end - - def get_metric(tid, id, %Metrics.Distribution{}, tags) do - key = {id, tags} - - case :ets.lookup(tid, key) do - [{_key, atomics}] -> Storage.Atomics.values(atomics) - _ -> nil - end - end - @impl true def prune_tags(tid, patterns) do match_spec = diff --git a/lib/peep/storage/striped.ex b/lib/peep/storage/striped.ex index a5013e0..5f28b0a 100644 --- a/lib/peep/storage/striped.ex +++ b/lib/peep/storage/striped.ex @@ -98,76 +98,6 @@ defmodule Peep.Storage.Striped do elem(tids, scheduler_id - 1) end - @impl true - def get_metric(tids, id, %Metrics.Counter{}, tags) do - key = {id, tags} - - for tid <- Tuple.to_list(tids), reduce: 0 do - acc -> - case :ets.lookup(tid, key) do - [] -> acc - [{_, value}] -> acc + value - end - end - end - - def get_metric(tids, id, %Metrics.Sum{}, tags) do - key = {id, tags} - - for tid <- Tuple.to_list(tids), reduce: 0 do - acc -> - case :ets.lookup(tid, key) do - [] -> acc - [{_, value}] -> acc + value - end - end - end - - def get_metric(tids, id, %Metrics.LastValue{}, tags) do - key = {id, tags} - - {_ts, value} = - for tid <- Tuple.to_list(tids), reduce: nil do - acc -> - case :ets.lookup(tid, key) do - [] -> - acc - - [{_, {_, _} = b}] -> - if acc do - max(acc, b) - else - b - end - end - end - - value - end - - def get_metric(tids, id, %Metrics.Distribution{}, tags) do - key = {id, tags} - - merge_fun = fn _k, v1, v2 -> v1 + v2 end - - for tid <- Tuple.to_list(tids), reduce: nil do - acc -> - case :ets.lookup(tid, key) do - [] -> - acc - - [{_key, atomics}] -> - values = Storage.Atomics.values(atomics) - - if acc do - Map.merge(acc, values, merge_fun) - else - values - end - end - end - end - @impl true def prune_tags(tids, patterns) do match_spec = diff --git a/mix.exs b/mix.exs index 2b77232..5161d2d 100644 --- a/mix.exs +++ b/mix.exs @@ -21,7 +21,7 @@ defmodule Peep.MixProject do ] end - defp elixirc_paths(:test), do: ["lib", "test/support"] + defp elixirc_paths(:test), do: ["lib", "test/support", "test/shared"] defp elixirc_paths(_), do: ["lib"] # Run "mix help compile.app" to learn about applications. diff --git a/test/peep_test.exs b/test/peep_test.exs index 8040938..333400d 100644 --- a/test/peep_test.exs +++ b/test/peep_test.exs @@ -72,10 +72,11 @@ defmodule PeepTest do :telemetry.execute([:gauge], %{value: 10}) :telemetry.execute([:dist], %{value: 15}) - assert Peep.get_metric(name, counter, tags) == 1 - assert Peep.get_metric(name, sum, tags) == 5 - assert Peep.get_metric(name, last_value, tags) == 10 - assert Peep.get_metric(name, distribution, tags).sum == 15 + all = Peep.get_all_metrics(name) + assert Peep.Test.get_metric(all, counter, tags) == 1 + assert Peep.Test.get_metric(all, sum, tags) == 5 + assert Peep.Test.get_metric(all, last_value, tags) == 10 + assert Peep.Test.get_metric(all, distribution, tags).sum == 15 end test "Peep process name can be used with Peep.Storage" do @@ -201,8 +202,9 @@ defmodule PeepTest do :telemetry.execute([name, :last_value], %{value: "bar"}) :telemetry.execute([name, :dist], %{value: []}) - assert Peep.get_metric(name, sum, []) == 0 - assert Peep.get_metric(name, last_value, []) == nil - assert Peep.get_metric(name, dist, []) == nil + all = Peep.get_all_metrics(name) + assert Peep.Test.get_metric(all, sum, []) == 0 + assert Peep.Test.get_metric(all, last_value, []) == nil + assert Peep.Test.get_metric(all, dist, []) == nil end end diff --git a/test/shared/storage_test.exs b/test/shared/storage_test.exs index e733e9c..2af5430 100644 --- a/test/shared/storage_test.exs +++ b/test/shared/storage_test.exs @@ -5,10 +5,11 @@ defmodule Peep.Storage.Test do ## Usage To test one or more Storage backends, set the `:test_storages` on the `:peep` - configuration and require this file, ie: + configuration and require this file and the test helper, ie: # your_storage_test.exs Application.put_env(:peep, :test_storages, [{CustomStorage, 3}, {OtherStorage, []}]) + Code.require_file "../deps/peep/test/shared/test_helpers.ex", __DIR__ Code.require_file "../deps/peep/test/shared/storage_test.exs", __DIR__ """ @@ -37,8 +38,9 @@ defmodule Peep.Storage.Test do 1..100 |> Enum.map(fn _ -> Task.async(f) end) |> Task.await_many() - assert Peep.get_metric(name, counter, %{}) == 1000 - assert Peep.get_metric(name, counter, %{even: true}) == 500 + all = Peep.get_all_metrics(name) + assert Peep.Test.get_metric(all, counter, %{}) == 1000 + assert Peep.Test.get_metric(all, counter, %{even: true}) == 500 end test "#{inspect(impl)} - a sum can be stored and retrieved" do @@ -58,8 +60,9 @@ defmodule Peep.Storage.Test do 1..100 |> Enum.map(fn _ -> Task.async(f) end) |> Task.await_many() - assert Peep.get_metric(name, sum, []) == 100 * 20 - assert Peep.get_metric(name, sum, even: true) == 100 * 15 + all = Peep.get_all_metrics(name) + assert Peep.Test.get_metric(all, sum, []) == 100 * 20 + assert Peep.Test.get_metric(all, sum, even: true) == 100 * 15 end test "#{inspect(impl)} - a last_value can be stored and retrieved" do @@ -79,8 +82,9 @@ defmodule Peep.Storage.Test do 1..100 |> Enum.map(fn _ -> Task.async(f) end) |> Task.await_many() - assert Peep.get_metric(name, last_value, []) == 10 - assert Peep.get_metric(name, last_value, odd: true) == 9 + all = Peep.get_all_metrics(name) + assert Peep.Test.get_metric(all, last_value, []) == 10 + assert Peep.Test.get_metric(all, last_value, odd: true) == 9 end test "#{inspect(impl)} - a distribution can be stored and retrieved" do @@ -138,7 +142,8 @@ defmodule Peep.Storage.Test do :sum => 100 * 2_001_000 } - assert Peep.get_metric(name, dist, []) == expected + all = Peep.get_all_metrics(name) + assert Peep.Test.get_metric(all, dist, []) == expected end test "#{inspect(impl)} - distribution bucket variability" do @@ -180,7 +185,8 @@ defmodule Peep.Storage.Test do :sum => 500_500 * 100 } - assert Peep.get_metric(name, dist, []) == expected + all = Peep.get_all_metrics(name) + assert Peep.Test.get_metric(all, dist, []) == expected end test "#{inspect(impl)} - default distribution handles negative values" do @@ -221,7 +227,8 @@ defmodule Peep.Storage.Test do :sum => 0 } - assert Peep.get_metric(name, dist, []) == expected + all = Peep.get_all_metrics(name) + assert Peep.Test.get_metric(all, dist, []) == expected end test "#{inspect(impl)} - storage_size/1" do diff --git a/test/shared/test_helpers.ex b/test/shared/test_helpers.ex new file mode 100644 index 0000000..881d67c --- /dev/null +++ b/test/shared/test_helpers.ex @@ -0,0 +1,18 @@ +defmodule Peep.Test do + @moduledoc false + alias Telemetry.Metrics + + def get_metric(all_metrics, metric, tags) do + tags = to_map(tags) + tags_map = Map.get(all_metrics, metric, %{}) + + case metric do + %Metrics.Counter{} -> Map.get(tags_map, tags, 0) + %Metrics.Sum{} -> Map.get(tags_map, tags, 0) + _other -> Map.get(tags_map, tags) + end + end + + defp to_map(tags) when is_map(tags), do: tags + defp to_map(tags) when is_list(tags), do: Map.new(tags) +end diff --git a/test/support/custom_storage.ex b/test/support/custom_storage.ex index 4b30777..3a82d7b 100644 --- a/test/support/custom_storage.ex +++ b/test/support/custom_storage.ex @@ -13,7 +13,6 @@ defmodule CustomStorage do alias Telemetry.Metrics alias Peep.Storage - @impl true @spec new(non_neg_integer) :: tuple def new(n_agents) do @@ -98,65 +97,6 @@ defmodule CustomStorage do |> group_metrics(itm, %{}) end - @impl true - def get_metric(agents, id, %Metrics.Counter{}, tags) do - key = {id, tags} - - for agent <- Tuple.to_list(agents), reduce: 0 do - acc -> acc + Agent.get(agent, &Map.get(&1, key, 0)) - end - end - - def get_metric(agents, id, %Metrics.Sum{}, tags) do - key = {id, tags} - - for agent <- Tuple.to_list(agents), reduce: 0 do - acc -> acc + Agent.get(agent, &Map.get(&1, key, 0)) - end - end - - def get_metric(agents, id, %Metrics.LastValue{}, tags) do - key = {id, tags} - - for agent <- Tuple.to_list(agents), reduce: nil do - acc -> - case Agent.get(agent, &Map.get(&1, key)) do - nil -> - acc - - value -> - if acc do - max(value, acc) - else - value - end - end - end - end - - def get_metric(agents, id, %Metrics.Distribution{}, tags) do - key = {id, tags} - - merge_fun = fn _k, v1, v2 -> v1 + v2 end - - for agent <- Tuple.to_list(agents), reduce: nil do - acc -> - case Agent.get(agent, &Map.get(&1, key)) do - nil -> - acc - - atomics -> - values = Storage.Atomics.values(atomics) - - if acc do - Map.merge(acc, values, merge_fun) - else - values - end - end - end - end - @impl true def prune_tags(agents, patterns) do agents @@ -211,7 +151,11 @@ defmodule CustomStorage do defp group_metric({{id, tags}, Storage.Atomics.atomic() = atomics}, itm, acc) do %{^id => metric} = itm - put_in(acc, [Access.key(metric, %{}), Access.key(tags)], Storage.Atomics.values(atomics)) + values = Storage.Atomics.values(atomics) + + update_in(acc, [Access.key(metric, %{}), Access.key(tags, %{})], fn m1 -> + Map.merge(m1, values, fn _k, v1, v2 -> v1 + v2 end) + end) end defp group_metric({{id, tags}, value}, itm, acc) do