Instrument/metric improvements

big_tests/tests/instrument_helper.erl

@@ -33,14 +33,16 @@ start(DeclaredEvents) ->
     mongoose_helper:inject_module(?HANDLER_MODULE),
     ets_helper:new(?STATUS_TABLE),
     [ets:insert(?STATUS_TABLE, {Event, untested}) || Event <- DeclaredEvents],
-    ok = rpc(mim(), ?HANDLER_MODULE, start, [DeclaredEvents]).
+    rpc(mim(), mongoose_instrument, add_handler,
+        [event_table, #{declared_events => DeclaredEvents}]).
 
 -spec stop() -> ok.
 stop() ->
     #{tested := Tested, untested := Untested} = classify_events(),
     ets_helper:delete(?STATUS_TABLE),
-    {ok, Logged} = rpc(mim(), ?HANDLER_MODULE, stop, []),
-    ct:log("Tested instrumentation events:~n ~p", [lists:sort(Tested)]),
+    Logged = rpc(mim(), ?HANDLER_MODULE, all_keys, []),
+    rpc(mim(), mongoose_instrument, remove_handler, [event_table]),
+    ct:log("Tested instrumentation events:~n~p", [lists:sort(Tested)]),
     verify_unlogged(Untested -- Logged),
     verify_logged_but_untested(Logged -- Tested).
 

big_tests/tests/mongoose_instrument_event_table.erl

@@ -6,20 +6,16 @@
 -define(TABLE, ?MODULE).
 
 %% API
--export([start/1, stop/0, get_events/2]).
+-export([get_events/2, all_keys/0]).
 
 %% mongoose_instrument callbacks
--export([set_up/3, handle_event/4]).
+-export([start/0, stop/0, set_up/3, handle_event/4]).
 
-start(DeclaredEvents) ->
-    ets_helper:new(?TABLE, [bag]), % repeating measurements are stored only once
-    mongoose_instrument:add_handler(event_table, #{declared_events => DeclaredEvents}).
+start() ->
+    ets_helper:new(?TABLE, [bag]). % repeating measurements are stored only once
 
 stop() ->
-    mongoose_instrument:remove_handler(event_table),
-    Logged = all_keys(?TABLE),
-    ets_helper:delete(?TABLE),
-    {ok, Logged}.
+    ets_helper:delete(?TABLE).
 
 set_up(EventName, Labels, _Config) ->
     DeclaredEvents = mongoose_config:get_opt([instrumentation, event_table, declared_events]),
@@ -31,6 +27,9 @@ handle_event(EventName, Labels, _Config, Measurements) ->
 get_events(EventName, Labels) ->
     ets:lookup(?TABLE, {EventName, Labels}).
 
+all_keys() ->
+    all_keys(?TABLE).
+
 all_keys(Tab) ->
     all_keys(Tab, ets:first(Tab), []).
 

src/instrument/mongoose_instrument.erl

@@ -23,7 +23,11 @@
 
 -type event_name() :: atom().
 -type labels() :: #{host_type => mongooseim:host_type()}. % to be extended
--type metrics() :: #{atom() => spiral | histogram}. % to be extended
+-type label_key() :: host_type. % to be extended
+-type label_value() :: mongooseim:host_type(). % to be extended
+-type metrics() :: #{metric_name() => metric_type()}.
+-type metric_name() :: atom().
+-type metric_type() :: spiral | histogram. % to be extended
 -type measurements() :: #{atom() => term()}.
 -type spec() :: {event_name(), labels(), config()}.
 -type config() :: #{metrics => metrics()}. % to be extended
@@ -34,12 +38,15 @@
 -type measure_fun(Result) :: fun((execution_time(), Result) -> measurements()).
 
 -callback config_spec() -> mongoose_config_spec:config_section().
+-callback start() -> ok.
+-callback stop() -> ok.
 -callback set_up(event_name(), labels(), config()) -> boolean().
 -callback handle_event(event_name(), labels(), config(), measurements()) -> any().
 
--optional_callbacks([config_spec/0]).
+-optional_callbacks([config_spec/0, start/0, stop/0]).
 
--export_type([event_name/0, labels/0, config/0, measurements/0, spec/0, handlers/0]).
+-export_type([event_name/0, labels/0, label_key/0, label_value/0, config/0, measurements/0,
+              spec/0, handlers/0, metric_name/0, metric_type/0]).
 
 %% API
 
@@ -138,6 +145,7 @@
 
 -spec init([]) -> {ok, state()}.
 init([]) ->
+    lists:foreach(fun start_handler/1, handler_modules()),
     erlang:process_flag(trap_exit, true), % Make sure that terminate is called
     persistent_term:erase(?MODULE), % Prevent inconsistency when restarted after a kill
     {ok, #{}}.
@@ -160,7 +168,9 @@
     case mongoose_config:lookup_opt([instrumentation, Key]) of
         {error, not_found} ->
             mongoose_config:set_opt([instrumentation, Key], ConfigOpts),
-            NewState = update_handlers(State, [], [handler_module(Key)]),
+            Module = handler_module(Key),
+            start_handler(Module),
+            NewState = update_handlers(State, [], [Module]),
             update_if_persisted(State, NewState),
             {reply, ok, NewState};
         {ok, ExistingConfig} ->
@@ -174,8 +184,10 @@
             {reply, {error, #{what => handler_not_configured, handler_key => Key}}, State};
         {ok, _} ->
             mongoose_config:unset_opt([instrumentation, Key]),
-            NewState = update_handlers(State, [handler_module(Key)], []),
+            Module = handler_module(Key),
+            NewState = update_handlers(State, [Module], []),
             update_if_persisted(State, NewState),
+            stop_handler(Module),
             {reply, ok, NewState}
     end;
 handle_call(persist, _From, State) ->
@@ -200,7 +212,7 @@
 -spec terminate(any(), state()) -> ok.
 terminate(_Reason, _State) ->
     persistent_term:erase(?MODULE),
-    ok.
+    lists:foreach(fun stop_handler/1, handler_modules()).
 
 -spec code_change(any(), state(), any()) -> {ok, state()}.
 code_change(_OldVsn, State, _Extra) ->
@@ -315,8 +327,10 @@
     end.
 
 -spec handle_event(event_name(), labels(), measurements(), handlers()) -> ok.
-handle_event(Event, Labels, Measurements, {EventHandlers, Config}) ->
-    lists:foreach(fun(Handler) -> Handler(Event, Labels, Config, Measurements) end, EventHandlers).
+handle_event(EventName, Labels, Measurements, {EventHandlers, Config}) ->
+    lists:foreach(fun(HandlerFun) ->
+                          call_handler(HandlerFun, EventName, Labels, Config, Measurements)
+                  end, EventHandlers).
 
 -spec modules_to_funs([module()]) -> [handler_fun()].
 modules_to_funs(Modules) ->
@@ -341,3 +355,30 @@
 -spec all_handler_keys() -> [handler_key()].
 all_handler_keys() ->
     [prometheus, exometer, log].
+
+-spec start_handler(module()) -> ok.
+start_handler(Module) ->
+    case mongoose_lib:is_exported(Module, start, 0) of
+        true -> Module:start();
+        false -> ok
+    end.
+
+-spec stop_handler(module()) -> ok.
+stop_handler(Module) ->
+    case mongoose_lib:is_exported(Module, stop, 0) of
+        true -> Module:stop();
+        false -> ok
+    end.
+
+-spec call_handler(handler_fun(), event_name(), labels(), config(), measurements()) -> any().
+call_handler(HandlerFun, EventName, Labels, Config, Measurements) ->
+    try
+        HandlerFun(EventName, Labels, Config, Measurements)
+    catch
+        Class:Reason:StackTrace ->
+            ?LOG_ERROR(#{what => event_handler_failed,
+                         handler_fun => HandlerFun,
+                         event_name => EventName, labels => Labels, config => Config,
+                         measurements => Measurements,
+                         class => Class, reason => Reason, stacktrace => StackTrace})
+    end.

src/instrument/mongoose_instrument_exometer.erl

@@ -2,7 +2,29 @@
 
 -behaviour(mongoose_instrument).
 
--export([set_up/3, handle_event/4]).
+-define(PREFIXES, {?MODULE, prefixes}).
+
+-export([config_spec/0, start/0, stop/0, set_up/3, handle_event/4]).
+
+-include("mongoose.hrl").
+-include("mongoose_config_spec.hrl").
+
+-spec config_spec() -> mongoose_config_spec:config_section().
+config_spec() ->
+    #section{items = #{<<"all_metrics_are_global">> => #option{type = boolean}},
+             defaults = #{<<"all_metrics_are_global">> => false}}.
+
+-spec start() -> ok.
+start() ->
+    AllGlobal = mongoose_config:get_opt([instrumentation, exometer, all_metrics_are_global]),
+    Prefixes = [{HostType, make_host_type_prefix(HostType, AllGlobal)}
+                || HostType <- ?ALL_HOST_TYPES],
+    persistent_term:put(?PREFIXES, maps:from_list(Prefixes)).
+
+-spec stop() -> ok.
+stop() ->
+    persistent_term:erase(?PREFIXES),
+    ok.
 
 -spec set_up(mongoose_instrument:event_name(), mongoose_instrument:labels(),
              mongoose_instrument:config()) -> boolean().
@@ -21,11 +43,19 @@ handle_event(EventName, Labels, #{metrics := Metrics}, Measurements) ->
                          handle_metric_event(EventName, Labels, MetricName, MetricType, Measurements)
                  end, Metrics).
 
+-spec set_up_metric(mongoose_instrument:event_name(), mongoose_instrument:labels(),
+                    mongoose_instrument:metric_name(), mongoose_instrument:metric_type()) ->
+          ok.
 set_up_metric(EventName, Labels, MetricName, MetricType) ->
-    %% TODO improve handling of already existing metrics
     Name = exometer_metric_name(EventName, Labels, MetricName),
-    catch exometer:new(Name, MetricType).
+    try exometer:new(Name, MetricType)
+    catch
+        error:exists -> ok = exometer:reset(Name)
+    end.
 
+-spec handle_metric_event(mongoose_instrument:event_name(), mongoose_instrument:labels(),
+                          mongoose_instrument:metric_name(), mongoose_instrument:metric_type(),
+                          mongoose_instrument:measurements()) -> ok.
 handle_metric_event(EventName, Labels, MetricName, MetricType, Measurements) ->
     case Measurements of
         #{MetricName := MetricValue} ->
@@ -35,11 +65,25 @@ handle_metric_event(EventName, Labels, MetricName, MetricType, Measurements) ->
             ok
     end.
 
+-spec update_metric(exometer:name(), spiral | histogram, integer()) -> ok.
 update_metric(Name, spiral, Value) when is_integer(Value), Value >= 0 ->
-    exometer:update(Name, Value);
+    ok = exometer:update(Name, Value);
 update_metric(Name, histogram, Value) when is_integer(Value) ->
-    exometer:update(Name, Value).
+    ok = exometer:update(Name, Value).
 
 %% This logic will need extending if we add more labels
+-spec exometer_metric_name(mongoose_instrument:event_name(), mongoose_instrument:labels(),
+                           mongoose_instrument:metric_name()) -> exometer:name().
 exometer_metric_name(EventName, #{host_type := HostType}, MetricName) ->
-    [mongoose_metrics:get_host_type_prefix(HostType), EventName, MetricName].
+    [get_host_type_prefix(HostType), EventName, MetricName].
+
+-spec get_host_type_prefix(mongooseim:host_type()) -> global | binary().
+get_host_type_prefix(HostType) ->
+    #{HostType := Prefix} = persistent_term:get(?PREFIXES),
+    Prefix.
+
+-spec make_host_type_prefix(mongooseim:host_type(), boolean()) -> global | binary().
+make_host_type_prefix(_HostType, true) ->
+    global;
+make_host_type_prefix(HostType, false) ->
+    binary:replace(HostType, <<" ">>, <<"_">>, [global]).

src/instrument/mongoose_instrument_prometheus.erl

@@ -4,12 +4,16 @@
 
 -export([set_up/3, handle_event/4]).
 
+%% Define Prometheus metric-related types, because the library has no type specs
+-type spec() :: proplists:proplist().
+-type name() :: string().
+-type help() :: string().
+
 -spec set_up(mongoose_instrument:event_name(), mongoose_instrument:labels(),
-            mongoose_instrument:config()) -> boolean().
+             mongoose_instrument:config()) -> boolean().
 set_up(EventName, Labels, #{metrics := Metrics}) ->
-    LabelKeys = labels_to_keys(Labels),
     maps:foreach(fun(MetricName, MetricType) ->
-                         set_up_metric(EventName, LabelKeys, MetricName, MetricType)
+                         set_up_metric(EventName, Labels, MetricName, MetricType)
                  end, Metrics),
     true;
 set_up(_EventName, _Labels, #{}) ->
@@ -23,20 +27,41 @@ handle_event(EventName, Labels, #{metrics := Metrics}, Measurements) ->
                          handle_metric_event(EventName, LabelValues, MetricName, MetricType, Measurements)
                  end, Metrics).
 
-set_up_metric(EventName, LabelKeys, MetricName, MetricType) ->
+-spec set_up_metric(mongoose_instrument:event_name(), mongoose_instrument:labels(),
+                    mongoose_instrument:metric_name(), mongoose_instrument:metric_type()) ->
+          ok.
+set_up_metric(EventName, Labels, MetricName, MetricType) ->
+    LabelKeys = labels_to_keys(Labels),
+    LabelValues = labels_to_values(Labels),
     MetricSpec = metric_spec(EventName, LabelKeys, MetricName),
-    declare_metric(MetricSpec, MetricType).
+    case declare_metric(MetricSpec, MetricType) of
+        true -> ok;
+        false -> reset_metric(proplists:get_value(name, MetricSpec), LabelValues, MetricType)
+    end.
 
+-spec declare_metric(proplists:proplist(), mongoose_instrument:metric_type()) -> boolean().
 declare_metric(MetricSpec, spiral) ->
     prometheus_counter:declare(MetricSpec);
 declare_metric(MetricSpec, histogram) ->
     prometheus_histogram:declare([{buckets, histogram_buckets()} | MetricSpec]).
 
+-spec reset_metric(name(), [mongoose_instrument:label_value()],
+                   mongoose_instrument:metric_type()) -> ok.
+reset_metric(Name, LabelValues, spiral) ->
+    prometheus_counter:reset(Name, LabelValues),
+    prometheus_counter:inc(Name, LabelValues, 0);
+reset_metric(Name, LabelValues, histogram) ->
+    prometheus_histogram:reset(Name, LabelValues),
+    ok.
+
+-spec metric_spec(mongoose_instrument:event_name(), [mongoose_instrument:label_key()],
+                  mongoose_instrument:metric_name()) -> spec().
 metric_spec(EventName, LabelKeys, MetricName) ->
     [{name, full_metric_name(EventName, MetricName)},
      {help, metric_help(EventName, MetricName)},
      {labels, LabelKeys}].
 
+-spec histogram_buckets() -> [integer()].
 histogram_buckets() ->
     histogram_buckets([], 1 bsl 30). % ~1.07 * 10^9
 
@@ -45,6 +70,9 @@ histogram_buckets(AccBuckets, Val) when Val > 0 ->
 histogram_buckets(AccBuckets, _Val) ->
     AccBuckets.
 
+-spec handle_metric_event(mongoose_instrument:event_name(), [mongoose_instrument:label_value()],
+                          mongoose_instrument:metric_name(), mongoose_instrument:metric_type(),
+                          mongoose_instrument:measurements()) -> ok.
 handle_metric_event(EventName, LabelValues, MetricName, MetricType, Measurements) ->
     case Measurements of
         #{MetricName := MetricValue} ->
@@ -54,19 +82,26 @@ handle_metric_event(EventName, LabelValues, MetricName, MetricType, Measurements
             ok
     end.
 
+-spec metric_help(mongoose_instrument:event_name(), mongoose_instrument:metric_name()) -> help().
 metric_help(EventName, MetricName) ->
     lists:flatten(io_lib:format("Event: ~p, Metric: ~p", [EventName, MetricName])).
 
+-spec full_metric_name(mongoose_instrument:event_name(), mongoose_instrument:metric_name()) ->
+          name().
 full_metric_name(EventName, MetricName) ->
-    list_to_atom(atom_to_list(EventName) ++ "_" ++ atom_to_list(MetricName)).
+    atom_to_list(EventName) ++ "_" ++ atom_to_list(MetricName).
 
+-spec labels_to_keys(mongoose_instrument:labels()) -> [mongoose_instrument:label_key()].
 labels_to_keys(Labels) ->
     lists:sort(maps:keys(Labels)).
 
+-spec labels_to_values(mongoose_instrument:labels()) -> [mongoose_instrument:label_value()].
 labels_to_values(Labels) ->
     [V || {_K, V} <- lists:keysort(1, maps:to_list(Labels))].
 
+-spec update_metric(name(), [mongoose_instrument:label_value()],
+                    mongoose_instrument:metric_type(), integer()) -> ok.
 update_metric(Name, Labels, spiral, Value) when is_integer(Value), Value >= 0 ->
-    prometheus_counter:inc(Name, Labels, Value);
+    ok = prometheus_counter:inc(Name, Labels, Value);
 update_metric(Name, Labels, histogram, Value) when is_integer(Value) ->
-    prometheus_histogram:observe(Name, Labels, Value).
+    ok = prometheus_histogram:observe(Name, Labels, Value).

src/metrics/mongoose_metrics.erl

@@ -42,8 +42,7 @@
          get_mnesia_running_db_nodes_count/0,
          remove_host_type_metrics/1,
          remove_all_metrics/0,
-         get_report_interval/0,
-         get_host_type_prefix/1
+         get_report_interval/0
         ]).
 
 -ignore_xref([get_dist_data_stats/0, get_mnesia_running_db_nodes_count/0,

test/common/config_parser_helper.erl

@@ -94,7 +94,7 @@ options("miscellaneous") ->
                                               secret => "Secret"
                                              }}}},
      {instrumentation, #{prometheus => #{},
-                         exometer => #{},
+                         exometer => #{all_metrics_are_global => true},
                          log => #{level => info}}},
      {{s2s, <<"anonymous.localhost">>}, default_s2s()},
      {{s2s, <<"localhost">>}, default_s2s()},
@@ -265,7 +265,7 @@ options("mongooseim-pgsql") ->
      {sm_backend, mnesia},
      {component_backend, mnesia},
      {s2s_backend, mnesia},
-     {instrumentation, #{exometer => #{},
+     {instrumentation, #{exometer => default_config([instrumentation, exometer]),
                          log => default_config([instrumentation, log])}},
      {{outgoing_pools, <<"anonymous.localhost">>},
       [host_pool_config(
@@ -1104,6 +1104,8 @@ extra_service_listener_config() ->
 
 default_config([instrumentation, log]) ->
     #{level => debug};
+default_config([instrumentation, exometer]) ->
+    #{all_metrics_are_global => false};
 default_config([instrumentation, _]) ->
     #{};
 default_config([listen, http]) ->