Feuerlabs / exometer_core

Core components of exometer
Mozilla Public License 2.0
192 stars 121 forks source link

Dynamic subscribers MFA does not work #61

Open eproxus opened 8 years ago

eproxus commented 8 years ago

The following configuration works:

{exometer_core, [
  {predefined, {apply, stats, entries, []},
  {report, [
    {reporters, [
      {exometer_report_librato, [...]}
    ]},
    {subscribers, [
      {exometer_report_librato, [erlang, memory], [total, processes, processes_used, system, atom, atom_used, binary, code, ets], 5000},
    ]},
  ]}
]}.

Result:

1> exometer_report:list_subscriptions(exometer_report_librato).
[{[erlang,memory],
  [total,processes,processes_used,system,atom,atom_used,binary,code,ets],
  5000,undefined}]

The following does not:

{exometer_core, [
  {predefined, {apply, stats, entries, []},
  {report, [
    {reporters, [
      {exometer_report_librato, [...]}
    ]},
    {subscribers, [{apply, stats, subscribers, []}]}
  ]}
]}.

Result:

1> exometer_report:list_subscriptions(exometer_report_librato).
[]

Implementation of stats.erl:

-module(stats).

-export([entries/0]).
-export([subscribers/0]).

entries() ->
    {ok, [
        {
            [erlang, memory],
            {function, erlang, memory, [], proplist, [total, processes, processes_used, system, atom, atom_used, binary, code, ets]},
            []
        },
    ]}.

subscribers() ->
    [
        % Complete copy of what is in the sys.config
        {exometer_report_librato, [erlang, memory], [total, processes, processes_used, system, atom, atom_used, binary, code, ets], 5000}
    ].
tolbrino commented 8 years ago

@eproxus The MFA should be a tuple instead of simple values in the list. Try

{exometer_core, [
  {predefined, {apply, stats, entries, []},
  {report, [
    {reporters, [
      {exometer_report_librato, [...]}
    ]},
    {subscribers, [{apply, {stats, subscribers, []}}]}
  ]}
]}.
eproxus commented 8 years ago

Ok, that works. :smile:

Two things: in predefined entries, the format is {apply, M, F, A} which is confusing, and I think invalid subscription formats should be rejected (although in this case it matches the {Reporter, Metric, DataPoint, Interval} pattern, so I'm not sure what would be the best way).

Also, the documentation says directly after the correct description:

In the case of {apply, M, F, A}, the result of apply(M, F, A) must be a list of subscribers tuples.

tolbrino commented 8 years ago

It is confusing, I agree. I guess it would be best to change how apply works for predefined entries to be in sync with how it works for subscriptions.