Cleanup strategy debug logs (#8656)

- fixes parent_select plugin to only have 1 debug tag
- clarifies a misleading debug log about parent selection
- changes parent_select plugin to not unnecessarily call nextHopExists

Author: rob05c

plugins/experimental/parent_select/parent_select.cc

@@ -87,7 +87,7 @@ handle_send_request(TSHttpTxn txnp, StrategyTxn *strategyTxn)
   strategy->next(txnp, strategyTxn->txn, ra.hostname, ra.hostname_len, ra.port, &ra.hostname, &ra.hostname_len, &ra.port,
                  &ra.is_retry, &ra.no_cache);
 
-  ra.nextHopExists = strategy->nextHopExists(txnp);
+  ra.nextHopExists = ra.hostname_len != 0;
   ra.fail = !ra.nextHopExists; // failed is whether to fail and return to the client. failed=false means to retry the parent we set
                                // in the response_action
 
@@ -256,7 +256,7 @@ handle_os_dns(TSHttpTxn txnp, StrategyTxn *strategyTxn)
 
   ra.fail = ra.hostname == nullptr; // failed is whether to immediately fail and return the client a 502. In this case: whether or
                                     // not we found another parent.
-  ra.nextHopExists       = strategy->nextHopExists(txnp);
+  ra.nextHopExists       = ra.hostname_len != 0;
   ra.responseIsRetryable = strategy->responseIsRetryable(strategyTxn->request_count, STATUS_CONNECTION_FAILURE);
   ra.goDirect            = strategy->goDirect();
   ra.parentIsProxy       = strategy->parentIsProxy();

plugins/experimental/parent_select/strategy.cc

@@ -287,8 +287,10 @@ PLNextHopSelectionStrategy::nextHopExists(TSHttpTxn txnp)
     for (auto &hh : host_groups[gg]) {
       PLHostRecord *p = hh.get();
       if (p->available) {
-        PL_NH_Debug(PL_NH_DEBUG_TAG, "[%" PRIu64 "] found available next hop %.*s", sm_id, int(p->hostname.size()),
-                    p->hostname.c_str());
+        PL_NH_Debug(PL_NH_DEBUG_TAG,
+                    "[%" PRIu64 "] found available next hop %.*s (this is NOT necessarily the parent which will be selected, just "
+                    "the first available parent found)",
+                    sm_id, int(p->hostname.size()), p->hostname.c_str());
         return true;
       }
     }

plugins/experimental/parent_select/strategy.h

@@ -36,7 +36,7 @@
 // TODO rename, move to respective sub-plugins
 #define PLUGIN_NAME "pparent_select"
 
-constexpr const char *PL_NH_DEBUG_TAG = "plugin_nexthop";
+constexpr const char *PL_NH_DEBUG_TAG = PLUGIN_NAME;
 
 // ring mode strings
 extern const std::string_view alternate_rings;

proxy/http/remap/NextHopSelectionStrategy.cc

@@ -278,7 +278,10 @@ NextHopSelectionStrategy::nextHopExists(TSHttpTxn txnp, void *ih)
     for (auto &hh : host_groups[gg]) {
       HostRecord *p = hh.get();
       if (p->available.load()) {
-        NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] found available next hop %s", sm_id, p->hostname.c_str());
+        NH_Debug(NH_DEBUG_TAG,
+                 "[%" PRIu64 "] found available next hop %s (this is NOT necessarily the parent which will be selected, just the "
+                 "first available parent found)",
+                 sm_id, p->hostname.c_str());
         return true;
       }
     }

tests/gold_tests/pluginTest/parent_select/parent_select_peer.test.py

@@ -80,7 +80,7 @@
 
     ts.Disk.records_config.update({
         'proxy.config.diags.debug.enabled': 1,
-        'proxy.config.diags.debug.tags': 'http|dns|parent|next_hop|host_statuses|hostdb|cachekey|plugin_nexthop',
+        'proxy.config.diags.debug.tags': 'http|dns|parent|next_hop|host_statuses|hostdb|cachekey|pparent_select',
         'proxy.config.dns.nameservers': f"127.0.0.1:{dns.Variables.Port}",  # Only nameservers if resolv_conf NULL.
         'proxy.config.dns.resolv_conf': "NULL",  # This defaults to /etc/resvolv.conf (OS namesevers) if not NULL.
         'proxy.config.http.cache.http': 1,
@@ -171,7 +171,7 @@
 tr = Test.AddTestRun()
 tr.Processes.Default.Command = (
     "grep -e '^+++' -e '^[A-Z].*TTP/' -e '^.alts. --' -e 'PARENT_SPECIFIED' trace_peer*.log"
-    " | sed 's/^.*(plugin_nexthop) [^ ]* //' | sed 's/[.][0-9]*$$//'"
+    " | sed 's/^.*(pparent_select) [^ ]* //' | sed 's/[.][0-9]*$$//'"
 )
 tr.Processes.Default.Streams.stdout = "peer.trace.gold"
 tr.Processes.Default.ReturnCode = 0

tests/gold_tests/pluginTest/parent_select/parent_select_peer2.test.py

@@ -80,7 +80,7 @@
 
     ts.Disk.records_config.update({
         'proxy.config.diags.debug.enabled': 1,
-        'proxy.config.diags.debug.tags': 'http|dns|parent|next_hop|host_statuses|hostdb|plugin_nexthop',
+        'proxy.config.diags.debug.tags': 'http|dns|parent|next_hop|host_statuses|hostdb|pparent_select',
         'proxy.config.dns.nameservers': f"127.0.0.1:{dns.Variables.Port}",  # Only nameservers if resolv_conf NULL.
         'proxy.config.dns.resolv_conf': "NULL",  # This defaults to /etc/resvolv.conf (OS namesevers) if not NULL.
         'proxy.config.http.cache.http': 1,
@@ -166,7 +166,7 @@
 tr = Test.AddTestRun()
 tr.Processes.Default.Command = (
     "grep -e '^+++' -e '^[A-Z].*TTP/' -e '^.alts. --' -e 'PARENT_SPECIFIED' trace_peer*.log"
-    " | sed 's/^.*(plugin_nexthop) [^ ]* //' | sed 's/[.][0-9]*$$//' " + normalize_ports
+    " | sed 's/^.*(pparent_select) [^ ]* //' | sed 's/[.][0-9]*$$//' " + normalize_ports
 )
 tr.Processes.Default.Streams.stdout = "peer2.trace.gold"
 tr.Processes.Default.ReturnCode = 0