Hi all, On Mon, Dec 16, 2019 at 05:53:36PM +0100, Lukas Tribus wrote: > > We could imagine adding one (or multiple) optional arguments to force > > the output to another destination (e.g. stdout, stderr, other ring), > > and maybe another one for the format or to prepend a prefix. Then most > > likely we'd use the ring buffer by default as it's the least impacting > > one and the only self-sustaining output. And probably that we could > > switch to stderr by default in backports (or make it mandatory to > > force the destination). > > > > What do you think ? > > I like it, more flexibility to debug in production is always a good.
OK, so here's what I came up with. Should I merge it or does anyone have any comment about it ? Thanks, Willy
>From ed8b2a7f79b5e98865aba6005d8a0c584cf42c6f Mon Sep 17 00:00:00 2001 From: Willy Tarreau <[email protected]> Date: Tue, 17 Dec 2019 10:07:25 +0100 Subject: MINOR: debug: support logging to various sinks As discussed in the thread below [1], the debug converter is currently not of much use given that it's only built when DEBUG_EXPR is set, and it is limited to stderr only. This patch changes this to make it take an optional prefix and an optional target sink so that it can log to stdout, stderr or a ring buffer. The default output is the "buf0" ring buffer, that can be consulted from the CLI. [1] https://www.mail-archive.com/[email protected]/msg35671.html Note: if this patch is backported, it also requiest the following commit to work: 46dfd78cbf ("BUG/MINOR: sample: always check converters' arguments"). --- doc/configuration.txt | 18 +++++-- src/sample.c | 108 ++++++++++++++++++++++++++++-------------- 2 files changed, 87 insertions(+), 39 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index fdcdb04fae..8d27e8a416 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -13284,10 +13284,20 @@ da-csv-conv(<prop>[,<prop>*]) default_backend servers http-request set-header X-DeviceAtlas-Data %[req.fhdr(User-Agent),da-csv(primaryHardwareType,osName,osVersion,browserName,browserVersion,browserRenderingEngine)] -debug - This converter is used as debug tool. It dumps on screen the content and the - type of the input sample. The sample is returned as is on its output. This - converter only exists when haproxy was built with debugging enabled. +debug([<prefix][,<destination>]) + This converter is used as debug tool. It takes a capture of the input sample + and sends it to event sink <destination>, which may designate a ring buffer + such as "buf0", as well as "stdout", or "stderr". Available sinks may be + checked at run time by issuing "show events" on the CLI. When not specified, + the output will be "buf0", which may be consulted via the CLI's "show events" + command. An optional prefix <prefix> may be passed to help distinguish + outputs from multiple expressions. It will then appear before the colon in + the output message. The input sample is passed as-is on the output, so that + it is safe to insert the debug converter anywhere in a chain, even with non- + printable sample types. + + Example: + tcp-request connection track-sc0 src,debug(track-sc) div(<value>) Divides the input value of type signed integer by <value>, and returns the diff --git a/src/sample.c b/src/sample.c index b124c4b861..a035a035f5 100644 --- a/src/sample.c +++ b/src/sample.c @@ -32,6 +32,7 @@ #include <proto/proxy.h> #include <proto/protocol_buffers.h> #include <proto/sample.h> +#include <proto/sink.h> #include <proto/stick_table.h> #include <proto/vars.h> @@ -1436,45 +1437,85 @@ void release_sample_expr(struct sample_expr *expr) /* These functions set the data type on return. */ /*****************************************************************/ -#ifdef DEBUG_EXPR static int sample_conv_debug(const struct arg *arg_p, struct sample *smp, void *private) { int i; struct sample tmp; + struct buffer *buf; + struct sink *sink; + struct ist line; + char *pfx; - if (!(global.mode & MODE_QUIET) || (global.mode & (MODE_VERBOSE | MODE_STARTING))) { - fprintf(stderr, "[debug converter] type: %s ", smp_to_type[smp->data.type]); - if (!sample_casts[smp->data.type][SMP_T_STR]) { - fprintf(stderr, "(undisplayable)"); - } else { - - /* Copy sample fetch. This put the sample as const, the - * cast will copy data if a transformation is required. - */ - memcpy(&tmp, smp, sizeof(struct sample)); - tmp.flags = SMP_F_CONST; - - if (!sample_casts[smp->data.type][SMP_T_STR](&tmp)) - fprintf(stderr, "(undisplayable)"); - - else { - /* Display the displayable chars*. */ - fputc('<', stderr); - for (i = 0; i < tmp.data.u.str.data; i++) { - if (isprint(tmp.data.u.str.area[i])) - fputc(tmp.data.u.str.area[i], - stderr); - else - fputc('.', stderr); - } - fputc('>', stderr); - } - } - fputc('\n', stderr); + buf = alloc_trash_chunk(); + if (!buf) + goto end; + + sink = (struct sink *)arg_p[1].data.str.area; + BUG_ON(!sink); + + pfx = arg_p[0].data.str.area; + BUG_ON(!pfx); + + chunk_printf(buf, "[debug] %s: type=%s ", pfx, smp_to_type[smp->data.type]); + if (!sample_casts[smp->data.type][SMP_T_STR]) + goto nocast; + + /* Copy sample fetch. This puts the sample as const, the + * cast will copy data if a transformation is required. + */ + memcpy(&tmp, smp, sizeof(struct sample)); + tmp.flags = SMP_F_CONST; + + if (!sample_casts[smp->data.type][SMP_T_STR](&tmp)) + goto nocast; + + /* Display the displayable chars*. */ + b_putchr(buf, '<'); + for (i = 0; i < tmp.data.u.str.data; i++) { + if (isprint(tmp.data.u.str.area[i])) + b_putchr(buf, tmp.data.u.str.area[i]); + else + b_putchr(buf, '.'); + } + b_putchr(buf, '>'); + + done: + line = ist2(buf->area, buf->data); + sink_write(sink, &line, 1); + end: + free_trash_chunk(buf); + return 1; + nocast: + chunk_appendf(buf, "(undisplayable)"); + goto done; +} + +// This function checks the "debug" converter's arguments. +static int smp_check_debug(struct arg *args, struct sample_conv *conv, + const char *file, int line, char **err) +{ + const char *name = "buf0"; + struct sink *sink = NULL; + + if (args[0].type != ARGT_STR) { + /* optional prefix */ + args[0].data.str.area = ""; + args[0].data.str.data = 0; } + + if (args[1].type == ARGT_STR) + name = args[1].data.str.area; + + sink = sink_find(name); + if (!sink) { + memprintf(err, "No such sink '%s'", name); + return 0; + } + + args[1].data.str.area = (char *)sink; + args[1].data.str.data = 0; // that's not a string anymore return 1; } -#endif static int sample_conv_base642bin(const struct arg *arg_p, struct sample *smp, void *private) { @@ -3334,10 +3375,7 @@ INITCALL1(STG_REGISTER, sample_register_fetches, &smp_kws); /* Note: must not be declared <const> as its list will be overwritten */ static struct sample_conv_kw_list sample_conv_kws = {ILH, { -#ifdef DEBUG_EXPR - { "debug", sample_conv_debug, 0, NULL, SMP_T_ANY, SMP_T_ANY }, -#endif - + { "debug", sample_conv_debug, ARG2(0,STR,STR), smp_check_debug, SMP_T_ANY, SMP_T_ANY }, { "b64dec", sample_conv_base642bin,0, NULL, SMP_T_STR, SMP_T_BIN }, { "base64", sample_conv_bin2base64,0, NULL, SMP_T_BIN, SMP_T_STR }, { "upper", sample_conv_str2upper, 0, NULL, SMP_T_STR, SMP_T_STR }, -- 2.20.1
