Andrew, My appologies about the proxy_find_by_name function, I was not in the right context!!! Tested and approved.
Willy, you can apply :) Thanks a lot for your contribution, Andrew. Baptiste On Mon, Oct 5, 2015 at 5:47 PM, Andrew Hayworth <[email protected]> wrote: > On Mon, Oct 5, 2015 at 7:24 AM, Baptiste <[email protected]> wrote: >> Hi, >> >> No problem for me about the feature itself. >> That said, a few things should be changed in the code: >> >> - use of proxy_find_by_name() instead of parsing the proxy list > > I'm fairly certain that 'proxy_find_by_name' does not search the > dns_resolvers list (both from reading the code, and from empirically > testing it). Notably, this looping-through-the-list behavior was > already present in src/dumpstats.c before I touched it, and we also do > the same thing when parsing the config files. I _do_ believe we should > have a nice function for finding a resolvers section by name (either > 'resolver_find_by_name' or by extending 'proxy_find_by_name'), but I > don't think this is the commit to do that right before a release. > >> - the following statement is hardly readable: "if >> (appctx->ctx.resolvers.ptr != NULL && appctx->ctx.resolvers.ptr != >> presolvers) continue;" >> Please write "continue" on a new line. > > Done. > >> >> Please repost an updated patch and I'll give it a try before final approval. >> >> Baptiste > > Updated patch below: > > From 190fee509a81755a8be3d9281c2edd7d3f72ff19 Mon Sep 17 00:00:00 2001 > From: Andrew Hayworth <[email protected]> > Date: Fri, 2 Oct 2015 20:33:01 +0000 > Subject: [PATCH] MINOR: cli: Dump all resolvers stats if no resolver section > is given > > This commit adds support for dumping all resolver stats. Specifically > if a command 'show stats resolvers' is issued withOUT a resolver section > id, we dump all known resolver sections. If none are configured, a > message is displayed indicating that. > --- > doc/configuration.txt | 6 +++-- > src/dumpstats.c | 73 > +++++++++++++++++++++++++++------------------------ > 2 files changed, 43 insertions(+), 36 deletions(-) > > diff --git a/doc/configuration.txt b/doc/configuration.txt > index 3102516..e519662 100644 > --- a/doc/configuration.txt > +++ b/doc/configuration.txt > @@ -16043,8 +16043,10 @@ show stat [<iid> <type> <sid>] > A similar empty line appears at the end of the second block (stats) so > that > the reader knows the output has not been truncated. > > -show stat resolvers <resolvers section id> > - Dump statistics for the given resolvers section. > +show stat resolvers [<resolvers section id>] > + Dump statistics for the given resolvers section, or all resolvers sections > + if no section is supplied. > + > For each name server, the following counters are reported: > sent: number of DNS requests sent to this server > valid: number of DNS valid responses received from this server > diff --git a/src/dumpstats.c b/src/dumpstats.c > index bdfb7e3..ff44120 100644 > --- a/src/dumpstats.c > +++ b/src/dumpstats.c > @@ -1166,23 +1166,19 @@ static int stats_sock_parse_request(struct > stream_interface *si, char *line) > if (strcmp(args[2], "resolvers") == 0) { > struct dns_resolvers *presolvers; > > - if (!*args[3]) { > - appctx->ctx.cli.msg = "Missing resolver section identifier.\n"; > - appctx->st0 = STAT_CLI_PRINT; > - return 1; > - } > - > - appctx->ctx.resolvers.ptr = NULL; > - list_for_each_entry(presolvers, &dns_resolvers, list) { > - if (strcmp(presolvers->id, args[3]) == 0) { > - appctx->ctx.resolvers.ptr = presolvers; > - break; > + if (*args[3]) { > + appctx->ctx.resolvers.ptr = NULL; > + list_for_each_entry(presolvers, &dns_resolvers, list) { > + if (strcmp(presolvers->id, args[3]) == 0) { > + appctx->ctx.resolvers.ptr = presolvers; > + break; > + } > + } > + if (appctx->ctx.resolvers.ptr == NULL) { > + appctx->ctx.cli.msg = "Can't find that resolvers section\n"; > + appctx->st0 = STAT_CLI_PRINT; > + return 1; > } > - } > - if (appctx->ctx.resolvers.ptr == NULL) { > - appctx->ctx.cli.msg = "Can't find resolvers section.\n"; > - appctx->st0 = STAT_CLI_PRINT; > - return 1; > } > > appctx->st2 = STAT_ST_INIT; > @@ -6400,24 +6396,33 @@ static int > stats_dump_resolvers_to_buffer(struct stream_interface *si) > /* fall through */ > > case STAT_ST_LIST: > - presolvers = appctx->ctx.resolvers.ptr; > - chunk_appendf(&trash, "Resolvers section %s\n", presolvers->id); > - list_for_each_entry(pnameserver, &presolvers->nameserver_list, list) { > - chunk_appendf(&trash, " nameserver %s:\n", pnameserver->id); > - chunk_appendf(&trash, " sent: %ld\n", pnameserver->counters.sent); > - chunk_appendf(&trash, " valid: %ld\n", pnameserver->counters.valid); > - chunk_appendf(&trash, " update: %ld\n", pnameserver->counters.update); > - chunk_appendf(&trash, " cname: %ld\n", pnameserver->counters.cname); > - chunk_appendf(&trash, " cname_error: %ld\n", > pnameserver->counters.cname_error); > - chunk_appendf(&trash, " any_err: %ld\n", pnameserver->counters.any_err); > - chunk_appendf(&trash, " nx: %ld\n", pnameserver->counters.nx); > - chunk_appendf(&trash, " timeout: %ld\n", pnameserver->counters.timeout); > - chunk_appendf(&trash, " refused: %ld\n", pnameserver->counters.refused); > - chunk_appendf(&trash, " other: %ld\n", pnameserver->counters.other); > - chunk_appendf(&trash, " invalid: %ld\n", pnameserver->counters.invalid); > - chunk_appendf(&trash, " too_big: %ld\n", pnameserver->counters.too_big); > - chunk_appendf(&trash, " truncated: %ld\n", > pnameserver->counters.truncated); > - chunk_appendf(&trash, " outdated: %ld\n", pnameserver->counters.outdated); > + if (LIST_ISEMPTY(&dns_resolvers)) { > + chunk_appendf(&trash, "No resolvers found\n"); > + } > + else { > + list_for_each_entry(presolvers, &dns_resolvers, list) { > + if (appctx->ctx.resolvers.ptr != NULL && appctx->ctx.resolvers.ptr > != presolvers) > + continue; > + > + chunk_appendf(&trash, "Resolvers section %s\n", presolvers->id); > + list_for_each_entry(pnameserver, &presolvers->nameserver_list, list) { > + chunk_appendf(&trash, " nameserver %s:\n", pnameserver->id); > + chunk_appendf(&trash, " sent: %ld\n", pnameserver->counters.sent); > + chunk_appendf(&trash, " valid: %ld\n", pnameserver->counters.valid); > + chunk_appendf(&trash, " update: %ld\n", pnameserver->counters.update); > + chunk_appendf(&trash, " cname: %ld\n", pnameserver->counters.cname); > + chunk_appendf(&trash, " cname_error: %ld\n", > pnameserver->counters.cname_error); > + chunk_appendf(&trash, " any_err: %ld\n", pnameserver->counters.any_err); > + chunk_appendf(&trash, " nx: %ld\n", pnameserver->counters.nx); > + chunk_appendf(&trash, " timeout: %ld\n", pnameserver->counters.timeout); > + chunk_appendf(&trash, " refused: %ld\n", pnameserver->counters.refused); > + chunk_appendf(&trash, " other: %ld\n", pnameserver->counters.other); > + chunk_appendf(&trash, " invalid: %ld\n", pnameserver->counters.invalid); > + chunk_appendf(&trash, " too_big: %ld\n", pnameserver->counters.too_big); > + chunk_appendf(&trash, " truncated: %ld\n", > pnameserver->counters.truncated); > + chunk_appendf(&trash, " outdated: %ld\n", pnameserver->counters.outdated); > + } > + } > } > > /* display response */ > -- > 2.1.3
