Add prometheus collector for delayed messages count - #17
Conversation
|
|
||
| -define(METRIC_NAME_PREFIX, "rabbitmq_detailed_"). | ||
|
|
||
| -define(METRIC_FAMILY_DELAYED_MESSAGES, delayed_messages_by_exchange). |
There was a problem hiding this comment.
should the family be something like delayed_exchange_metrics? like ra_metrics queue_coarse_metrics?
the-mikedavis
left a comment
the-mikedavis
left a comment
There was a problem hiding this comment.
I think there's some room to use seshat for counters here too. With rabbitmq/seshat#17 we could show the total number of delayed messages on the node, summed between all delayed exchanges.
| is_vhost_enabled(VHost, VHostsFilter) -> | ||
| ordsets:is_element(VHost, VHostsFilter). |
There was a problem hiding this comment.
Is ordsets from upstream? This ought to be a sets v2 these days, eh? It would probably be a tough breaking change to make.
There was a problem hiding this comment.
Is ordsets from upstream?
No, upstream builds a map, not an ordset, representing the vhost filter in prometheus_rabbitmq_core_metrics_collector:vhosts_filter_from_pdict . It does so to use the filter in some guards.
This ought to be a sets v2 these days, eh?
I used an ordset because AFAIR ordsets are the leanest type that is not a bare list. I think ordset, the upstream map, or a set v2 would be practically the same in our case, but ordset always feels the leanest option (smallest heap and no need to pass the v2 argument).
| lists:foldl( | ||
| fun(#resource{virtual_host = VHost} = XName, Acc) -> | ||
| case is_vhost_enabled(VHost, VHostsFilter) of | ||
| true -> | ||
| delayed_messages(XName, VHost, Acc); | ||
| false -> | ||
| Acc | ||
| end | ||
| end, | ||
| [], | ||
| rabbit_exchange:list_names()). | ||
|
|
||
| delayed_messages(#resource{name = Name} = XName, VHost, Acc) -> | ||
| case rabbit_exchange:lookup(XName) of | ||
| {ok, #exchange{type = ?EXCHANGE_TYPE} = X} -> | ||
| Count = rabbit_delayed_message:messages_delayed(X), |
There was a problem hiding this comment.
It would be handy to have a list_by_type in the server. Queue types already have helpers like that I believe. Maybe a nice contribution upstream!
@the-mikedavis A few thoughts around this idea. The seshat:fold function (rabbitmq/seshat#17) is still not available so we would need to wait for merge, release and rmq bumping the dep pin. Also, the current implementation enumerates live exchanges so we don't need any cleanup on exchange deletion. That would be needed if we used seshat counters. On the other hand, the internal structure seshat uses feels more correct than the one I implemented. One ETS table plus few (probably just one) persistent term entries rather than one persistent term entry per new exchange. This is relevant bc each persistent term write copies its index table iirc. Since I'm leaving today on vacations I'll commit one other thing I have almost ready (byte count in addition to message count), and let @gomoripeti make the final decision in my absence. 👍 |
3 participants
Proposed Changes
This PR provides a
prometheus_collectorfor the plugin exposing just one gauge metricrabbitmq_detailed_delayed_messagesunder thedelayed_messages_by_exchangefamily.The metric represents the number of delayed messages (still not routed out of the delayed exchange) per exchange and vhost.
The metric is not returned cluster-aggregated i.e. collecting the metric in a node returns the number of delayed messages in that node, not in the whole cluster. To get the total number of messages delayed in a cluster a user would need to aggregate all returned values for the N nodes of the cluster.
Regarding the relationship with the Prometheus dependency,
prometheusis set just as a build dependency so the plugin can be enabled without metrics.