Skip to content

Add Prometheus exporter#890

Closed
stv0g wants to merge 4 commits into
masterfrom
add-prometheus-exporter
Closed

Add Prometheus exporter#890
stv0g wants to merge 4 commits into
masterfrom
add-prometheus-exporter

Conversation

@stv0g
Copy link
Copy Markdown
Contributor

@stv0g stv0g commented Apr 24, 2025

@stv0g stv0g added enhancement New feature or request api labels Apr 24, 2025
@stv0g stv0g self-assigned this Apr 24, 2025
@stv0g stv0g requested a review from n-eiling as a code owner April 24, 2025 06:07
SystemsPurge and others added 4 commits April 24, 2025 13:19
Signed-off-by: SystemsPurge <naktiyoussef@proton.me>
Signed-off-by: Steffen Vogel <post@steffenvogel.de>
Signed-off-by: Steffen Vogel <post@steffenvogel.de>
Signed-off-by: Steffen Vogel <post@steffenvogel.de>
@stv0g stv0g force-pushed the add-prometheus-exporter branch from 584c9e6 to 77e4815 Compare April 24, 2025 11:19
@stv0g
Copy link
Copy Markdown
Contributor Author

stv0g commented Apr 25, 2025

Hi @SystemsPurge,

while adding an integration test for your new API endpoint, I noticed that it only exposes a fraction of the available metrics.

Would you be open to extending it to all available metrics?

E.g., here is the code which prints the periodic stats to the console:

table->row(11, n->getNameShort().c_str(),

But this is still only a fraction, as most of the statistics are backed by a histogram:
https://github.com/VILLASframework/node/blob/master/common/lib/hist.cpp

Prometheus also has quite good support for histograms:
https://prometheus.io/docs/practices/histograms/

I think it would be pretty neat, if we could export the full histograms to Prometheus :)
That way we could render a nice RTT/One-way-delay distribution in Grafana :)

Here is an example which plots the histogram data even over time:

image

@SystemsPurge
Copy link
Copy Markdown
Contributor

@stv0g
I was hesitant about exporting histograms because it seemed a tiny bit innefficient, since you say it's ok i will do it.

@stv0g
Copy link
Copy Markdown
Contributor Author

stv0g commented Apr 25, 2025

Performance-wise I think it should be fine..

We might have to check if we can directly use the data from the histograms, because they are collecting their counts over the whole runtime, rather than a moving windows (e.g. last 5minutes).

We probably need to check, but I believe Prometheus expects the counts/bucket over a certain time frame / moving window..

@SystemsPurge
Copy link
Copy Markdown
Contributor

@stv0g With histograms specifically it does not seem to require a timestamp
However one possible piece of missing information is sum.
If my understanding is correct the histograms does not hold values associated with the buckets. One therefore only gets metrics in percentile form in this case.
It should be done by the way, you can take a look

@stv0g
Copy link
Copy Markdown
Contributor Author

stv0g commented Jun 24, 2025

Closed by #932

@stv0g stv0g closed this Jun 24, 2025
@stv0g stv0g deleted the add-prometheus-exporter branch August 21, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants