Merge pull request #4289 from esl/instrument/hooks

Instrument/hooks

big_tests/tests/service_domain_db_SUITE.erl

@@ -251,7 +251,7 @@ end_per_group(_GroupName, Config) ->
     end.
 
 init_per_testcase(rest_cannot_enable_deleting, Config) ->
-    HostType = <<"type1">>,
+    HostType = dummy_auth_host_type(),
     Server = start_domain_removal_hook(HostType),
     init_per_testcase(generic, [{server, Server}, {host_type, HostType} | Config]);
 init_per_testcase(db_crash_on_initial_load_restarts_service, Config) ->

src/ejabberd_app.erl

@@ -96,12 +96,12 @@ prep_stop(State) ->
     broadcast_c2s_shutdown_sup(),
     mongoose_wpool:stop(),
     mongoose_metrics:remove_all_metrics(),
-    mongoose_config:stop(),
     mongoose_graphql_commands:stop(),
     State.
 
 %% All the processes were killed when this function is called
 stop(_State) ->
+    mongoose_config:stop(),
     ?LOG_NOTICE(#{what => mongooseim_node_stopped, version => ?MONGOOSE_VERSION, node => node()}),
     delete_pid_file(),
     update_status_file(stopped),

src/gen_hook.erl

@@ -139,7 +139,7 @@ run_fold(HookName, Tag, Acc, Params) ->
     Key = hook_key(HookName, Tag),
     case persistent_term:get(?MODULE, #{}) of
         #{Key := Ls} ->
-            mongoose_metrics:increment_generic_hook_metric(Tag, HookName),
+            mongoose_instrument_hooks:execute(HookName, Tag),
             run_hook(Ls, Acc, Params, Key);
         _ ->
             {ok, Acc}
@@ -156,12 +156,12 @@ init([]) ->
     erlang:process_flag(trap_exit, true), %% We need to make sure that terminate is called in tests
     {ok, #{}}.
 
-handle_call({add_handler, Key, #hook_handler{} = HookHandler}, _From, State) ->
+handle_call({add_handler, Key = {Name, Tag}, #hook_handler{} = HookHandler}, _From, State) ->
     NewState =
         case maps:get(Key, State, []) of
             [] ->
                 NewLs = [HookHandler],
-                create_hook_metric(Key),
+                mongoose_instrument_hooks:set_up(Name, Tag),
                 maps:put(Key, NewLs, State);
             Ls ->
                 case lists:search(fun_is_handler_equal_to(HookHandler), Ls) of
@@ -178,7 +178,7 @@ handle_call({add_handler, Key, #hook_handler{} = HookHandler}, _From, State) ->
         end,
     maybe_insert_immediately(NewState),
     {reply, ok, NewState};
-handle_call({delete_handler, Key, #hook_handler{} = HookHandler}, _From, State) ->
+handle_call({delete_handler, Key = {Name, Tag}, #hook_handler{} = HookHandler}, _From, State) ->
     NewState =
         case maps:get(Key, State, []) of
             [] ->
@@ -190,6 +190,10 @@ handle_call({delete_handler, Key, #hook_handler{} = HookHandler}, _From, State)
                 %% by using `erlang:fun_info/2`
                 Pred = fun_is_handler_equal_to(HookHandler),
                 {_, NewLs} = lists:partition(Pred, Ls),
+                case NewLs of
+                    [] -> mongoose_instrument_hooks:tear_down(Name, Tag);
+                    _ -> ok
+                end,
                 maps:put(Key, NewLs, State)
         end,
     maybe_insert_immediately(NewState),
@@ -324,7 +328,3 @@ extend_extra({HookName, Tag, _Function, OriginalExtra, _Priority}) ->
     %% only the new keys from the ExtraExtension map will be added
     %% to the NewExtra map
     maps:merge(ExtraExtension, OriginalExtra).
-
--spec create_hook_metric(Key :: key()) -> any().
-create_hook_metric({HookName, Tag}) ->
-    mongoose_metrics:create_generic_hook_metric(Tag, HookName).

src/hooks/mongoose_hooks.erl

@@ -94,7 +94,6 @@
          mam_remove_archive/3,
          mam_lookup_messages/2,
          mam_archive_message/2,
-         mam_flush_messages/2,
          mam_archive_sync/1,
          mam_retraction/3]).
 
@@ -106,7 +105,6 @@
          mam_muc_remove_archive/3,
          mam_muc_lookup_messages/2,
          mam_muc_archive_message/2,
-         mam_muc_flush_messages/2,
          mam_muc_archive_sync/1,
          mam_muc_retraction/3]).
 
@@ -1005,14 +1003,6 @@ mam_lookup_messages(HostType, Params) ->
 mam_archive_message(HostType, Params) ->
     run_hook_for_host_type(mam_archive_message, HostType, ok, Params).
 
-%%% @doc The `mam_flush_messages' hook is run after the async bulk write
-%%% happens for messages despite the result of the write.
--spec mam_flush_messages(HostType :: mongooseim:host_type(),
-                         MessageCount :: integer()) -> ok.
-mam_flush_messages(HostType, MessageCount) ->
-    Params = #{count => MessageCount},
-    run_hook_for_host_type(mam_flush_messages, HostType, ok, Params).
-
 %% @doc Waits until all pending messages are written
 -spec mam_archive_sync(HostType :: mongooseim:host_type()) -> ok.
 mam_archive_sync(HostType) ->
@@ -1133,14 +1123,6 @@ mam_muc_lookup_messages(HostType, Params) ->
 mam_muc_archive_message(HostType, Params) ->
     run_hook_for_host_type(mam_muc_archive_message, HostType, ok, Params).
 
-%%% @doc The `mam_muc_flush_messages' hook is run after the async bulk write
-%%% happens for MUC messages despite the result of the write.
--spec mam_muc_flush_messages(HostType :: mongooseim:host_type(),
-                             MessageCount :: integer()) -> ok.
-mam_muc_flush_messages(HostType, MessageCount) ->
-    Params = #{count => MessageCount},
-    run_hook_for_host_type(mam_muc_flush_messages, HostType, ok, Params).
-
 %% @doc Waits until all pending messages are written
 -spec mam_muc_archive_sync(HostType :: mongooseim:host_type()) -> ok.
 mam_muc_archive_sync(HostType) ->

src/instrument/mongoose_instrument_hooks.erl

@@ -0,0 +1,95 @@
+-module(mongoose_instrument_hooks).
+
+-export([set_up/2, tear_down/2, execute/2]).
+
+-include("mongoose.hrl").
+
+-spec set_up(gen_hook:hook_name(), gen_hook:hook_tag()) -> ok.
+set_up(HookName, Tag) ->
+    case is_instrumented(HookName) of
+        true ->
+            EventName = event_name(HookName),
+            persistent_term:put({?MODULE, HookName}, EventName),
+            try
+                mongoose_instrument:set_up(EventName, labels(Tag), #{metrics => #{count => spiral}})
+            catch
+                error:#{what := inconsistent_labels} ->
+                    log_tag_error(HookName, Tag)
+            end;
+        false ->
+            ok
+    end.
+
+-spec log_tag_error(gen_hook:hook_name(), gen_hook:hook_tag()) -> ok.
+log_tag_error(HookName, Tag) ->
+    ?LOG_ERROR(#{what => inconsistent_hook_handler_tags,
+                 text => <<"A single hook should not have both global and host-type handlers. "
+                           "Instrumentation is not set up.">>,
+                 hook_name => HookName, tag => Tag}).
+
+-spec tear_down(gen_hook:hook_name(), gen_hook:hook_tag()) -> ok.
+tear_down(HookName, Tag) ->
+    case is_instrumented(HookName) of
+        true ->
+            EventName = event_name(HookName),
+            mongoose_instrument:tear_down(EventName, labels(Tag)),
+            persistent_term:erase({?MODULE, HookName});
+        false ->
+            ok
+    end.
+
+-spec execute(gen_hook:hook_name(), gen_hook:hook_tag()) -> ok.
+execute(HookName, Tag) ->
+    case is_instrumented(HookName) of
+        true ->
+            try persistent_term:get({?MODULE, HookName}) of
+                EventName ->
+                    mongoose_instrument:execute(EventName, labels(Tag), #{count => 1})
+            catch error:badarg ->
+                    ok % no handlers registered for this hook
+            end;
+        false ->
+            ok
+    end.
+
+-spec event_name(gen_hook:hook_name()) -> mongoose_instrument:event_name().
+event_name(HookName) ->
+    list_to_atom("hook_" ++ atom_to_list(HookName)).
+
+-spec labels(gen_hook:hook_tag()) -> mongoose_instrument:labels().
+labels(global) -> #{};
+labels(HostType) -> #{host_type => HostType}.
+
+-spec is_instrumented(gen_hook:hook_name()) -> boolean().
+is_instrumented(sm_register_connection) -> false;
+is_instrumented(sm_remove_connection) -> false;
+is_instrumented(auth_failed) -> false;
+is_instrumented(user_send_packet) -> false;
+is_instrumented(user_send_message) -> false;
+is_instrumented(user_send_presence) -> false;
+is_instrumented(user_send_iq) -> false;
+is_instrumented(user_receive_packet) -> false;
+is_instrumented(user_receive_message) -> false;
+is_instrumented(user_receive_presence) -> false;
+is_instrumented(user_receive_iq) -> false;
+is_instrumented(xmpp_bounce_message) -> false;
+is_instrumented(xmpp_stanza_dropped) -> false;
+is_instrumented(xmpp_send_element) -> false;
+is_instrumented(roster_get) -> false;
+is_instrumented(roster_set) -> false;
+is_instrumented(roster_push) -> false;
+is_instrumented(register_user) -> false;
+is_instrumented(remove_user) -> false;
+is_instrumented(privacy_iq_get) -> false;
+is_instrumented(privacy_iq_set) -> false;
+is_instrumented(privacy_check_packet) -> false;
+is_instrumented(mam_get_prefs) -> false;
+is_instrumented(mam_set_prefs) -> false;
+is_instrumented(mam_remove_archive) -> false;
+is_instrumented(mam_archive_message) -> false;
+is_instrumented(mam_muc_get_prefs) -> false;
+is_instrumented(mam_muc_set_prefs) -> false;
+is_instrumented(mam_muc_remove_archive) -> false;
+is_instrumented(mam_muc_lookup_messages) -> false;
+is_instrumented(mam_muc_archive_message) -> false;
+is_instrumented(_) -> true.

src/mam/mod_mam_muc_rdbms_arch_async.erl

@@ -81,7 +81,6 @@ do_flush_muc(Acc, #{host_type := HostType, queue_length := MessageCount,
             process_list_results(Process, HostType)
     end,
     [mod_mam_muc_rdbms_arch:retract_message(HostType, Params) || Params <- Acc],
-    mongoose_hooks:mam_muc_flush_messages(HostType, MessageCount),
     ok.
 
 process_batch_result({updated, _Count}, _, _, _) ->

src/mam/mod_mam_rdbms_arch_async.erl

@@ -109,7 +109,6 @@ do_flush_pm(Acc, #{host_type := HostType, queue_length := MessageCount,
             process_list_results(Process, HostType)
     end,
     [mod_mam_rdbms_arch:retract_message(HostType, Params) || Params <- Acc],
-    mongoose_hooks:mam_flush_messages(HostType, MessageCount),
     ok.
 
 process_batch_result({updated, _Count}, _, _, _) ->

src/metrics/mongoose_metrics.erl

@@ -21,7 +21,6 @@
 %% API
 -export([init/0,
          init_mongooseim_metrics/0,
-         create_generic_hook_metric/2,
          create_probe_metric/3,
          ensure_db_pool_metric/1,
          update/3,
@@ -34,7 +33,6 @@
          get_host_type_metric_names/1,
          get_global_metric_names/0,
          get_aggregated_values/1,
-         increment_generic_hook_metric/2,
          get_rdbms_data_stats/0,
          get_rdbms_data_stats/1,
          get_dist_data_stats/0,
@@ -53,8 +51,6 @@
 -define(PREFIXES, mongoose_metrics_prefixes).
 -define(DEFAULT_REPORT_INTERVAL, 60000). %%60s
 
--type use_or_skip() :: use | skip.
--type hook_name() :: atom().
 -type metric_name() :: atom() | list(atom() | binary()).
 -type short_metric_type() :: spiral | histogram | counter | gauge.
 -type metric_type() :: tuple() | short_metric_type().
@@ -85,12 +81,6 @@ init_subscriptions() ->
                 subscribe_to_all(Name, Interval)
         end, Reporters).
 
--spec create_generic_hook_metric(mongooseim:host_type(), atom()) ->
-    ok | {ok, already_present} | {error, any()}.
-create_generic_hook_metric(HostType, Hook) ->
-    UseOrSkip = filter_hook(Hook),
-    do_create_generic_hook_metric(HostType, Hook, UseOrSkip).
-
 -spec create_probe_metric(mongooseim:host_type_or_global(), atom(), module()) ->
     ok | {ok, already_present} | {error, any()}.
 create_probe_metric(HostType, Name, Module) ->
@@ -145,11 +135,6 @@ get_global_metric_names() ->
 get_aggregated_values(Metric) ->
     exometer:aggregate([{{['_', Metric], '_', '_'}, [], [true]}], [one, count, value]).
 
--spec increment_generic_hook_metric(mongooseim:host_type_or_global(), atom()) -> ok | {error, any()}.
-increment_generic_hook_metric(HostType, Hook) ->
-    UseOrSkip = filter_hook(Hook),
-    do_increment_generic_hook_metric(HostType, Hook, UseOrSkip).
-
 get_rdbms_data_stats() ->
     Pools = lists:filter(fun({Type, _Host, _Tag}) -> Type == rdbms end, mongoose_wpool:get_pools()),
     get_rdbms_data_stats(Pools).
@@ -236,24 +221,6 @@ get_report_interval() ->
     application:get_env(exometer_core, mongooseim_report_interval,
                         ?DEFAULT_REPORT_INTERVAL).
 
--spec do_create_generic_hook_metric(HostType :: mongooseim:host_type_or_global(),
-                                    Hook :: hook_name(),
-                                    UseOrSkip :: use_or_skip()) ->
-    ok | {ok, already_present} | {error, any()}.
-do_create_generic_hook_metric(_, _, skip) ->
-    ok;
-do_create_generic_hook_metric(HostType, Hook, use) ->
-    ensure_metric(HostType, Hook, spiral).
-
--spec do_increment_generic_hook_metric(HostType :: mongooseim:host_type_or_global(),
-                                       Hook :: hook_name(),
-                                       UseOrSkip :: use_or_skip()) ->
-    ok | {error, any()}.
-do_increment_generic_hook_metric(_, _, skip) ->
-    ok;
-do_increment_generic_hook_metric(HostType, Hook, use) ->
-    update(HostType, Hook, 1).
-
 get_rdbms_stats(RDBMSWorkers) ->
     RDBMSConnections = [{catch mongoose_rdbms:get_db_info(Pid), Pid} || Pid <- RDBMSWorkers],
     Ports = [get_port_from_rdbms_connection(Conn) || Conn <- RDBMSConnections],
@@ -353,43 +320,6 @@ inet_stats(Port) ->
 remove_metric({Name, _, _}) ->
     exometer_admin:delete_entry(Name).
 
-%% decided whether to use a metric for given hook or not
--spec filter_hook(hook_name()) -> use_or_skip().
-filter_hook(sm_register_connection) -> skip;
-filter_hook(sm_remove_connection) -> skip;
-filter_hook(auth_failed) -> skip;
-filter_hook(user_send_packet) -> skip;
-filter_hook(user_send_message) -> skip;
-filter_hook(user_send_presence) -> skip;
-filter_hook(user_send_iq) -> skip;
-filter_hook(user_receive_packet) -> skip;
-filter_hook(user_receive_message) -> skip;
-filter_hook(user_receive_presence) -> skip;
-filter_hook(user_receive_iq) -> skip;
-filter_hook(xmpp_bounce_message) -> skip;
-filter_hook(xmpp_stanza_dropped) -> skip;
-filter_hook(xmpp_send_element) -> skip;
-filter_hook(roster_get) -> skip;
-filter_hook(roster_set) -> skip;
-filter_hook(roster_push) -> skip;
-filter_hook(register_user) -> skip;
-filter_hook(remove_user) -> skip;
-filter_hook(privacy_iq_get) -> skip;
-filter_hook(privacy_iq_set) -> skip;
-filter_hook(privacy_check_packet) -> skip;
-filter_hook(mam_get_prefs) -> skip;
-filter_hook(mam_set_prefs) -> skip;
-filter_hook(mam_remove_archive) -> skip;
-filter_hook(mam_archive_message) -> skip;
-filter_hook(mam_muc_get_prefs) -> skip;
-filter_hook(mam_muc_set_prefs) -> skip;
-filter_hook(mam_muc_remove_archive) -> skip;
-filter_hook(mam_muc_lookup_messages) -> skip;
-filter_hook(mam_muc_archive_message) -> skip;
-filter_hook(mam_muc_flush_messages) -> skip;
-
-filter_hook(_) -> use.
-
 create_global_metrics(Metrics) ->
     lists:foreach(fun({Metric, Spec}) -> ensure_metric(global, Metric, Spec) end, Metrics).