Skip to content

Commit

Permalink
fix: cleanup Peep metrics
Browse files Browse the repository at this point in the history
As we are depending on the undocumented internals of the Peep library,
we accidentally were broken by recent update in Peep where they have
added feature of stripped metrics storage. In this change there were no
more named ETS tables and there may be more than one ETS table in case
of striped tables. Now we are traversing all tables used by Peep to
cleanup them all (even though we are currently using regular store, but
this fix should work in case of striped tables as well).
  • Loading branch information
hauleth committed Nov 27, 2024
1 parent 9453cf8 commit 806d7f2
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 24 deletions.
15 changes: 10 additions & 5 deletions lib/supavisor/monitoring/prom_ex.ex
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,16 @@ defmodule Supavisor.Monitoring.PromEx do
search_path: search_path
}

Supavisor.Monitoring.PromEx.Metrics
|> :ets.select_delete([
{{{:_, meta}, :_}, [], [true]},
{{{:_, meta, :_}, :_}, [], [true]}
])
{_, tids} = Peep.Persistent.storage(Supavisor.Monitoring.PromEx.Metrics)

tids
|> List.wrap()
|> Enum.each(
&:ets.select_delete(&1, [
{{{:_, meta}, :_}, [], [true]},
{{{:_, meta, :_}, :_}, [], [true]}
])
)
end

@spec set_metrics_tags() :: map()
Expand Down
14 changes: 7 additions & 7 deletions test/integration/proxy_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ defmodule Supavisor.Integration.ProxyTest do
end

test "too many clients in session mode" do
Process.flag(:trap_exit, true)
db_conf = Application.get_env(:supavisor, Repo)
port = Application.get_env(:supavisor, :proxy_port_session)

Expand All @@ -166,8 +165,8 @@ defmodule Supavisor.Integration.ProxyTest do
port: port
)

{:ok, pid1} = single_connection(connection_opts)
{:ok, pid2} = single_connection(connection_opts)
assert {:ok, _} = single_connection(connection_opts)
assert {:ok, _} = single_connection(connection_opts)

:timer.sleep(1000)

Expand All @@ -182,8 +181,6 @@ defmodule Supavisor.Integration.ProxyTest do
pg_code: "XX000"
}
}} = single_connection(connection_opts)

for pid <- [pid1, pid2], do: :gen_statem.stop(pid)
end

test "http to proxy server returns 200 OK" do
Expand Down Expand Up @@ -379,15 +376,18 @@ defmodule Supavisor.Integration.ProxyTest do
defp single_connection(db_conf, c_port \\ nil) when is_list(db_conf) do
port = c_port || db_conf[:port]

[
opts = [
hostname: db_conf[:hostname],
port: port,
database: db_conf[:database],
password: db_conf[:password],
username: db_conf[:username],
pool_size: 1
]
|> SingleConnection.connect()

with {:error, {error, _}} <- start_supervised({SingleConnection, opts}) do
{:error, error}
end
end

defp parse_uri(uri) do
Expand Down
17 changes: 5 additions & 12 deletions test/supavisor/prom_ex_test.exs
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
defmodule Supavisor.PromExTest do
use ExUnit.Case, async: true
use Supavisor.DataCase
use Supavisor.DataCase, async: false

import Supavisor.Asserts

alias Supavisor.Monitoring.PromEx
@subject Supavisor.Monitoring.PromEx

@tenant "prom_tenant"

# These tests are known to be flaky, and while these do not affect users
# directly we can run them independently when needed. In future we probably
# should make them pass "regularly", but for now that is easier to filter them
# out.
@moduletag flaky: true

setup do
db_conf = Application.get_env(:supavisor, Repo)

Expand All @@ -34,12 +27,12 @@ defmodule Supavisor.PromExTest do
end

test "remove tenant tag upon termination", %{proxy: proxy, user: user, db_name: db_name} do
assert PromEx.get_metrics() =~ "tenant=\"#{@tenant}\""
assert @subject.get_metrics() =~ "tenant=\"#{@tenant}\""

:ok = GenServer.stop(proxy)
:ok = Supavisor.stop({{:single, @tenant}, user, :transaction, db_name, nil})

refute_eventually(10, fn -> PromEx.get_metrics() =~ "tenant=\"#{@tenant}\"" end)
refute_eventually(10, fn -> @subject.get_metrics() =~ "tenant=\"#{@tenant}\"" end)
end

test "clean_string/1 removes extra spaces from metric string" do
Expand All @@ -49,6 +42,6 @@ defmodule Supavisor.PromExTest do
expected_output =
"db_name=\"postgres\",mode=\"transaction\",tenant=\"dev_tenant\",type=\"single\",user=\"postgres\""

assert expected_output == PromEx.clean_string(input)
assert expected_output == @subject.clean_string(input)
end
end
8 changes: 8 additions & 0 deletions test/support/fixtures/single_connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ defmodule SingleConnection do

@behaviour P.SimpleConnection

def child_spec(conf) do
%{
id: {__MODULE__, System.unique_integer()},
start: {__MODULE__, :connect, [conf]},
restart: :temporary
}
end

def connect(conf) do
P.SimpleConnection.start_link(__MODULE__, [], conf)
end
Expand Down

0 comments on commit 806d7f2

Please sign in to comment.