Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-22 Thread William Dauchy
On Fri, Nov 22, 2019 at 11:25:44AM +0100, Christopher Faulet wrote:
> URI delimiters must not be encoded, except if you want to escape it. So,
> your first url is not equivalent to the second one. The encoded question
> mark is part of the path in the first url, it is not the query-string
> delimiter.

wow I learned something today :)
and there is indeed a specific parameter for this precise reason you
mentioned. The end config looks like:

  - job_name: 'lb-metrics'
metrics_path: '/metrics'
params:
  scope: ['global', 'frontend', 'backend']
consul_sd_configs:
  - server: localhost:8500
services:
  - lb
relabel_configs:
  - source_labels: ['__meta_consul_dc']
target_label: 'dc'
  - source_labels: ['__address__', '__meta_consul_service_id']
target_label: 'instance'

and this time it works; I'm reassured :)

> But, cause of your remark, I re-read the function code and it revealed a
> bug. The query-string is decoded before parsing. Only parameters names and
> values must be decoded. Thus, a fix is still required.

good, I will handle the fix.

Thanks,
-- 
William



Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-22 Thread Christopher Faulet

Le 21/11/2019 à 16:54, William Dauchy a écrit :

Hi Christopher,

On Tue, Nov 19, 2019 at 04:35:47PM +0100, Christopher Faulet wrote:

+/* Parse the query stirng of request URI to filter the metrics. It returns 1 on
+ * success and -1 on error. */
+static int promex_parse_uri(struct appctx *appctx, struct stream_interface *si)
+{
+   struct channel *req = si_oc(si);
+   struct channel *res = si_ic(si);
+   struct htx *req_htx, *res_htx;
+   struct htx_sl *sl;
+   const char *p, *end;
+   struct buffer *err;
+   int default_scopes = PROMEX_FL_SCOPE_ALL;
+   int len;
+
+   /* Get the query-string */
+   req_htx = htxbuf(>buf);
+   sl = http_get_stline(req_htx);
+   if (!sl)
+   goto error;
+   p = http_find_param_list(HTX_SL_REQ_UPTR(sl), HTX_SL_REQ_ULEN(sl), '?');
+   if (!p)
+   goto end;


It's my turn to be sorry. I wrongly tested on my side regarding a real
integration with prometheus, because I mixed the old metrics and the new
ones. Indeed, prometheus is trying to scrape the encoded url such as:

metrics%3Fscope=global=frontend=backend
instead of
metrics?scope=global=frontend=backend



Hi William,

For the record, I report here our private conversation.

URI delimiters must not be encoded, except if you want to escape it. So, your 
first url is not equivalent to the second one. The encoded question mark is part 
of the path in the first url, it is not the query-string delimiter.


But, cause of your remark, I re-read the function code and it revealed a bug. 
The query-string is decoded before parsing. Only parameters names and values 
must be decoded. Thus, a fix is still required.


--
Christopher Faulet



Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-21 Thread William Dauchy
Hi Christopher,

On Tue, Nov 19, 2019 at 04:35:47PM +0100, Christopher Faulet wrote:
> +/* Parse the query stirng of request URI to filter the metrics. It returns 1 
> on
> + * success and -1 on error. */
> +static int promex_parse_uri(struct appctx *appctx, struct stream_interface 
> *si)
> +{
> + struct channel *req = si_oc(si);
> + struct channel *res = si_ic(si);
> + struct htx *req_htx, *res_htx;
> + struct htx_sl *sl;
> + const char *p, *end;
> + struct buffer *err;
> + int default_scopes = PROMEX_FL_SCOPE_ALL;
> + int len;
> +
> + /* Get the query-string */
> + req_htx = htxbuf(>buf);
> + sl = http_get_stline(req_htx);
> + if (!sl)
> + goto error;
> + p = http_find_param_list(HTX_SL_REQ_UPTR(sl), HTX_SL_REQ_ULEN(sl), '?');
> + if (!p)
> + goto end;

It's my turn to be sorry. I wrongly tested on my side regarding a real
integration with prometheus, because I mixed the old metrics and the new
ones. Indeed, prometheus is trying to scrape the encoded url such as:

metrics%3Fscope=global=frontend=backend
instead of
metrics?scope=global=frontend=backend

Do you think it could be acceptable to send a patch adding a function
such as:
static inline char *http_find_encoded_param_list(char *path, size_t path_l, 
char* delim);

and test the encoded search if we don't find '?'
Or is there a way to convert the url first?

I'm ok to handle the patch if you validate the solution.

Thanks,
-- 
William



Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-21 Thread Илья Шипицин
чт, 21 нояб. 2019 г. в 15:18, William Dauchy :

> On Thu, Nov 21, 2019 at 03:09:30PM +0500, Илья Шипицин wrote:
> > I understand. However, those patches add complexity (which might be moved
> > to another dedicated tool)
>
> those patch makes sense for heavy haproxy instances. As you might have
> seen above, we are talking about > 130MB of data. So for a full scraping
> every 60s or less, this is not realistic. Even the data loading might
> take too much time. We had cases where loading the data on exporter side
> was taking more time than the frequency of scraping, generating a
> snowball effect.
> It's a good pratice to avoid exporting data you know
> you won't use instead of: scraping -> deleting -> aggregate
> In our case we do not use most of the server metrics. It represents a
> factor of 10 in terms of exported data.
>

yep. I did see 130mb and I was impressed.


>
> --
> William
>


Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-21 Thread William Dauchy
On Thu, Nov 21, 2019 at 03:09:30PM +0500, Илья Шипицин wrote:
> I understand. However, those patches add complexity (which might be moved
> to another dedicated tool)

those patch makes sense for heavy haproxy instances. As you might have
seen above, we are talking about > 130MB of data. So for a full scraping
every 60s or less, this is not realistic. Even the data loading might
take too much time. We had cases where loading the data on exporter side
was taking more time than the frequency of scraping, generating a
snowball effect.
It's a good pratice to avoid exporting data you know
you won't use instead of: scraping -> deleting -> aggregate
In our case we do not use most of the server metrics. It represents a
factor of 10 in terms of exported data.

-- 
William


Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-21 Thread Илья Шипицин
чт, 21 нояб. 2019 г. в 15:07, William Dauchy :

> On Thu, Nov 21, 2019 at 03:00:02PM +0500, Илья Шипицин wrote:
> > is it common thing in Prometheus world to perform cascade export ? like
> > "exporter" --> "filter out" --> "aggregate" --> "deliver to prometheus"
> > in order to keep things simple and not to push everything into single
> tool
>
> no, the best practice is often to keep the original data in a local
> prometheus, and aggregate (quite often you use another instance for
> aggregation)
> But here those patch are about dropping the data you know you will never
> use.
>

I understand. However, those patches add complexity (which might be moved
to another dedicated tool)


> --
> William
>


Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-21 Thread William Dauchy
On Thu, Nov 21, 2019 at 03:00:02PM +0500, Илья Шипицин wrote:
> is it common thing in Prometheus world to perform cascade export ? like
> "exporter" --> "filter out" --> "aggregate" --> "deliver to prometheus"
> in order to keep things simple and not to push everything into single tool

no, the best practice is often to keep the original data in a local
prometheus, and aggregate (quite often you use another instance for
aggregation)
But here those patch are about dropping the data you know you will never
use.
-- 
William


Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-21 Thread Илья Шипицин
btw, side question...

is it common thing in Prometheus world to perform cascade export ? like
"exporter" --> "filter out" --> "aggregate" --> "deliver to prometheus"
in order to keep things simple and not to push everything into single tool

чт, 21 нояб. 2019 г. в 14:49, Christopher Faulet :

> Le 20/11/2019 à 21:23, William Dauchy a écrit :
> > Hi Christopher,
> >
> > On Wed, Nov 20, 2019 at 02:56:28PM +0100, Christopher Faulet wrote:
> >> Nice, Thanks for your feedback. It is merged now. And I'm on the
> backports
> >> for the 2.0.
> >
> > You apparently forgot to backport
> > commit 0d1c2a65e8370a770d01 (MINOR: stats: Report max times in addition
> of the averages for sessions)
> >
> > 2.0 tree does not build anymore because ST_F_QT_MAX is not defined.
> >
>
> Damned ! You're right. I'm sorry. It was backported now. Thanks !
>
> --
> Christopher Faulet
>
>


Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-21 Thread Christopher Faulet

Le 20/11/2019 à 21:23, William Dauchy a écrit :

Hi Christopher,

On Wed, Nov 20, 2019 at 02:56:28PM +0100, Christopher Faulet wrote:

Nice, Thanks for your feedback. It is merged now. And I'm on the backports
for the 2.0.


You apparently forgot to backport
commit 0d1c2a65e8370a770d01 (MINOR: stats: Report max times in addition of the 
averages for sessions)

2.0 tree does not build anymore because ST_F_QT_MAX is not defined.



Damned ! You're right. I'm sorry. It was backported now. Thanks !

--
Christopher Faulet



Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-20 Thread William Dauchy
Hi Christopher,

On Wed, Nov 20, 2019 at 02:56:28PM +0100, Christopher Faulet wrote:
> Nice, Thanks for your feedback. It is merged now. And I'm on the backports
> for the 2.0.

You apparently forgot to backport
commit 0d1c2a65e8370a770d01 (MINOR: stats: Report max times in addition of the 
averages for sessions)

2.0 tree does not build anymore because ST_F_QT_MAX is not defined. 
-- 
William



Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-20 Thread Christopher Faulet

Le 20/11/2019 à 13:03, William Dauchy a écrit :

On Tue, Nov 19, 2019 at 04:35:47PM +0100, Christopher Faulet wrote:

Here is updated patches with the support for "scope" and "no-maint"
parameters. If this solution is good enough for you (and if it works :), I
will push it.


$ curl 
"http://127.0.0.1:8080/metrics?scope=global=frontend=backend=server;
151M
$ curl 
"http://127.0.0.1:8080/metrics?scope=global=frontend=backend=server;
13.9M

looks very useful from here :)
I think you can push this last version!



Nice, Thanks for your feedback. It is merged now. And I'm on the backports for 
the 2.0.


--
Christopher Faulet



Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-20 Thread William Dauchy
On Tue, Nov 19, 2019 at 04:35:47PM +0100, Christopher Faulet wrote:
> Here is updated patches with the support for "scope" and "no-maint"
> parameters. If this solution is good enough for you (and if it works :), I
> will push it.

$ curl 
"http://127.0.0.1:8080/metrics?scope=global=frontend=backend=server;
151M
$ curl 
"http://127.0.0.1:8080/metrics?scope=global=frontend=backend=server;
13.9M

looks very useful from here :)
I think you can push this last version!
-- 
William



Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-19 Thread William Dauchy
Hi Christopher,

On Tue, Nov 19, 2019 at 04:35:47PM +0100, Christopher Faulet wrote:
> Here is updated patches with the support for "scope" and "no-maint"
> parameters. If this solution is good enough for you (and if it works :), I
> will push it.

this looks good to me and the test was conclusive on our side.
For now we won't make use of no-maint as the server are completely
deactivated. We should however measure how much data does it represent
now, in order to see if we could simply use no-maint parameter.
I will push the no-maint patch on our production later today.

Thank you for your work,
-- 
William



Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-19 Thread Christopher Faulet

Le 19/11/2019 à 14:51, Christopher Faulet a écrit :


Regarding the problem of servers in maintenance, since we parse the
query-string, it is possible to add more filters. I may add a parameter to
filter out servers in maintenance. For instance, by passing "no-maint" in the
query-string, all servers in maintenance could be ignored. This way, it would
still be possible to have servers metrics.



William,

Here is updated patches with the support for "scope" and "no-maint" parameters. 
If this solution is good enough for you (and if it works :), I will push it.


Thanks,
--
Christopher Faulet
>From 205c4775ccec914a2a501fe9499a77b96285315b Mon Sep 17 00:00:00 2001
From: Christopher Faulet 
Date: Mon, 18 Nov 2019 14:47:08 +0100
Subject: [PATCH 1/2] MINOR: contrib/prometheus-exporter: filter exported
 metrics by scope

Now, the prometheus exporter parses the HTTP query-string to filter or to adapt
the exported metrics. In this first version, it is only possible select the
scopes of metrics to export. To do so, one or more parameters with "scope" as
name must be passed in the query-string, with one of those values: global,
frontend, backend, server or '*' (means all). A scope parameter with no value
means to filter out all scopes (nothing is returned). The scope parameters are
parsed in their appearance order in the query-string. So an empty scope will
reset all scopes already parsed. But it can be overridden by following scope
parameters in the query-string. By default everything is exported.

The filtering can also be done on prometheus scraping configuration, but general
aim is to optimise the source of data to improve load and scraping time. This is
particularly true for huge configuration with thousands of backends and servers.
Also note that this configuration was possible on the previous official haproxy
exporter but with even more parameters to select the needed metrics. Here we
thought it was sufficient to simply avoid a given type of metric. However, more
filters are still possible.

Thanks to William Dauchy. This patch is based on his work.
---
 contrib/prometheus-exporter/README|  24 +++
 .../prometheus-exporter/service-prometheus.c  | 157 +++---
 2 files changed, 156 insertions(+), 25 deletions(-)

diff --git a/contrib/prometheus-exporter/README b/contrib/prometheus-exporter/README
index 915fc7f54..84ae8e27e 100644
--- a/contrib/prometheus-exporter/README
+++ b/contrib/prometheus-exporter/README
@@ -50,6 +50,30 @@ spend much more ressources to produce the Prometheus metrics than the CSV export
 through the stats page. To give a comparison order, quick benchmarks shown that
 a PROMEX dump is 5x slower and 20x more verbose than a CSV export.
 
+
+metrics filtering
+---
+
+It is possible to dynamically select the metrics to export if you don't use all
+of them passing parameters in the query-string.
+
+* Filtering on scopes
+
+The metrics may be filtered by scopes. Multiple parameters with "scope" as name
+may be passed in the query-string to filter exported metrics, with one of those
+values: global, frontend, backend, server or '*' (means all). A scope parameter
+with no value means to filter out all scopes (nothing is returned). The scope
+parameters are parsed in their appearance order in the query-string. So an empty
+scope will reset all scopes already parsed. But it can be overridden by
+following scope parameters in the query-string. By default everything is
+exported. Here are examples:
+
+  /metrics?scope=server # ==> server metrics will be exported
+  /metrics?scope=frontend=backend # ==> Frontend and backend metrics will be exported
+  /metrics?scope=*=   # ==> no metrics will be exported
+  /metrics?scope==global  # ==> global metrics will be exported
+
+
 Exported metrics
 --
 
diff --git a/contrib/prometheus-exporter/service-prometheus.c b/contrib/prometheus-exporter/service-prometheus.c
index 508e6b1fc..c70daa905 100644
--- a/contrib/prometheus-exporter/service-prometheus.c
+++ b/contrib/prometheus-exporter/service-prometheus.c
@@ -64,6 +64,12 @@ enum {
 #define PROMEX_FL_METRIC_HDR0x0001
 #define PROMEX_FL_INFO_METRIC   0x0002
 #define PROMEX_FL_STATS_METRIC  0x0004
+#define PROMEX_FL_SCOPE_GLOBAL  0x0008
+#define PROMEX_FL_SCOPE_FRONT   0x0010
+#define PROMEX_FL_SCOPE_BACK0x0020
+#define PROMEX_FL_SCOPE_SERVER  0x0040
+
+#define PROMEX_FL_SCOPE_ALL (PROMEX_FL_SCOPE_GLOBAL|PROMEX_FL_SCOPE_FRONT|PROMEX_FL_SCOPE_BACK|PROMEX_FL_SCOPE_SERVER)
 
 /* The max length for metrics name. It is a hard limit but it should be
  * enougth.
@@ -2102,72 +2108,81 @@ static int promex_dump_srv_metrics(struct appctx *appctx, struct htx *htx)
 static int promex_dump_metrics(struct appctx *appctx, struct stream_interface *si, struct htx *htx)
 {
 	int ret;
+	int flags = appctx->ctx.stats.flags;
 
 	switch (appctx->st1) {
 		case PROMEX_DUMPER_INIT:
 			appctx->ctx.stats.px = 

Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-19 Thread Christopher Faulet

Hi William,

I missed Pierre's email. I'm CCing him.

Le 18/11/2019 à 21:00, William Dauchy a écrit :

Thanks. Having a way to filter metrics in the Prometheus exporter was on my
todo-list :) Filtering on scopes is pretty simple and it is a good start to
solve performance issues for huge configs. It is a good idea.
However, IMHO, it is easier to use the query-string to select exported
metrics. I don't know if it is compatible with the way Prometheus is used.
For instance, based on your idea, to get only metrics about servers, the URI
could be "/metric?scope=server". And to get metrics about frontends and
backends, it could be "/metric?scope=frontend=backend". Of course, it
is extensible. We can imagine to add filters on frontend/backend/server
names.
Here is a quick patch based on this. What do you think about it ? If you
said me your way to select metrics is better from the Prometheus point of
view, I'm ok with that.


In fact I even did not thought about it because my state of mind was to
filter at startup like we were doing before. I like you propostion as it
is way more extensible than mine. I was not even thinking it would be
possible to access the URL, I now have a lot of ideas of the future ;)

example config would look like:

- job_name: 'lb-builtin-metrics'
 metrics_path: '/metrics?scope=backend'
 consul_sd_configs:
   - server: localhost:8500
 services:
   - lb-builtin-exporter
 relabel_configs:
   - source_labels: ['__meta_consul_dc']
 target_label: 'dc'
   - source_labels: ['__address__', '__meta_consul_service_id']
 target_label: 'instance'

If you have other things on your todo list regarding that feel free to
share so we might share the work around it.


Well, for now, there is nothing concrete. But I've planned to add a filter on 
frontend/backend names and, if possible, a way to list the metrics to export. 
The scope idea may be used too for this last one, by grouping metrics by 
categories (http, conn, timers, checks, rates...). In the end, it's more a 
question of time than anything else :)



Also have you seen the message from Pierre? They are a few things
we would like to discuss whether it would be possible to aggregate a few
things on backend side.
https://www.mail-archive.com/haproxy@formilux.org/msg35369.html
(we somehow forgot to put you in copy though)


Regarding the problem of servers in maintenance, since we parse the 
query-string, it is possible to add more filters. I may add a parameter to 
filter out servers in maintenance. For instance, by passing "no-maint" in the 
query-string, all servers in maintenance could be ignored. This way, it would 
still be possible to have servers metrics.


For others points, I will reply to the previous email.



Thank you for this patch. Do you think it could be acceptable to mark it
as a possible backport for v2.0.x? It's quite critical on our side as we
are dealing with ~130MB on some cases. If not we will do the backport on
our wide while waiting for the v2.1.



It can safely be backported to 2.0. It is not a critical component and it is 
independent of other parts.




I will push a test based on your patch tomorrow on our side.


Ok, let me know if you encounter any issues.

Thanks,
--
Christopher Faulet



Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-18 Thread William Dauchy
Hello Christopher,

Thank you for your quick answer.

On Mon, Nov 18, 2019 at 04:28:37PM +0100, Christopher Faulet wrote:
> Thanks. Having a way to filter metrics in the Prometheus exporter was on my
> todo-list :) Filtering on scopes is pretty simple and it is a good start to
> solve performance issues for huge configs. It is a good idea.
> However, IMHO, it is easier to use the query-string to select exported
> metrics. I don't know if it is compatible with the way Prometheus is used.
> For instance, based on your idea, to get only metrics about servers, the URI
> could be "/metric?scope=server". And to get metrics about frontends and
> backends, it could be "/metric?scope=frontend=backend". Of course, it
> is extensible. We can imagine to add filters on frontend/backend/server
> names.
> Here is a quick patch based on this. What do you think about it ? If you
> said me your way to select metrics is better from the Prometheus point of
> view, I'm ok with that.

In fact I even did not thought about it because my state of mind was to
filter at startup like we were doing before. I like you propostion as it
is way more extensible than mine. I was not even thinking it would be
possible to access the URL, I now have a lot of ideas of the future ;)

example config would look like:

- job_name: 'lb-builtin-metrics'
metrics_path: '/metrics?scope=backend'
consul_sd_configs:
  - server: localhost:8500
services:
  - lb-builtin-exporter
relabel_configs:
  - source_labels: ['__meta_consul_dc']
target_label: 'dc'
  - source_labels: ['__address__', '__meta_consul_service_id']
target_label: 'instance'

If you have other things on your todo list regarding that feel free to
share so we might share the work around it. 
Also have you seen the message from Pierre? They are a few things
we would like to discuss whether it would be possible to aggregate a few
things on backend side.
https://www.mail-archive.com/haproxy@formilux.org/msg35369.html
(we somehow forgot to put you in copy though)

> From 413eedf7814660218d2207e7be5e203433c399a7 Mon Sep 17 00:00:00 2001
> From: Christopher Faulet 
> Date: Mon, 18 Nov 2019 14:47:08 +0100
> Subject: [PATCH] MINOR: contrib/prometheus-exporter: filter exported metrics
>  by scope
> 
> Now, the prometheus exporter parses the HTTP query-string to filter or to 
> adapt
> the exported metrics. In this first version, it is only possible select the
> scopes of metrics to export. To do so, one or more parameters with "scope" as
> name must be passed in the query-string, with one of those values: global,
> frontend, backend, server or '*' (means all). A scope parameter with no value
> means to filter out all scopes (nothing is returned). The scope parameters are
> parsed in their appearance order in the query-string. So an empty scope will
> reset all scopes already parsed. But it can be overridden by following scope
> parameters in the query-string. By default everything is exported.
> 
> The filtering can also be done on prometheus scraping configuration, but 
> general
> aim is to optimise the source of data to improve load and scraping time. This 
> is
> particularly true for huge configuration with thousands of backends and 
> servers.
> Also note that this configuration was possible on the previous official 
> haproxy
> exporter but with even more parameters to select the needed metrics. Here we
> thought it was sufficient to simply avoid a given type of metric. However, 
> more
> filters are still possible.
> 
> Thanks to William Dauchy. This patch is based on his work.
> ---
>  contrib/prometheus-exporter/README|  16 ++
>  .../prometheus-exporter/service-prometheus.c  | 157 +++---
>  2 files changed, 148 insertions(+), 25 deletions(-)

Thank you for this patch. Do you think it could be acceptable to mark it
as a possible backport for v2.0.x? It's quite critical on our side as we
are dealing with ~130MB on some cases. If not we will do the backport on
our wide while waiting for the v2.1.

I will push a test based on your patch tomorrow on our side.
-- 
William



Re: [PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-18 Thread Christopher Faulet

Le 16/11/2019 à 21:02, William Dauchy a écrit :

this patch is an attempt to permit parameters on the prometheus exporter
configuration: global, frontend, backend, listener, server.
This allows to avoid exporting metrics you don't necessary use. The
filtering can also be done on prometheus scraping configuration, but
general aim is to optimise the source of data to improve load and
scraping time. This is particularly true for huge configuration with
thousands of backends and servers.
Also note that this configuration was possible on the previous official
haproxy exporter but with even more parameters to select the needed
metrics. Here we thought it was sufficient to simply avoid a given type
of metric.
By default, everything is exported as before.



Hi William,

Thanks. Having a way to filter metrics in the Prometheus exporter was on my 
todo-list :) Filtering on scopes is pretty simple and it is a good start to 
solve performance issues for huge configs. It is a good idea.


However, IMHO, it is easier to use the query-string to select exported metrics. 
I don't know if it is compatible with the way Prometheus is used. For instance, 
based on your idea, to get only metrics about servers, the URI could be 
"/metric?scope=server". And to get metrics about frontends and backends, it 
could be "/metric?scope=frontend=backend". Of course, it is extensible. We 
can imagine to add filters on frontend/backend/server names.


Here is a quick patch based on this. What do you think about it ? If you said me 
your way to select metrics is better from the Prometheus point of view, I'm ok 
with that.


--
Christopher Faulet
>From 413eedf7814660218d2207e7be5e203433c399a7 Mon Sep 17 00:00:00 2001
From: Christopher Faulet 
Date: Mon, 18 Nov 2019 14:47:08 +0100
Subject: [PATCH] MINOR: contrib/prometheus-exporter: filter exported metrics
 by scope

Now, the prometheus exporter parses the HTTP query-string to filter or to adapt
the exported metrics. In this first version, it is only possible select the
scopes of metrics to export. To do so, one or more parameters with "scope" as
name must be passed in the query-string, with one of those values: global,
frontend, backend, server or '*' (means all). A scope parameter with no value
means to filter out all scopes (nothing is returned). The scope parameters are
parsed in their appearance order in the query-string. So an empty scope will
reset all scopes already parsed. But it can be overridden by following scope
parameters in the query-string. By default everything is exported.

The filtering can also be done on prometheus scraping configuration, but general
aim is to optimise the source of data to improve load and scraping time. This is
particularly true for huge configuration with thousands of backends and servers.
Also note that this configuration was possible on the previous official haproxy
exporter but with even more parameters to select the needed metrics. Here we
thought it was sufficient to simply avoid a given type of metric. However, more
filters are still possible.

Thanks to William Dauchy. This patch is based on his work.
---
 contrib/prometheus-exporter/README|  16 ++
 .../prometheus-exporter/service-prometheus.c  | 157 +++---
 2 files changed, 148 insertions(+), 25 deletions(-)

diff --git a/contrib/prometheus-exporter/README b/contrib/prometheus-exporter/README
index 915fc7f54..f540d496f 100644
--- a/contrib/prometheus-exporter/README
+++ b/contrib/prometheus-exporter/README
@@ -50,6 +50,22 @@ spend much more ressources to produce the Prometheus metrics than the CSV export
 through the stats page. To give a comparison order, quick benchmarks shown that
 a PROMEX dump is 5x slower and 20x more verbose than a CSV export.
 
+It is possible to dynamically select the metrics to export if you don't use all
+of them passing parameters in the query-string. The metrics may be filtered by
+scope. To do so, one or more parameters with "scope" as name must be passed in
+the query-string, with one of those values: global, frontend, backend, server or
+'*' (means all). A scope parameter with no value means to filter out all scopes
+(nothing is returned). The scope parameters are parsed in their appearance order
+in the query-string. So an empty scope will reset all scopes already parsed. But
+it can be overridden by following scope parameters in the query-string. By
+default everything is exported. Here are examples:
+
+  /metrics?scope=server # ==> server metrics will be exported
+  /metrics?scope=frontend=backend # ==> Frontend and backend metrics will be exported
+  /metrics?scope=*=   # ==> no metrics will be exported
+  /metrics?scope==global  # ==> global metrics will be exported
+
+
 Exported metrics
 --
 
diff --git a/contrib/prometheus-exporter/service-prometheus.c b/contrib/prometheus-exporter/service-prometheus.c
index 508e6b1fc..09e8ad403 100644
--- 

[PATCH] MINOR: contrib/prometheus-exporter: allow to select the exported metrics

2019-11-16 Thread William Dauchy
this patch is an attempt to permit parameters on the prometheus exporter
configuration: global, frontend, backend, listener, server.
This allows to avoid exporting metrics you don't necessary use. The
filtering can also be done on prometheus scraping configuration, but
general aim is to optimise the source of data to improve load and
scraping time. This is particularly true for huge configuration with
thousands of backends and servers.
Also note that this configuration was possible on the previous official
haproxy exporter but with even more parameters to select the needed
metrics. Here we thought it was sufficient to simply avoid a given type
of metric.
By default, everything is exported as before.

Signed-off-by: William Dauchy 
---
 contrib/prometheus-exporter/README|   7 ++
 .../prometheus-exporter/service-prometheus.c  | 100 ++
 2 files changed, 87 insertions(+), 20 deletions(-)

diff --git a/contrib/prometheus-exporter/README 
b/contrib/prometheus-exporter/README
index 915fc7f5..59c600ca 100644
--- a/contrib/prometheus-exporter/README
+++ b/contrib/prometheus-exporter/README
@@ -49,6 +49,13 @@ must loop on all proxies for each metric. Same for the 
servers. Thus, it will
 spend much more ressources to produce the Prometheus metrics than the CSV 
export
 through the stats page. To give a comparison order, quick benchmarks shown that
 a PROMEX dump is 5x slower and 20x more verbose than a CSV export.
+You may however select a given metric if you don't use all of them. The 
available
+keywords are: global, frontend, backend, listener, server.
+I can be used as argument with a space in between:
+
+http-request use-service prometheus-exporter global frontend backend if { 
path /metrics }
+
+By default, all the metrics are activated.
 
 Exported metrics
 --
diff --git a/contrib/prometheus-exporter/service-prometheus.c 
b/contrib/prometheus-exporter/service-prometheus.c
index 508e6b1f..ea341009 100644
--- a/contrib/prometheus-exporter/service-prometheus.c
+++ b/contrib/prometheus-exporter/service-prometheus.c
@@ -13,6 +13,8 @@
  *
  */
 
+#include 
+
 #include 
 #include 
 #include 
@@ -60,6 +62,21 @@ enum {
PROMEX_DUMPER_DONE, /* finished */
 };
 
+typedef struct {
+   char *key;
+   int val;
+} t_promex_args;
+
+static const t_promex_args promex_args[] = {
+   { "global",   PROMEX_DUMPER_GLOBAL},
+   { "frontend", PROMEX_DUMPER_FRONT},
+   { "backend",  PROMEX_DUMPER_BACK},
+   { "listener", PROMEX_DUMPER_LI},
+   { "server",   PROMEX_DUMPER_SRV},
+};
+
+#define PROMEX_ARGS_LEN (sizeof(promex_args) / sizeof(t_promex_args))
+
 /* Prometheus exporter flags (appctx->ctx.stats.flags) */
 #define PROMEX_FL_METRIC_HDR0x0001
 #define PROMEX_FL_INFO_METRIC   0x0002
@@ -2101,6 +2118,7 @@ static int promex_dump_srv_metrics(struct appctx *appctx, 
struct htx *htx)
  * -1 in case of any error. */
 static int promex_dump_metrics(struct appctx *appctx, struct stream_interface 
*si, struct htx *htx)
 {
+   bool *promex_settings = *appctx->rule->arg.act.p;
int ret;
 
switch (appctx->st1) {
@@ -2113,11 +2131,13 @@ static int promex_dump_metrics(struct appctx *appctx, 
struct stream_interface *s
/* fall through */
 
case PROMEX_DUMPER_GLOBAL:
-   ret = promex_dump_global_metrics(appctx, htx);
-   if (ret <= 0) {
-   if (ret == -1)
-   goto error;
-   goto full;
+   if (promex_settings[PROMEX_DUMPER_GLOBAL]) {
+   ret = promex_dump_global_metrics(appctx, htx);
+   if (ret <= 0) {
+   if (ret == -1)
+   goto error;
+   goto full;
+   }
}
 
appctx->ctx.stats.px = proxies_list;
@@ -2128,11 +2148,13 @@ static int promex_dump_metrics(struct appctx *appctx, 
struct stream_interface *s
/* fall through */
 
case PROMEX_DUMPER_FRONT:
-   ret = promex_dump_front_metrics(appctx, htx);
-   if (ret <= 0) {
-   if (ret == -1)
-   goto error;
-   goto full;
+   if (promex_settings[PROMEX_DUMPER_FRONT]) {
+   ret = promex_dump_front_metrics(appctx, htx);
+   if (ret <= 0) {
+   if (ret == -1)
+   goto error;
+   goto full;
+   }
}
 
appctx->ctx.stats.px =