feat(prometheus): Add native /metrics endpoint - #2402
Conversation
|
Maybe an idea for improvement for broker with many queues an topics. Collect metrics periodically in the background and answer http request with the last collected result. This avoids a lot of preasure on the broker and keep getting results back to prometheus "quickly" available. In my experience is better to not not the most actual data but have at most actual data by providing broker stability. |
|
I like the idea @graben. I know for RabbitMQ, there's an internal metrics store that both their console and their prometheus plugin scrape from, so the plugin still has 0 load on idle, does that pattern fix what you're getting at? A couple of other parallel additions I might also just add in this vein:
|
|
Looking into the source a little more, it looks like everything is already just describing MBeans for metrics anyways. The reason RabbitMQ has that pattern is that those metrics need to be actually aggregated, ActiveMQ MBeans are already ready to use. So I'm not sure I see the savings that pre-caching metrics has over lazily updating a cache in response to a caller. I'll still add the streaming response and the lazy TTL cache, as those are definitely worthwhile (imo). |
|
|
||
| @Override | ||
| protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { | ||
| boolean perObject = request != null && "true".equalsIgnoreCase(request.getParameter("per_object")); |
There was a problem hiding this comment.
I prefer to use final as much as possible
|
|
||
| try { | ||
| writeMetrics(ManagementFactory.getPlatformMBeanServer(), writer, perObject); | ||
| } catch (Exception exception) { |
There was a problem hiding this comment.
I think we should log this exception too, not just silently drop. So we can know when and what failed
| ObjectName pattern = new ObjectName("org.apache.activemq:type=Broker,brokerName=*"); | ||
| Set<ObjectName> brokers = mBeanServer.queryNames(pattern, null); | ||
|
|
||
| for (MetricDefinition metric : BROKER_METRICS) { |
There was a problem hiding this comment.
Just as a curiosity, is there any practical difference between iterating the brokers inside the metrics loop or doing the other way around ?
for (ObjectName broker : brokers) {
for (MetricDefinition metric : BROKER_METRICS) {
...
}
}
Do we need to have the metrics grouped together ?
| return ((Number) value).doubleValue(); | ||
| } | ||
| } catch (Exception ignored) { | ||
| // Some attributes are not available on every ActiveMQ deployment (eg: bridge metrics) |
There was a problem hiding this comment.
Should we log a warning?
| if (value == null) { | ||
| return "unknown"; | ||
| } | ||
| return value.replace("\\", "\\\\").replace("\"", "\\\"").replace("\n", "\\n"); |
There was a problem hiding this comment.
Again, just my curiosity. If I have broker called "# TYPE" (or anything else that is meaningful for the prometheus format), it won't mess things up right?
| return value.replace("\\", "\\\\").replace("\"", "\\\"").replace("\n", "\\n"); | ||
| } | ||
|
|
||
| private static final class MetricDefinition { |
There was a problem hiding this comment.
+1 final classes, final everything!
3 participants
New activemq-prometheus module that adds a
/metricsendpoint to the broker that vends metrics in the Prometheus format without requiring any sort of open JMX port. The plugin relies on the existing Jetty auth systems and only scrapes MBean during a request, so it doesn't put any additional load on the broker unless someone is actually using it and users define their own Prometheus scrapers and monitoring setup.It depends only on the jakarta stuff jetty already does.
I've included an example grafana dashboard in this PR just as a way to get up an running quickly (./activemq-prometheus/src/main/resources/example-grafana-dashboard.json). This wouldn't be in the final PR, instead something like it would be uploaded to grafana.com/dashboards.
See Discussion: #2226
Questions for the community: