Improvement: Including query time in dnstap CLIENT_RESPONSE messages
Description
While the dnstap specification recommends including the query time for AUTH_RESPONSE, RESOLVER_RESPONSE and CLIENT_RESPONSE dnstap messages, the latter is excluded.
Having the query time in CLIENT_RESPONSE dnstap messages would be very useful when using dnstap to keep track of response times.
Request
In lib/dns/dnstap.c (both for 9.16 and 9.18) the dns_dt_send function accepts the qtime and rtime parameters.
However, when building the dnstap message, CLIENT_RESPONSE messages are prevented from using the qtime parameter.
` dm.m.has_response_time_sec = 1; dm.m.response_time_nsec = isc_time_nanoseconds(t); dm.m.has_response_time_nsec = 1;
/*
* Types CR, RR, and FR can fall through and get the query
* time set as well. Any other response type, break.
*/
if (msgtype != DNS_DTTYPE_RR && msgtype != DNS_DTTYPE_FR
&& msgtype != DNS_DTTYPE_CR) { // << I HAVE ADDED THIS!
break;
}
FALLTHROUGH;
case DNS_DTTYPE_AQ:
case DNS_DTTYPE_CQ:
case DNS_DTTYPE_FQ:
case DNS_DTTYPE_RQ:
case DNS_DTTYPE_SQ:
case DNS_DTTYPE_TQ:
case DNS_DTTYPE_UQ:
if (qtime != NULL) {
t = qtime;
}
dm.m.query_time_sec = isc_time_seconds(t);
dm.m.has_query_time_sec = 1;
dm.m.query_time_nsec = isc_time_nanoseconds(t);
dm.m.has_query_time_nsec = 1;
break;
` I have tried making the simple change shown above (so that qtime is considered for CLIENT_RESPONSE messages as well) and it works both for 9.16.35 and 9.18.9.
The change looks safe enough (it won´t crash because if qtime is NULL t will contain a timestamp obtained when dns_dt_send() is invoked) and at worst it would contain a false qtime.
A more correct alternative would be to include it for CLIENT_RESPONSE messages only if qtime != NULL. But I don´t know whether it can happen or all the calls to dns_dt_send() will contain qtime.
Also, is it possible for qtime to be missing for a CLIENT_RESPONSE but not for a RESOLVER_RESPONSE? Because for a RESOLVER_RESPONSE it would mean that query time in the dnstap message would contain the timestamp obtained in dns_dt_send() and, being probably greater than the response time itself that would botch a time difference calculation.