review fixes

config/sys.config

@@ -29,7 +29,7 @@
             request_timeout => 1000,
             %% Pool options, see gunner_pool:pool_opts()
             pool_opts => #{
-                event_handler => {bouncer_gunner_event_h, #{}},
+                event_handler => {bouncer_gunner_metrics_event_h, #{}},
                 cleanup_interval => 1000,
                 max_connection_load => 1,
                 max_connection_idle_age => 2000,

src/bouncer_gunner_metrics_event_h.erl

@@ -1,4 +1,4 @@
--module(bouncer_gunner_event_h).
+-module(bouncer_gunner_metrics_event_h).
 
 -include_lib("gunner/include/gunner_events.hrl").
 
@@ -10,12 +10,14 @@
 
 %% Internal types
 
--type state() :: gunner_event_h:state().
+-type state() :: #{timer_id() => timer_start_ts()}.
+
+-type timer_id() :: term().
+-type timer_start_ts() :: non_neg_integer().
 
 -type metric() :: how_are_you:metric().
 -type metrics() :: [metric()].
 -type metric_key() :: how_are_you:metric_key().
--type bin() :: {number(), Name :: binary()}.
 
 %%
 %% gunner_event_h behaviour
@@ -23,13 +25,14 @@
 
 -spec handle_event(gunner_event_h:event(), state()) -> state().
 handle_event(Event, State) ->
-    {Metrics, State1} = event_to_metrics(Event, State),
-    ok = push_metric(Metrics),
+    {TimerMeric, State1} = process_timers(Event, State),
+    ok = push_metrics(create_metric(Event) ++ TimerMeric),
     State1.
 
 %%
 %% Metrics
 %%
+
 -define(METRIC_KEY(Tag, Content), [gunner, Tag, Content]).
 -define(METRIC_KEY(Tag, Content, GroupID), [gunner, Tag, Content, group, encode_group(GroupID)]).
 
@@ -52,30 +55,22 @@ handle_event(Event, State) ->
     ?TIMER_KEY(connection_init, {ConnectionID, GroupID})
 ).
 
--define(RECORD_NAME(Record), element(1, Record)).
-
-event_to_metrics(Event, State) ->
-    {TimerMeric, State1} = process_timers(Event, State),
-    {create_metric(Event) ++ TimerMeric, State1}.
-
 process_timers(Event, State) ->
     case is_timed_event(Event) of
         {true, {start, TimerID}} ->
             {ok, State1} = start_timer(TimerID, State),
             {[], State1};
-        {true, {finish, TimerID}} ->
+        {true, {finish, TimerID, MetricID}} ->
             {ok, Elapsed, State1} = stop_timer(TimerID, State),
-            {[create_duration(event_to_duration_metric(Event), Elapsed)], State1};
+            {[create_duration(MetricID, Elapsed)], State1};
         false ->
             {[], State}
     end.
 
-event_to_duration_metric(Event) -> ?METRIC_DURATION(?RECORD_NAME(Event)).
-
 is_timed_event(#gunner_acquire_started_event{group_id = GroupID, client = ClientID}) ->
     {true, {start, ?TIMER_ACQUIRE(GroupID, ClientID)}};
 is_timed_event(#gunner_acquire_finished_event{group_id = GroupID, client = ClientID}) ->
-    {true, {finish, ?TIMER_ACQUIRE(GroupID, ClientID)}};
+    {true, {finish, ?TIMER_ACQUIRE(GroupID, ClientID), ?METRIC_DURATION(acquire)}};
 is_timed_event(#gunner_free_started_event{
     group_id = GroupID,
     client = ClientID,
@@ -87,11 +82,11 @@ is_timed_event(#gunner_free_finished_event{
     client = ClientID,
     connection = ConnectionID
 }) ->
-    {true, {finish, ?TIMER_FREE(ConnectionID, GroupID, ClientID)}};
+    {true, {finish, ?TIMER_FREE(ConnectionID, GroupID, ClientID), ?METRIC_DURATION(free)}};
 is_timed_event(#gunner_cleanup_started_event{}) ->
     {true, {start, ?TIMER_CLEANUP}};
 is_timed_event(#gunner_cleanup_finished_event{}) ->
-    {true, {finish, ?TIMER_CLEANUP}};
+    {true, {finish, ?TIMER_CLEANUP, ?METRIC_DURATION(cleanup)}};
 is_timed_event(
     #gunner_connection_init_started_event{
         group_id = GroupID,
@@ -105,7 +100,8 @@ is_timed_event(
         connection = ConnectionID
     }
 ) ->
-    {true, {finish, ?TIMER_CONNECTION_INIT(GroupID, ConnectionID)}};
+    {true,
+        {finish, ?TIMER_CONNECTION_INIT(GroupID, ConnectionID), ?METRIC_DURATION(connection_init)}};
 is_timed_event(_) ->
     false.
 
@@ -131,8 +127,8 @@ create_metric(#gunner_free_finished_event{group_id = GroupID}) ->
 create_metric(#gunner_free_error_event{}) ->
     [counter_inc([gunner, free, error])];
 %%
-create_metric(#gunner_cleanup_started_event{active_connections = Active}) ->
-    [create_gauge(?METRIC_CONNECTION_COUNT(active), Active)];
+create_metric(#gunner_cleanup_started_event{}) ->
+    [];
 create_metric(#gunner_cleanup_finished_event{active_connections = Active}) ->
     [create_gauge(?METRIC_CONNECTION_COUNT(active), Active)];
 %%
@@ -162,8 +158,8 @@ create_metric(#gunner_connection_down_event{group_id = GroupID}) ->
 %% Internal
 %%
 
-encode_group({Host, Port}) when is_tuple(Host) ->
-    [tuple_to_list(Host), Port];
+encode_group({IP, Port}) when is_tuple(IP) ->
+    [inet:ntoa(IP), Port];
 encode_group({Host, Port}) ->
     [Host, Port].
 
@@ -175,16 +171,14 @@ encode_result({error, {connection_failed, _Reason}}) ->
     connection_failed.
 
 start_timer(TimerKey, State) ->
-    Time = erlang:monotonic_time(millisecond),
-    Timers = maps:get(timers, State, #{}),
-    {ok, State#{timers => Timers#{TimerKey => Time}}}.
+    Time = erlang:monotonic_time(microsecond),
+    {ok, State#{TimerKey => Time}}.
 
 stop_timer(TimerKey, State) ->
-    Time = erlang:monotonic_time(millisecond),
-    Timers = maps:get(timers, State, #{}),
-    case maps:get(TimerKey, Timers, undefined) of
+    Time = erlang:monotonic_time(microsecond),
+    case maps:get(TimerKey, State, undefined) of
         TimeStarted when TimeStarted =/= undefined ->
-            {ok, Time - TimeStarted, State#{timers => maps:remove(TimerKey, Timers)}};
+            {ok, Time - TimeStarted, maps:remove(TimerKey, State)};
         undefined ->
             {error, no_timer}
     end.
@@ -193,13 +187,13 @@ stop_timer(TimerKey, State) ->
 %% Hay utils
 %%
 
--spec push_metric(metrics()) -> ok.
+-spec push_metrics(metrics()) -> ok.
 
-push_metric([]) ->
+push_metrics([]) ->
     ok;
-push_metric([M | Metrics]) ->
+push_metrics([M | Metrics]) ->
     ok = how_are_you:metric_push(M),
-    push_metric(Metrics).
+    push_metrics(Metrics).
 
 -spec counter_inc(metric_key()) -> metric().
 counter_inc(Key) ->
@@ -219,37 +213,46 @@ create_gauge(Key, Number) ->
 
 -spec create_duration(metric_key(), non_neg_integer()) -> metric().
 create_duration(KeyPrefix, Duration) ->
-    BinKey = build_bin_key(build_bins(), Duration),
+    BinKey = build_bin_key(Duration),
     how_are_you:metric_construct(counter, [KeyPrefix, BinKey], 1).
 
 %%
 
--spec build_bin_key(Bins :: [bin()], Value :: number()) -> metric_key().
-build_bin_key([{HeadValue, HeadName} | _Bins], Value) when HeadValue > Value ->
-    <<"less_than_", HeadName/binary>>;
-build_bin_key([{LastValue, LastName}], Value) when LastValue =< Value ->
-    <<"greater_than_", LastName/binary>>;
-build_bin_key([{LeftValue, LeftName}, {RightValue, RightName} | _Bins], Value) when
-    LeftValue =< Value andalso RightValue > Value
-->
-    <<"from_", LeftName/binary, "_to_", RightName/binary>>;
-build_bin_key([{HeadValue, _HeadName} | Bins], Value) when HeadValue =< Value ->
-    build_bin_key(Bins, Value).
+-define(_10US, "10μs").
+-define(_50US, "50μs").
+-define(_100US, "100μs").
+-define(_500US, "500μs").
+-define(_1MS, "1ms").
+-define(_5MS, "5ms").
+-define(_10MS, "10ms").
+-define(_25MS, "25ms").
+-define(_50MS, "50ms").
+-define(_100MS, "100ms").
+-define(_250MS, "250ms").
+-define(_500MS, "500ms").
+-define(_1S, "1s").
+-define(_5S, "5s").
 
--spec build_bins() -> [bin()].
-build_bins() ->
-    [
-        {1000, <<"1ms">>},
-        {5 * 1000, <<"5ms">>},
-        {10 * 1000, <<"10ms">>},
-        {25 * 1000, <<"25ms">>},
-        {50 * 1000, <<"50ms">>},
-        {100 * 1000, <<"100ms">>},
-        {250 * 1000, <<"250ms">>},
-        {500 * 1000, <<"500ms">>},
-        {1000 * 1000, <<"1s">>},
-        {10 * 1000 * 1000, <<"10s">>},
-        {30 * 1000 * 1000, <<"30s">>},
-        {60 * 1000 * 1000, <<"1m">>},
-        {5 * 60 * 1000 * 1000, <<"5m">>}
-    ].
+-define(BETWEEN(Bin1, Bin2), <<"from_", Bin1, "_to_", Bin2>>).
+-define(LT(Bin), <<"less_than_", Bin>>).
+-define(GT(Bin), <<"greater_than_", Bin>>).
+
+-spec build_bin_key(Value :: number()) -> metric_key().
+build_bin_key(Value) ->
+    if
+        Value < 10 -> ?LT(?_10US);
+        Value < 50 -> ?BETWEEN(?_10US, ?_50US);
+        Value < 100 -> ?BETWEEN(?_50US, ?_100US);
+        Value < 500 -> ?BETWEEN(?_100US, ?_100US);
+        Value < 1000 -> ?BETWEEN(?_500US, ?_1MS);
+        Value < 5 * 1000 -> ?BETWEEN(?_1MS, ?_5MS);
+        Value < 10 * 1000 -> ?BETWEEN(?_5MS, ?_10MS);
+        Value < 25 * 1000 -> ?BETWEEN(?_10MS, ?_25MS);
+        Value < 50 * 1000 -> ?BETWEEN(?_25MS, ?_50MS);
+        Value < 100 * 1000 -> ?BETWEEN(?_50MS, ?_100MS);
+        Value < 250 * 1000 -> ?BETWEEN(?_100MS, ?_250MS);
+        Value < 500 * 1000 -> ?BETWEEN(?_250MS, ?_500MS);
+        Value < 1000 * 1000 -> ?BETWEEN(?_500MS, ?_1S);
+        Value < 5 * 1000 * 1000 -> ?BETWEEN(?_1S, ?_5S);
+        true -> ?GT(?_5S)
+    end.

test/bouncer_audit_tests_SUITE.erl

@@ -274,7 +274,7 @@ start_bouncer(Env, C) ->
             {opa, #{
                 endpoint => ?OPA_ENDPOINT,
                 pool_opts => #{
-                    event_handler => {bouncer_gunner_event_h, #{}},
+                    event_handler => {bouncer_gunner_metrics_event_h, #{}},
                     connection_opts => #{
                         transport => tcp
                     }

test/bouncer_stub_tests_SUITE.erl

@@ -78,7 +78,7 @@ start_bouncer(Env, C) ->
             {opa, #{
                 endpoint => ?OPA_ENDPOINT,
                 pool_opts => #{
-                    event_handler => {bouncer_gunner_event_h, #{}},
+                    event_handler => {bouncer_gunner_metrics_event_h, #{}},
                     connection_opts => #{
                         transport => tcp
                     }

test/bouncer_tests_SUITE.erl

@@ -125,7 +125,7 @@ start_bouncer(Env, C) ->
                 endpoint => ?OPA_ENDPOINT_RESOLVE,
                 request_timeout => 1000,
                 pool_opts => #{
-                    event_handler => {bouncer_gunner_event_h, #{}},
+                    event_handler => {bouncer_gunner_metrics_event_h, #{}},
                     connection_opts => #{
                         transport => tcp
                     }