Add format codes for local FQDN#2429
Conversation
| <A Server FQDN or peer name | ||
| <p Server port number of the last server or peer connection | ||
| <la Local IP address of the last server or peer connection | ||
| <lA Local FQDN for address of the last server or peer connection |
There was a problem hiding this comment.
Here and elsewhere in this PR, please avoid the term "local FQDN". It is confusing (e.g., was some special a "local" DNS resolver used to obtain that FQDN?) and not descriptive enough ("address of the last server or peer connection" matches two different addresses, not one).
I do not insist on any specific wording, but something like this would address the problem:
| <lA Local FQDN for address of the last server or peer connection | |
| <lA FQDN for the local (a.k.a. source) address of the last Squid-to-server (including Squid-to-cache_peer) TCP connection |
N.B. LFT_CLIENT_LOCAL_FQDN and LFT_SERVER_LOCAL_FQDN do not need to be renamed. In that existing naming pattern, "local" binds to "from-client" and "to-server" connection rather than binding to "FQDN".
| break; | ||
|
|
||
| case LFT_CLIENT_LOCAL_FQDN: | ||
| // May be too late for ours, but FQDN_LOOKUP_IF_MISS might help the next caller. |
There was a problem hiding this comment.
Please mimic existing Dns::ResolveClientAddressesAsap logic to enable "early" or "as soon as possible" DNS lookups of these addresses.
I suggest naming the new flag ResolveClientLocalAddressesAsap and renaming the old flag to ResolveClientRemoteAddressesAsap. Using ResolveLocalClientAddressesAsap and ResolveRemoteClientAddressesAsap is also OK but does not match our LFT_CLIENT_LOCAL_FQDN naming pattern.
Same for tcpServer->local lookups, of course -- Dns::ResolveServerLocalAddressesAsap.
| >p Client source port | ||
| >eui Client source EUI (MAC address, EUI-48 or EUI-64 identifier) | ||
| >la Local IP address the client connected to | ||
| >lA Local FQDN for address the client connected to |
There was a problem hiding this comment.
Please document (in cf.dada.pre) that these FQDNs are provided on a best-effort basis (i.e. that, when the corresponding DNS lookups are needed, Squid does not wait for them to complete when logging transactions).
| TokenTableEntry("lp", LFT_LOCAL_LISTENING_PORT), | ||
|
|
||
| TokenTableEntry("la", LFT_LOCAL_LISTENING_IP), | ||
| /*TokenTableEntry( "lA", LFT_LOCAL_NAME ), */ |
There was a problem hiding this comment.
Please remove this old commented out code, especially since this PR is already moving around unchanged entries. Even if LFT_LOCAL_NAME was meant to represent something other than one of the %codes added in this PR, it is very unlikely to be added under that old name, especially after this PR is merged...
| /*TokenTableEntry( "lA", LFT_LOCAL_NAME ), */ |
| LFT_CLIENT_LOCAL_IP, | ||
| LFT_CLIENT_LOCAL_FQDN, | ||
| LFT_CLIENT_LOCAL_PORT, | ||
| /*LFT_CLIENT_LOCAL_FQDN, (rDNS) */ |
There was a problem hiding this comment.
| /*LFT_CLIENT_LOCAL_FQDN, (rDNS) */ |
rousskov
added
the
S-waiting-for-author
2 participants
No description provided.