[GitHub] [trafficcontrol] TaylorCFrey commented on a change in pull request #5730: Fixes an issue where multiple interfaces reported by a cache are included in the vitals (bandwidth)

[GitBox <gi...@apache.org>]

TaylorCFrey commented on a change in pull request #5730:
URL: https://github.com/apache/trafficcontrol/pull/5730#discussion_r614334243



##########
File path: traffic_monitor/health/cache.go
##########
@@ -53,14 +53,44 @@ func GetVitals(newResult *cache.Result, prevResult *cache.Result, mc *tc.Traffic
 		return
 	}
 
+	if mc == nil {
+		log.Errorf("mc TrafficMonitorConfigMap cannot be nil")
+		return
+	}
+
 	if newResult.InterfaceVitals == nil {
 		newResult.InterfaceVitals = map[string]cache.Vitals{}
 	}
 
 	// proc.loadavg -- we're using the 1 minute average (!?)
 	newResult.Vitals.LoadAvg = newResult.Statistics.Loadavg.One
 
-	for ifaceName, iface := range newResult.Interfaces() {
+	if mc.TrafficServer[newResult.ID].Interfaces == nil {
+		log.Debugf("no interfaces reported in config map to be monitored")
+		return
+	}
+
+	var monitoredInterfaces []tc.ServerInterfaceInfo

Review comment:
       Excellent question. Ideally with DH servers, this would be the single interface being collected, rather than marking both (or more) of the other interfaces with "Monitor this interface". As such, we don't want to put any restrictions on this value for two reasons:
   1) We can't guarantee the naming of the interfaces. Thus any restrictions would have to be done just on conventions around names (such as `lo`)
   2) Should we want to monitor the `lo` interface in the future, any coded restrictions now would require a codebase change and a deployment, rather than a toggle of a checkbox in Traffic Portal. This does mean there is a potential for User-Error by checking or unchecking the various interfaces in TP to be monitored, but there isn't an elegant approach beyond that for now. I will look into adding a tooltip to the "Monitor this interface" checkbox label in TP to see if that is an appropriate fix to help prevent accidental User-Error




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org