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
0001-MINOR-cli-Dump-all-resolvers-stats-if-no-resolver-se.patch
Description: Binary data
