graylog-labs / graylog-plugin-metrics-reporter

Graylog Metrics Reporter Plugins
https://www.graylog.org/
GNU General Public License v3.0
80 stars 18 forks source link

Export saner metrics names to prometheus #37

Closed jabbrwcky closed 2 years ago

jabbrwcky commented 5 years ago

The current plugin version embeds ids in the metric names, which leads to a proliferation of metrics names and makes aggregations more difficult than necessary:

# HELP org_graylog2_plugin_streams_Stream_000000000000000000000001_incomingMessages_1_sec_rate Generated from Dropwizard metric import (metric=org.graylog2.plugin.streams.Stream.000000000000000000000001.incomingMessages.1-sec-rate, type=org.graylog2.periodical.ThroughputCalculator$2)
# TYPE org_graylog2_plugin_streams_Stream_000000000000000000000001_incomingMessages_1_sec_rate gauge
org_graylog2_plugin_streams_Stream_000000000000000000000001_incomingMessages_1_sec_rate 950.0

The updated version updates the prometheus clients version and introduces a custom sample builder to build better metrics names:

# HELP org_graylog2_plugin_streams_Stream_incomingMessages Generated from Dropwizard metric import (metric=org.graylog2.plugin.streams.Stream.000000000000000000000001.incomingMessages, type=com.codahale.metrics.Meter)
# TYPE org_graylog2_plugin_streams_Stream_incomingMessages_1_sec_rate gauge
org_graylog2_plugin_streams_Stream_incomingMessages{id="000000000000000000000001",stream-title="All messages",index-set-id="5d6638d23b3af5000fad234f",} 64784.0

For all metrics that contain an Id in the metric name, the Id is extracted to a label; other metric names are not modified.

For steams and stream rules some additional information is extracted and provided as labels:

CLAassistant commented 5 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Jens Hausherr seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

dennisoelkers commented 5 years ago

Hey @jabbrwcky, thanks for the great contribution! We will test and review your PR asap, but it might take a little while as our load is pretty high at the moment.

jabbrwcky commented 5 years ago

@dennisoelkers That's fine, I'm not in a hurry. Just wanted to share the improvements we built for our installation.

kormotodor commented 5 years ago

Hi @jabbrwcky, extremely useful contrubution, thank you for the work! @dennisoelkers, @bernd is there are any information about timings of rewiewing/merging and so on? Think this PR would closes issue https://github.com/graylog-labs/graylog-plugin-metrics-reporter/issues/35 very well.

pschichtel commented 4 years ago

Is there anything preventing this from getting merged?

jabbrwcky commented 4 years ago

@pschichtel IDK, happy to see it merged eventually

jabbrwcky commented 4 years ago

Hi, is there anything hindering progress on this PR? It's been in 'ready for review' for some time now šŸ˜…

akhy commented 3 years ago

still no signs of merging? :(

fungusakafungus commented 3 years ago

I've found this: https://github.com/Graylog2/graylog2-server/pull/10767

Akhyar Amarullah @.***> schrieb am Fr., 18. Juni 2021, 08:02:

still no signs of merging? :(

ā€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/graylog-labs/graylog-plugin-metrics-reporter/pull/37#issuecomment-863779569, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABHBBRPYMAGM7GUTXMBOI3TTLOP3ANCNFSM4IQ7L7FQ .

bernd commented 3 years ago

@akhy As @fungusakafungus mentioned, we will have a built-in Prometheus exporter in the upcoming 4.1 release which uses a better naming scheme. We will have documentation for this up soon. Watch https://docs.graylog.org/en/4.1/pages/metrics.html (doesn't exist yet :wink:)

dennisoelkers commented 2 years ago

@jabbrwcky, @akhy: With the prometheus exporter in 4.1, is there still need for this PR or can we close it?

jabbrwcky commented 2 years ago

@dennisoelkers I am currently not working with Graylog due to change of positions, so I no longer have real stakes in this.

pschichtel commented 2 years ago

Is this MR still relevant given that graylog directly integrates it since (I believe) 4.1 ?

bernd commented 2 years ago

@pschichtel No, it can be closed. Thanks for the heads-up! :+1: