Prometheus exporter without external dependencies, exporting histograms directly by SystemsPurge · Pull Request #921 · VILLASframework/node

SystemsPurge · 2025-06-10T09:22:59Z

It's not clear to me where i can find the values associated with the buckets of the histograms

stv0g

Looks good to me so far. I only have some minor comments.

Please also rebase your changes to the last master commit.

Also run clang-format on your changes, as code-formatting is now checked by the CI (you can also use pre-commit locally).

stv0g · 2025-06-12T05:00:27Z

  // Print ASCII style plot of histogram.
  void plot(Logger logger) const;

+  void test();


stv0g · 2025-06-12T05:01:27Z

+#include<string>
 #include <jansson.h>


Suggested change

#include<string>

#include <jansson.h>

#include<string>

#include <jansson.h>

We usually try to group headers into these sections separated by an empty line

libstdc++

libc

third-party

VILLASnode

You are also missing a space after in include<string>.

@stv0g this grouping is not part of the contributor guidelines, so you shouldn't enforce it :)

Okay, Its also something which we will fix repo-wide with the new clang-tidy fixes Philipp is working on..

I did not mention the missing space, as it will be fixed by clang-format.

stv0g · 2025-06-12T05:03:08Z

  // Write the histogram in JSON format to file \p f.
  int dumpJson(FILE *f) const;

+  std::string promFormat(std::string metric_name, std::string node_name) const;


Suggested change

std::string promFormat(std::string metric_name, std::string node_name) const;

std::string toPrometheusText(std::string metric_name, std::string node_name) const;

To be in-line with toJson and also be prepared for possible binary Promtheus format.

stv0g · 2025-06-12T05:03:34Z


+std::string Hist::promFormat(std::string metric_name, std::string node_name) const{
+  std::string base = "#TYPE HISTOGRAM "+metric_name;
+  //need this because prometheus understands quantiles 


Suggested change

//need this because prometheus understands quantiles

// Needed because Prometheus understands quantiles.

stv0g · 2025-06-12T05:03:50Z

  }
 }

+std::string Hist::promFormat(std::string metric_name, std::string node_name) const{


Suggested change

std::string Hist::promFormat(std::string metric_name, std::string node_name) const{

std::string Hist::toPrometheusText(std::string metric_name, std::string node_name) const{

stv0g · 2025-06-12T05:04:38Z

+      throw InvalidMethod(this);
+
+    if (body != nullptr)
+      throw BadRequest("Nodes endpoint does not accept any body data");


Suggested change

throw BadRequest("Nodes endpoint does not accept any body data");

throw BadRequest("The metrics endpoint does not accept any body data");

stv0g · 2025-06-12T05:05:48Z

+// Register API request
+static char n[] = "metrics";
+static char r[] = "/metrics";
+static char d[] = "Get stats of all nodes in desired format";


Suggested change

static char d[] = "Get stats of all nodes in desired format";

static char d[] = "Get stats of all nodes in Prometheus metrics format";

stv0g · 2025-06-12T05:06:17Z

+    if (body != nullptr)
+      throw BadRequest("Nodes endpoint does not accept any body data");
+
+    std::string text_res = "";


Please use an std::stringstream.

stv0g · 2025-06-12T05:06:57Z

@@ -0,0 +1,68 @@
+/* The "nodes" API ressource.
+ *
+ * Author: Steffen Vogel <post@steffenvogel.de>


Please put your name here :)

stv0g · 2025-06-12T05:07:52Z

+
+class MetricsRequest : public Request {
+private:
+std::unordered_map<Stats::Metric,std::string> metrics_subset = {


I am wondering: is the a reasoning behind only exporting a subset of statistics?

stv0g · 2025-06-12T10:57:32Z

@SystemsPurge I noticed this PR is against the add-prometheus-exporter branch. Is this intended? There seems to be a merge conflict.

Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

…an bucket values Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

…nction name and remove useless code. Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

…an bucket values Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

SystemsPurge · 2025-06-16T09:41:15Z

@stv0g i just was not sure i wanted to push to this branch directly, but you are welcome to change the target branch of PR

stv0g · 2025-06-17T08:59:31Z

Hi @SystemsPurge,

I've adjusted the base branch of this PR. If you dont mind, I would do some simple cleanups and make the Git history pretty before merging it. I will for sure maintain you as the authors of the changes.

SystemsPurge · 2025-06-17T10:19:44Z

@stv0g Do whatever you deem fit. I don't even really care about the crediting but thank you insisting. Do you have write access to the branch or do i have to give it explicitely ?

stv0g · 2025-06-24T09:08:58Z

Closing in favour of #932

SystemsPurge requested review from n-eiling and stv0g as code owners June 10, 2025 09:22

stv0g requested changes Jun 12, 2025

View reviewed changes

SystemsPurge added 8 commits June 16, 2025 08:50

Add metrics path API handler

bc1716f

Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

Change metrics endpoint to export histograms of all metrics

a33221a

Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

fix: export histogram prometheus format with cumulative sum rather th…

328f9fa

…an bucket values Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

fix: Formatting, Add author, Use stringstream, proper comments and fu…

6b11f13

…nction name and remove useless code. Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

Add metrics path API handler

48b1175

Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

Change metrics endpoint to export histograms of all metrics

a0c71ea

Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

fix: export histogram prometheus format with cumulative sum rather th…

f4753f3

…an bucket values Signed-off-by: SystemsPurge <naktiyoussef@proton.me>

Merge branch 'draft' into no-extern-deps

de88199

SystemsPurge requested a review from windrad6 as a code owner June 16, 2025 09:38

stv0g changed the base branch from add-prometheus-exporter to master June 17, 2025 08:57

stv0g mentioned this pull request Jun 24, 2025

feat(api): Add a new metrics endpoint for exporting statistics to Prometheus #932

Merged

stv0g closed this Jun 24, 2025

	std::string promFormat(std::string metric_name, std::string node_name) const;
	std::string toPrometheusText(std::string metric_name, std::string node_name) const;

	//need this because prometheus understands quantiles
	// Needed because Prometheus understands quantiles.

	std::string Hist::promFormat(std::string metric_name, std::string node_name) const{
	std::string Hist::toPrometheusText(std::string metric_name, std::string node_name) const{

	throw BadRequest("Nodes endpoint does not accept any body data");
	throw BadRequest("The metrics endpoint does not accept any body data");

	static char d[] = "Get stats of all nodes in desired format";
	static char d[] = "Get stats of all nodes in Prometheus metrics format";

Conversation

SystemsPurge commented Jun 10, 2025

Uh oh!

stv0g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stv0g commented Jun 12, 2025

Uh oh!

SystemsPurge commented Jun 16, 2025

Uh oh!

stv0g commented Jun 17, 2025

Uh oh!

SystemsPurge commented Jun 17, 2025

Uh oh!

stv0g commented Jun 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants