Implement persistent term storage for Telemetry handlers

hauleth · hauleth · commit 3067d4acd816 · 2026-02-21T14:03:52.000+01:00
This adds new `telemetry:lock/0` function that allows locking telemetry
handlers to improve execution performance at cost of slowing down
attaching and detaching handlerrs.
diff --git a/src/telemetry.erl b/src/telemetry.erl
@@ -2,6 +2,7 @@
 
 -export([attach/4,
          attach_many/4,
+         persist/0,
          detach/1,
          list_handlers/1,
          execute/2,
@@ -142,6 +143,17 @@ attach_many(HandlerId, EventNames, Function, Config) when is_function(Function,
     end,
     telemetry_handler_table:insert(HandlerId, EventNames, Function, Config).
 
+?DOC("""
+Persist telemetry handlers.
+
+This will improve performance of calling Telemetry handlers at the cost of
+reducing performance of attaching or detaching new handlers.
+
+This function should be used with care.
+""").
+persist() ->
+    telemetry_handler_table:persist().
+
 ?DOC("""
 Removes the existing handler.
 
diff --git a/src/telemetry_ets.erl b/src/telemetry_ets.erl
@@ -0,0 +1,69 @@
+-module(telemetry_ets).
+
+-include("telemetry.hrl").
+
+-export([persist/1,
+         list_for_event/2,
+         list_by_prefix/2,
+         insert/5,
+         delete/2]).
+
+persist(TID) ->
+    Handlers = ets:tab2list(TID),
+    Map = lists:foldl(fun(Handler, Acc) ->
+                              maps:update_with(Handler#handler.event_name,
+                                               fun(L) -> [Handler | L] end,
+                                               [Handler],
+                                               Acc)
+                      end,
+                      #{},
+                      Handlers),
+    {ok, Map}.
+
+list_for_event(TID, EventName) ->
+    try
+        ets:lookup(TID, EventName)
+    catch
+        error:badarg ->
+            []
+    end.
+
+list_by_prefix(TID, EventPrefix) ->
+    Pattern = match_pattern_for_prefix(EventPrefix),
+    try
+        ets:match_object(TID, Pattern)
+    catch
+        error:badarg ->
+            []
+    end.
+
+match_pattern_for_prefix(EventPrefix) ->
+    #handler{event_name=match_for_prefix(EventPrefix),
+             _='_'}.
+
+-dialyzer({nowarn_function, match_for_prefix/1}).
+match_for_prefix([]) ->
+    '_';
+match_for_prefix([Segment | Rest]) ->
+    [Segment | match_for_prefix(Rest)].
+
+insert(TID, HandlerId, EventNames, Function, Config) ->
+    case ets:match(TID, #handler{id=HandlerId,
+                                 _='_'}) of
+        [] ->
+            Objects = [#handler{id=HandlerId,
+                                event_name=EventName,
+                                function=Function,
+                                config=Config} || EventName <- EventNames],
+            ets:insert(TID, Objects),
+            {ok, TID};
+        _ ->
+            {error, already_exists}
+    end.
+
+delete(TID, HandlerId) ->
+    case ets:select_delete(TID, [{#handler{id=HandlerId,
+                                           _='_'}, [], [true]}]) of
+        0 -> {error, not_found};
+        _ -> {ok, TID}
+    end.
diff --git a/src/telemetry_handler_table.erl b/src/telemetry_handler_table.erl
@@ -13,6 +13,7 @@
 -behaviour(gen_server).
 
 -export([start_link/0,
+         persist/0,
          insert/4,
          delete/1,
          list_for_event/1,
@@ -25,6 +26,8 @@
          code_change/3,
          terminate/2]).
 
+-compile({inline, [impl_get/0]}).
+
 -include("telemetry.hrl").
 
 start_link() ->
@@ -42,21 +45,23 @@ insert(HandlerId, EventNames, Function, Config) ->
 delete(HandlerId) ->
     gen_server:call(?MODULE, {delete, HandlerId}).
 
+persist() ->
+    {Mod, State} = impl_get(),
+    case Mod:persist(State) of
+        {ok, NewState} ->
+            persistent_term:put(telemetry, {telemetry_pt, NewState}),
+            ok;
+        _ ->
+            ok
+    end.
+
 impl_get() -> persistent_term:get(telemetry).
 
 -spec list_for_event(telemetry:event_name()) -> [#handler{}].
 list_for_event(EventName) ->
     case impl_get() of
-        {ets, TID} ->
-            try
-                ets:lookup(TID, EventName)
-            catch
-                error:badarg ->
-                    persistent_term:erase(telemetry),
-                    ?LOG_WARNING("Failed to lookup telemetry handlers. "
-                                 "Ensure the telemetry application has been started. ", []),
-                    []
-            end;
+        {Mod, State} ->
+            Mod:list_for_event(State, EventName);
         _ ->
             ?LOG_WARNING("Failed to lookup telemetry handlers. "
                          "Ensure the telemetry application has been started. ", []),
@@ -66,9 +71,8 @@ list_for_event(EventName) ->
 -spec list_by_prefix(telemetry:event_prefix()) -> [#handler{}].
 list_by_prefix(EventPrefix) ->
     case impl_get() of
-        {ets, TID} ->
-            Pattern = match_pattern_for_prefix(EventPrefix),
-            ets:match_object(TID, Pattern);
+        {Mod, State} ->
+            Mod:list_by_prefix(State, EventPrefix);
         _ ->
             ?LOG_WARNING("Failed to lookup telemetry handlers. "
                          "Ensure the telemetry application has been started. ", []),
@@ -78,36 +82,27 @@ list_by_prefix(EventPrefix) ->
 init([]) ->
     TID = create_table(),
 
-    persistent_term:put(telemetry, {ets, TID}),
+    persistent_term:put(telemetry, {telemetry_ets, TID}),
 
     {ok, []}.
 
 handle_call({insert, HandlerId, EventNames, Function, Config}, _From, State) ->
-    case impl_get() of
-        {ets, TID} ->
-            case ets:match(TID, #handler{id=HandlerId,
-                                         _='_'}) of
-                [] ->
-                    Objects = [#handler{id=HandlerId,
-                                        event_name=EventName,
-                                        function=Function,
-                                        config=Config} || EventName <- EventNames],
-                    ets:insert(TID, Objects),
-                    {reply, ok, State};
-                _ ->
-                    {reply, {error, already_exists}, State}
-            end
+    {Mod, MState} = impl_get(),
+    case Mod:insert(MState, HandlerId, EventNames, Function, Config) of
+        {ok, NewState} ->
+            persistent_term:put(telemetry, {Mod, NewState}),
+            {reply, ok, State};
+        {error, _} = Error ->
+            {reply, Error, State}
     end;
 handle_call({delete, HandlerId}, _From, State) ->
-    case impl_get() of
-        {ets, TID} ->
-            case ets:select_delete(TID, [{#handler{id=HandlerId,
-                                                   _='_'}, [], [true]}]) of
-                0 ->
-                    {reply, {error, not_found}, State};
-                _ ->
-                    {reply, ok, State}
-            end
+    {Mod, MState} = impl_get(),
+    case Mod:delete(MState, HandlerId) of
+        {ok, NewState} ->
+            persistent_term:put(telemetry, {Mod, NewState}),
+            {reply, ok, State};
+        {error, _} = Error ->
+            {reply, Error, State}
     end.
 
 handle_cast(_Msg, State) ->
@@ -126,15 +121,5 @@ terminate(_Reason, _State) ->
 %%
 
 create_table() ->
-    ets:new(?MODULE, [duplicate_bag, protected, named_table,
+    ets:new(?MODULE, [duplicate_bag, protected,
                       {keypos, #handler.event_name}, {read_concurrency, true}]).
-
-match_pattern_for_prefix(EventPrefix) ->
-    #handler{event_name=match_for_prefix(EventPrefix),
-             _='_'}.
-
--dialyzer({nowarn_function, match_for_prefix/1}).
-match_for_prefix([]) ->
-    '_';
-match_for_prefix([Segment | Rest]) ->
-    [Segment | match_for_prefix(Rest)].
diff --git a/src/telemetry_pt.erl b/src/telemetry_pt.erl
@@ -0,0 +1,62 @@
+-module(telemetry_pt).
+
+-include("telemetry.hrl").
+
+-export([persist/1,
+         list_for_event/2,
+         list_by_prefix/2,
+         insert/5,
+         delete/2]).
+
+persist(_Map) -> error.
+
+list_for_event(Map, EventName) ->
+    case Map of
+        #{EventName := Handlers} -> Handlers;
+        _ -> []
+    end.
+
+list_by_prefix(Map, EventPrefix) ->
+    [Handler ||
+     {EventName, Handlers} <- maps:to_list(Map),
+     starts_with(EventName, EventPrefix),
+     Handler <- Handlers].
+
+starts_with(_Haystack, []) -> true;
+starts_with([A | Haystack], [A | Needle]) -> starts_with(Haystack, Needle);
+starts_with(_Haystack, _Needle) -> false.
+
+insert(Map, _HandlerId, [], _Function, _Config) ->
+    {ok, Map};
+insert(Map, HandlerId, [EventName | Rest], Function, Config) ->
+    Handler = #handler{id=HandlerId,
+                       event_name=EventName,
+                       function=Function,
+                       config=Config},
+    OldHandlers = maps:get(EventName, Map, []),
+    case OldHandlers of
+        #{HandlerId := _} -> {error, already_exists};
+        _ ->
+            case put_new(Handler, OldHandlers) of
+                {ok, NewHandlers} ->
+                    NewMap = Map#{EventName => NewHandlers},
+                    insert(NewMap, HandlerId, Rest, Function, Config);
+                {error, _} = Error ->
+                    Error
+            end
+    end.
+
+put_new(Handler, List) ->
+    case lists:keymember(Handler#handler.id, #handler.id, List) of
+        true -> {error, already_exists};
+        false -> {ok, [Handler | List]}
+    end.
+
+delete(Map, HandlerId) ->
+    Filtered = [{Event, lists:keydelete(HandlerId, #handler.id, Handlers)}
+                || {Event, Handlers} <- maps:to_list(Map)],
+    NewMap = maps:from_list(Filtered),
+    case NewMap =:= Map of
+        true -> {error, not_found};
+        false -> {ok, NewMap}
+    end.
diff --git a/test/telemetry_SUITE.erl b/test/telemetry_SUITE.erl
@@ -7,30 +7,65 @@
 
 -include("telemetry.hrl").
 
-all() ->
-    [bad_event_names, duplicate_attach, invoke_handler,
+all() -> [persist_with_existing_handlers, {group, ets}, {group, persisted}].
+
+groups() ->
+    Tests = [bad_event_names, duplicate_attach, invoke_handler,
      list_handlers, list_for_prefix, detach_on_exception,
      no_execute_detached, no_execute_on_prefix, no_execute_on_specific,
      handler_on_multiple_events, remove_all_handler_on_failure,
      list_handler_on_many, detach_from_all, old_execute, default_metadata,
      off_execute, invoke_successful_span_handlers, invoke_exception_span_handlers,
-     spans_generate_unique_default_contexts, logs_on_local_function].
+     spans_generate_unique_default_contexts, logs_on_local_function],
+
+    [{ets, [], Tests}, {persisted, [], Tests}].
 
-init_per_suite(Config) ->
+init_per_group(Name, Config) ->
     application:ensure_all_started(telemetry),
+    case Name of
+        persisted -> ok = telemetry:persist();
+        _ -> ok
+    end,
     Config.
 
-end_per_suite(_Config) ->
+end_per_group(_, _Config) ->
     application:stop(telemetry).
 
 init_per_testcase(_, Config) ->
     HandlerId = crypto:strong_rand_bytes(16),
     [{id, HandlerId} | Config].
 
+end_per_testcase(persist_with_existing_handlers, _Config) ->
+    ok;
 end_per_testcase(_, Config) ->
     HandlerId = ?config(id, Config),
     telemetry:detach(HandlerId).
 
+persist_with_existing_handlers(Config) ->
+    application:ensure_all_started(telemetry),
+    HandlerId = ?config(id, Config),
+    Event = [a, test, event],
+    HandlerConfig = #{send_to => self()},
+    Measurements = #{data => 3},
+    Metadata = #{some => metadata},
+    telemetry:attach(HandlerId, Event, fun ?MODULE:echo_event/4, HandlerConfig),
+
+    telemetry:persist(),
+
+    telemetry:execute(Event, Measurements, Metadata),
+
+    try
+        receive
+            {event, Event, Measurements, Metadata, HandlerConfig} ->
+                ok
+        after
+            1000 ->
+                ct:fail(timeout_receive_echo)
+        end
+    after
+        application:stop(telemetry)
+    end.
+
 bad_event_names(Config) ->
     HandlerId = ?config(id, Config),
     ?assertError(badarg, telemetry:attach(HandlerId, ["some", event], fun ?MODULE:echo_event/4, [])),
