From 797d1df519d1780fa27992adc038a73ef466f9fe Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 5 Jun 2020 08:18:56 +0200 Subject: [PATCH] BUG/MEDIUM: contrib/prometheus-exporter: Properly set flags to dump metrics When the metrics are dumped, in the main function promex_dump_metrics(), the appctx flags are set before entering in a new scope, among others things to know which metrics names and descriptions to use. But, those flags are not restored when the dump is interrupted because of a full output buffer. If this happens after the dump of global metrics, it may only lead to extra #TYPE and #HELP lines. But if this happens during the dump of global metrics, the following dumps of frontends, backends and servers metrics use names and descriptions of global ones with the unmatching indexes. This first leads to unexisting metrics names. For instance, "haproxy_frontend_nbproc". But also to out-of-bound accesses to name and description arrays because there are more stats fields than info fields. It is easy to reproduce the bug using small buffers, setting tune.bufsize to 8192 for instance. This patch should fix the issue #666. It must be backported as far as 2.0. (cherry picked from commit efde955bdb7b7479267cfcd823cce26bb3f5c4d3) Signed-off-by: Christopher Faulet --- contrib/prometheus-exporter/service-prometheus.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contrib/prometheus-exporter/service-prometheus.c b/contrib/prometheus-exporter/service-prometheus.c index 3a5a5f3..29e7503 100644 --- a/contrib/prometheus-exporter/service-prometheus.c +++ b/contrib/prometheus-exporter/service-prometheus.c @@ -2129,13 +2129,12 @@ 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 = NULL; appctx->ctx.stats.sv = NULL; - appctx->ctx.stats.flags = (flags|PROMEX_FL_METRIC_HDR|PROMEX_FL_INFO_METRIC); + appctx->ctx.stats.flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_INFO_METRIC); appctx->st2 = promex_global_metrics[INF_NAME]; appctx->st1 = PROMEX_DUMPER_GLOBAL; /* fall through */ @@ -2152,7 +2151,8 @@ static int promex_dump_metrics(struct appctx *appctx, struct stream_interface *s appctx->ctx.stats.px = proxies_list; appctx->ctx.stats.sv = NULL; - appctx->ctx.stats.flags = (flags|PROMEX_FL_METRIC_HDR|PROMEX_FL_STATS_METRIC); + appctx->ctx.stats.flags &= ~PROMEX_FL_INFO_METRIC; + appctx->ctx.stats.flags |= (PROMEX_FL_METRIC_HDR|PROMEX_FL_STATS_METRIC); appctx->st2 = promex_front_metrics[ST_F_PXNAME]; appctx->st1 = PROMEX_DUMPER_FRONT; /* fall through */ @@ -2169,7 +2169,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stream_interface *s appctx->ctx.stats.px = proxies_list; appctx->ctx.stats.sv = NULL; - appctx->ctx.stats.flags = (flags|PROMEX_FL_METRIC_HDR|PROMEX_FL_STATS_METRIC); + appctx->ctx.stats.flags |= PROMEX_FL_METRIC_HDR; appctx->st2 = promex_back_metrics[ST_F_PXNAME]; appctx->st1 = PROMEX_DUMPER_BACK; /* fall through */ @@ -2186,7 +2186,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stream_interface *s appctx->ctx.stats.px = proxies_list; appctx->ctx.stats.sv = (appctx->ctx.stats.px ? appctx->ctx.stats.px->srv : NULL); - appctx->ctx.stats.flags = (flags|PROMEX_FL_METRIC_HDR|PROMEX_FL_STATS_METRIC); + appctx->ctx.stats.flags |= PROMEX_FL_METRIC_HDR; appctx->st2 = promex_srv_metrics[ST_F_PXNAME]; appctx->st1 = PROMEX_DUMPER_SRV; /* fall through */ @@ -2203,7 +2203,7 @@ static int promex_dump_metrics(struct appctx *appctx, struct stream_interface *s appctx->ctx.stats.px = NULL; appctx->ctx.stats.sv = NULL; - appctx->ctx.stats.flags = flags; + appctx->ctx.stats.flags &= ~(PROMEX_FL_METRIC_HDR|PROMEX_FL_INFO_METRIC|PROMEX_FL_STATS_METRIC); appctx->st2 = 0; appctx->st1 = PROMEX_DUMPER_DONE; /* fall through */ -- 1.7.10.4