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 - 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. Please repost an updated patch and I'll give it a try before final approval. Baptiste On Sun, Oct 4, 2015 at 11:00 AM, Willy Tarreau <[email protected]> wrote: > At first glance it seems OK. Baptiste, can you please quickly check > and let me know if you don't see any issue there so that I can merge > it ? > > Willy > > On Fri, Oct 02, 2015 at 03:41:06PM -0500, Andrew Hayworth wrote: >> Hi all - >> >> Below is a patch for the 'show stat resolvers' cli command. It changes >> the command such that it will show all resolvers configured, if you do >> not specify a resolver id. >> >> I found this useful for debugging, and plan on using it in the hatop tool. >> >> Let me know if you have any feedback on it! >> >> Thanks - >> >> Andrew Hayworth >> -- >> >> >From c4061d948d21cabb95f093b5d9655c9d226724af Mon Sep 17 00:00:00 2001 >> From: Andrew Hayworth <[email protected]> >> Date: Fri, 2 Oct 2015 20:33:01 +0000 >> Subject: [PATCH 1/1] 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 | 72 >> +++++++++++++++++++++++++++------------------------ >> 2 files changed, 42 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 1a39258..ea3f49a 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; >> @@ -6402,24 +6398,32 @@ 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 > >
