Expand --nstimes option

[tgreenx]

Purpose

This PR proposes to expand the --nstimes option in several ways. See the "Changes" section below.

Note: this option applies only to "sent" queries. Cached queries are not counted.

Context

N/A

Changes

• Extend to all queried name servers: this means that non-queried name servers will not appear, with the exception of name servers of the child or parent zone
• Add classification of name servers ("child", "parent", "other"). Note that name servers shared by the child and parent zone will appear in both categories.
• Add number of queries sent per name server
• Add grand total row for query times and query count
• Refactoring

How to test this PR

Run any test with the --nstimes option. Check that the formatting is done correctly.

Example output:

$ zonemaster-cli --nstimes --raw --no-ipv6 --test basic02 afnic.fr

                           Server       Max      Min      Avg   Stddev   Median     Total     Count
=================================  ======== ======== ======== ======== ======== ========= =========
Child ---------------------------
          g.ext.nic.fr/194.0.36.1      8.73     5.80     6.91     0.72     6.85    103.63        15
      g.ext.nic.fr/2001:678:4c::1      0.00     0.00     0.00     0.00     0.00      0.00         0
           ns1.nic.fr/192.134.4.1      4.51     4.51     4.51     0.00     4.51      4.51         1
  ns1.nic.fr/2001:67c:2218:2::4:1      0.00     0.00     0.00     0.00     0.00      0.00         0
            ns2.nic.fr/192.93.0.4      5.36     5.36     5.36     0.00     5.36      5.36         1
  ns2.nic.fr/2001:660:3005:1::1:2      0.00     0.00     0.00     0.00     0.00      0.00         0
          ns3.nic.fr/192.134.0.49      6.05     6.05     6.05     0.00     6.05      6.05         1
  ns3.nic.fr/2001:660:3006:1::1:1      0.00     0.00     0.00     0.00     0.00      0.00         0
Parent --------------------------
               d.nic.fr/194.0.9.1     12.64    11.78    12.21     0.43    12.21     24.42         2
           d.nic.fr/2001:678:c::1      0.00     0.00     0.00     0.00     0.00      0.00         0
      f.ext.nic.fr/194.146.106.46      0.00     0.00     0.00     0.00     0.00      0.00         0
f.ext.nic.fr/2001:67c:1010:11::53      0.00     0.00     0.00     0.00     0.00      0.00         0
          g.ext.nic.fr/194.0.36.1      8.73     5.80     6.91     0.72     6.85    103.63        15
      g.ext.nic.fr/2001:678:4c::1      0.00     0.00     0.00     0.00     0.00      0.00         0
Other ---------------------------
    a.root-servers.net/198.41.0.4     17.91    17.91    17.91     0.00    17.91     17.91         1
  m.root-servers.net/202.12.27.33      8.23     5.76     6.92     0.57     6.94     96.86        14
=================================  ======== ======== ======== ======== ======== ========= =========
                            Total                                                  258.73        35

[tgreenx]

Could we also have a count of the number of queries answered? (Or conversely, the number of “lost” packets?)

We could, but not with the current implementation of Zonemaster::Engine::Nameserver.

[tgreenx]

@mattias-p @marc-vanderwal Sorry, I've further extended this PR with some new changes, could you make a fresh new review ? Thanks.

[matsduf]

What are the use cases for using --nstimes?

Note: this option applies only to "sent" queries. Cached queries are not counted.

Is there any technical reason to exclude those? Otherwise it could be reasonable to see the queries to cache too. Will there be any difference if global or local cache is used? If so, it can be confusing when using global cache.

[mattias-p]

Note: this option applies only to "sent" queries. Cached queries are not counted.

Is there any technical reason to exclude those? Otherwise it could be reasonable to see the queries to cache too.

AFAICT what is actually interesting is the number of distinct requests. That number should to be the same whether or not recorded data is used, and whether or not intermediate results within Engine are cached. I believe the number of queries that are actually sent, the number of queries that would be sent if we didn't cache, and the sum of those are of little interest.

[matsduf]

AFAICT what is actually interesting is the number of distinct requests. That number should to be the same whether or not recorded data is used, and whether or not intermediate results within Engine are cached. I believe the number of queries that are actually sent, the number of queries that would be sent if we didn't cache, and the sum of those are of little interest.

@mattias-p, what use case do consider when writing this? Do you see other use cases of the --ns-times option?

[mattias-p]

AFAICT what is actually interesting is the number of distinct requests. That number should to be the same whether or not recorded data is used, and whether or not intermediate results within Engine are cached. I believe the number of queries that are actually sent, the number of queries that would be sent if we didn't cache, and the sum of those are of little interest.

@mattias-p, what use case do consider when writing this? Do you see other use cases of the --ns-times option?

I'm not really thinking about use cases. I'm thinking about how to make meaningful measurements. A measurement is more meaningful when it is variable in a single dimension and isn't influenced by multiple different things. So I meant to suggest that we could define the count so that it isn't affected by whether caching options and implementation details.

[matsduf]

I think it is reasonable to have some use cases in mind when defining what figures to extract and present. Maybe it could be useful to distinguish between queries that are responded from cache and those that require sending a message externally.

[tgreenx]

@mattias-p @matsduf I should have addressed most comments, and I've also made small refactoring/improvements, see https://github.com/zonemaster/zonemaster-cli/pull/421/commits/b0c1bf2540dc2be2cee08e9cbc73c2d339f9b237. Please re-review.

And regarding the use cases, as Mattias pointed out, the purpose of this option is to measure response times of name servers, so that automatically excludes cached queries.

[tgreenx]

@mattias-p @matsduf @marc-vanderwal please (re-)review.

[tgreenx]

@matsduf @mattias-p @marc-vanderwal Rebased on latest develop, and addressed review comments (see https://github.com/zonemaster/zonemaster-cli/pull/421/commits/acf65f511822f78d551d2ba82d8dd753790c5ba4). Please re-review.

[tgreenx]

@matsduf @mattias-p @marc-vanderwal please re-review this PR.

[matsduf]

This looks fine as function, but I think there is a lack of documentation. I am not sure that the best place is to add documentation into the scripts. Rather I think a markdown document in the document tree would be better, but a reference from here. Please share your thoughts of documentation and I am ready to approve.

So far I think all the options are described that way. I could attempt to extend the documentation of this option in a future PR, although I don't immediately see what is missing. I think the current option description and its output are rather self-explanatory, considering that I've added explicit headers to the output.

Please look at it from a "foreign" eye, and then the output is far from self-explanatory.

[matsduf]

This can be merged, but three issues to be created:

• Create detailed user documentation and link to that from the script in v2025.2.
• Refactoring of the table printing code to better support translations and increase maintainability. -> https://github.com/zonemaster/zonemaster-cli/issues/439
• Refactoring of section mapping of data for both outputs. Grand total is to be included in the JSON output. -> https://github.com/zonemaster/zonemaster-cli/issues/440

[marc-vanderwal]

This can be merged, but three issues to be created: […]

* Refactoring of the table printing code to better support translations and increase maintainability.

I created #439 to track that.

[tgreenx]

[ ... ] * Refactoring of section mapping of data for both outputs. Grand total is to be included in the JSON output.

I created https://github.com/zonemaster/zonemaster-cli/issues/440 for this.

[marc-vanderwal]

Successfully release tested this PR on Rocky Linux 9.

Using the command line in the test procedure, the output I obtained looks similar to the example, albeit with different numbers. The formatting of the table looks correct, though currently, none of the strings are translated.