Use metrics_mod stored in pool record in send_metric

The metrics module specified in app config is now stored as part of
the pool record. This allows send_metric in pooler.erl to determine
whether or not metrics are enabled by using data in the server state
rather than having to retrieve data from app config for each metric
send call. The only downside is duplicating the metrics module name
across all pools.

The default value is pooler_no_metric and will make calls to
send_metric a no-op.

src/pooler.erl

@@ -16,7 +16,6 @@
 
 
 %% type specs for pool metrics
--type metric_label() :: binary().
 -type metric_value() :: 'unknown_pid' |
                         non_neg_integer() |
                         {'add_pids_failed', non_neg_integer(), non_neg_integer()} |
@@ -272,7 +271,7 @@ add_pids(N, Pool) ->
                     error_logger:error_msg("pool '~s' tried to add ~B members, only added ~B~n",
                                            [PoolName, N, NewPidCount]),
                     %% consider changing this to a count?
-                    send_metric(pool_metric(PoolName, events),
+                    send_metric(Pool, events,
                                 {add_pids_failed, N, NewPidCount}, history)
             end,
             Pool1 = Pool#pool{free_pids = Free ++ NewPids,
@@ -285,19 +284,18 @@ add_pids(N, Pool) ->
 -spec take_member_from_pool(#pool{}, {pid(), term()},
                             non_neg_integer()) ->
                                    {error_no_members | pid(), #pool{}}.
-take_member_from_pool(#pool{name = PoolName,
-                            max_count = Max,
+take_member_from_pool(#pool{max_count = Max,
                             free_pids = Free,
                             in_use_count = NumInUse,
                             free_count = NumFree,
                             consumer_to_pid = CPMap} = Pool,
                       From,
                       Retries) ->
-    send_metric(pool_metric(PoolName, take_rate), 1, meter),
+    send_metric(Pool, take_rate, 1, meter),
     case Free of
         [] when NumInUse =:= Max ->
-            send_metric(pool_metric(PoolName, error_no_members_count), {inc, 1}, counter),
-            send_metric(pool_metric(PoolName, events), error_no_members, history),
+            send_metric(Pool, error_no_members_count, {inc, 1}, counter),
+            send_metric(Pool, events, error_no_members, history),
             {error_no_members, Pool};
         [] when NumInUse < Max andalso Retries > 0 ->
             case add_pids(1, Pool) of
@@ -305,21 +303,21 @@ take_member_from_pool(#pool{name = PoolName,
                     %% add_pids may have updated our pool
                     take_member_from_pool(Pool1, From, Retries - 1);
                 {max_count_reached, _} ->
-                    send_metric(pool_metric(PoolName, error_no_members_count), {inc, 1}, counter),
-                    send_metric(pool_metric(PoolName, events), error_no_members, history),
+                    send_metric(Pool, error_no_members_count, {inc, 1}, counter),
+                    send_metric(Pool, events, error_no_members, history),
                     {error_no_members, Pool}
             end;
         [] when Retries =:= 0 ->
             %% max retries reached
-            send_metric(pool_metric(PoolName, error_no_members_count), {inc, 1}, counter),
-            send_metric(pool_metric(PoolName, events), error_no_members, history),
+            send_metric(Pool, error_no_members_count, {inc, 1}, counter),
+            send_metric(Pool, events, error_no_members, history),
             {error_no_members, Pool};
         [Pid|Rest] ->
             erlang:link(From),
             Pool1 = Pool#pool{free_pids = Rest, in_use_count = NumInUse + 1,
                               free_count = NumFree - 1},
-            send_metric(pool_metric(PoolName, in_use_count), Pool1#pool.in_use_count, histogram),
-            send_metric(pool_metric(PoolName, free_count), Pool1#pool.free_count, histogram),
+            send_metric(Pool, in_use_count, Pool1#pool.in_use_count, histogram),
+            send_metric(Pool, free_count, Pool1#pool.free_count, histogram),
             {Pid, Pool1#pool{
                     consumer_to_pid = add_member_to_consumer(Pid, From, CPMap),
                     all_members = set_cpid_for_member(Pid, From,
@@ -348,7 +346,7 @@ do_return_member(Pid, fail, #pool{all_members = AllMembers} = Pool) ->
     % removed, so use find instead of fetch and ignore missing.
     clean_group_table(Pid, Pool),
     case dict:find(Pid, AllMembers) of
-        {ok, {PoolName, _, _}} ->
+        {ok, {_PoolName, _, _}} ->
             Pool1 = remove_pid(Pid, Pool),
             case add_pids(1, Pool1) of
                 {Status, Pool2} when Status =:= ok;
@@ -357,7 +355,7 @@ do_return_member(Pid, fail, #pool{all_members = AllMembers} = Pool) ->
                 {Status, _} ->
                     erlang:error({error, "unexpected return from add_pid",
                                   Status, erlang:get_stacktrace()}),
-                    send_metric(pool_metric(PoolName, events), bad_return_from_add_pid,
+                    send_metric(Pool1, events, bad_return_from_add_pid,
                                 history)
             end;
         error ->
@@ -411,18 +409,18 @@ remove_pid(Pid, Pool) ->
             NumFree = Pool#pool.free_count - 1,
             Pool1 = Pool#pool{free_pids = FreePids, free_count = NumFree},
             exit(Pid, kill),
-            send_metric(pool_metric(PoolName, killed_free_count), {inc, 1}, counter),
+            send_metric(Pool1, killed_free_count, {inc, 1}, counter),
             Pool1#pool{all_members = dict:erase(Pid, AllMembers)};
         {ok, {PoolName, CPid, _Time}} ->
             Pool1 = Pool#pool{in_use_count = Pool#pool.in_use_count - 1},
             exit(Pid, kill),
-            send_metric(pool_metric(PoolName, killed_in_use_count), {inc, 1}, counter),
+            send_metric(Pool1, killed_in_use_count, {inc, 1}, counter),
             Pool1#pool{consumer_to_pid = cpmap_remove(Pid, CPid, CPMap),
                        all_members = dict:erase(Pid, AllMembers)};
         error ->
             error_logger:error_report({{pool, PoolName}, unknown_pid, Pid,
                                        erlang:get_stacktrace()}),
-            send_metric(pool_metric(PoolName, events), unknown_pid, history),
+            send_metric(Pool, events, unknown_pid, history),
             Pool
     end.
 
@@ -520,16 +518,17 @@ expired_free_members(Members, Now, MaxAge) ->
     [ MI || MI = {_, {_, free, LastReturn}} <- Members,
             timer:now_diff(Now, LastReturn) >= MaxMicros ].
 
--spec send_metric(Name :: metric_label(),
-                  Value :: metric_value(),
-                  Type :: metric_type()) -> ok.
 %% Send a metric using the metrics module from application config or
 %% do nothing.
-send_metric(Name, Value, Type) ->
-    case application:get_env(pooler, metrics_module) of
-        undefined -> ok;
-        {ok, Mod} -> Mod:notify(Name, Value, Type)
-    end,
+-spec send_metric(Pool  :: #pool{},
+                  Label :: atom(),
+                  Value :: metric_value(),
+                  Type  :: metric_type()) -> ok.
+send_metric(#pool{metrics_mod = undefined}, _Label, _Value, _Type) ->
+    ok;
+send_metric(#pool{name = PoolName, metrics_mod = MetricsMod}, Label, Value, Type) ->
+    MetricName = pool_metric(PoolName, Label),
+    MetricsMod:notify(MetricName, Value, Type),
     ok.
 
 -spec pool_metric(atom(), atom()) -> binary().

src/pooler.hrl

@@ -35,7 +35,13 @@
 
           member_sup,
           all_members = dict:new()     :: dict(),
-          consumer_to_pid = dict:new() :: dict()
+          consumer_to_pid = dict:new() :: dict(),
+
+          %% The module to use for collecting metrics. If set to
+          %% 'pooler_no_metrics', then metric sending calls do
+          %% nothing. A typical value to actually capture metrics is
+          %% folsom_metrics.
+          metrics_mod = pooler_no_metrics :: atom()
          }).
 
 -define(gv(X, Y), proplists:get_value(X, Y)).

src/pooler_config.erl

@@ -18,8 +18,8 @@ list_to_pool(P) ->
        start_mfa         = ?gv(start_mfa, P),
        add_member_retry  = ?gv(add_member_retry, P, ?DEFAULT_ADD_RETRY),
        cull_interval     = ?gv(cull_interval, P, ?DEFAULT_CULL_INTERVAL),
-       max_age           = ?gv(max_age, P, ?DEFAULT_MAX_AGE)}.
+       max_age           = ?gv(max_age, P, ?DEFAULT_MAX_AGE),
+       metrics_mod       = ?gv(metrics_mod, P, pooler_no_metrics)}.
 
 %% TODO: consider adding some type checking logic for parsing the
 %% config to make config errors easier to track down.
-

src/pooler_sup.erl

@@ -12,7 +12,14 @@ start_link() ->
 init([]) ->
     %% a list of pool configs
     {ok, Config} = application:get_env(pooler, pools),
-    Pools = [ pooler_config:list_to_pool(L) || L <- Config ],
+    MetricsMod = case application:get_env(pooler, metrics_module) of
+                     {ok, Mod} ->
+                         Mod;
+                     undefined ->
+                         pooler_no_metrics
+                 end,
+    MetricsConfig = {metrics_mod, MetricsMod},
+    Pools = [ pooler_config:list_to_pool([MetricsConfig | L]) || L <- Config ],
     PoolSupSpecs = [ pool_sup_spec(Pool) || Pool <- Pools ],
     ets:new(?POOLER_GROUP_TABLE, [set, public, named_table, {write_concurrency, true}]),
     {ok, {{one_for_one, 5, 60}, PoolSupSpecs}}.