You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by mi...@apache.org on 2020/12/14 18:40:55 UTC

[trafficcontrol] branch master updated: TO Better log messages when querying TM CacheStats (#5347)

This is an automated email from the ASF dual-hosted git repository.

mitchell852 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/master by this push:
     new a9b3ec2  TO Better log messages when querying TM CacheStats (#5347)
a9b3ec2 is described below

commit a9b3ec2d7db454b0c8df679d0a25f0ae38c6db51
Author: Steve Hamrick <sh...@users.noreply.github.com>
AuthorDate: Mon Dec 14 11:40:42 2020 -0700

    TO Better log messages when querying TM CacheStats (#5347)
    
    TO Better log messages when querying TM CacheStats
---
 CHANGELOG.md                                       |  4 +++
 .../traffic_ops_golang/cachesstats/cachesstats.go  |  4 +--
 traffic_ops/traffic_ops_golang/cdn/capacity.go     | 19 ++++++-----
 .../traffic_ops_golang/deliveryservice/capacity.go |  4 +--
 .../util/monitorhlp/monitorhlp.go                  | 37 +++++++++++++---------
 5 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5a63247..b40a5f6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
     of just column filters
 - #2881 Some API endpoints have incorrect Content-Types
 
+### Fixed
+- [#5311](https://github.com/apache/trafficcontrol/issues/5311) - Better TO log messages when failures calling TM 
+    CacheStats
+
 ## [5.0.0] - 2020-10-20
 ### Added
 - Traffic Ops Ort: Disabled ntpd verification (ntpd is deprecated in CentOS)
diff --git a/traffic_ops/traffic_ops_golang/cachesstats/cachesstats.go b/traffic_ops/traffic_ops_golang/cachesstats/cachesstats.go
index 0f49031..540755d 100644
--- a/traffic_ops/traffic_ops_golang/cachesstats/cachesstats.go
+++ b/traffic_ops/traffic_ops_golang/cachesstats/cachesstats.go
@@ -80,9 +80,9 @@ func getCachesStats(tx *sql.Tx) ([]CacheData, error) {
 
 			var cacheStats tc.Stats
 			stats := []string{ATSCurrentConnectionsStat, tc.StatNameBandwidth}
-			cacheStats, err = monitorhlp.GetCacheStats(monitorFQDN, client, stats)
+			cacheStats, _, err = monitorhlp.GetCacheStats(monitorFQDN, client, stats)
 			if err != nil {
-				legacyCacheStats, err := monitorhlp.GetLegacyCacheStats(monitorFQDN, client, stats)
+				legacyCacheStats, _, err := monitorhlp.GetLegacyCacheStats(monitorFQDN, client, stats)
 				if err != nil {
 					errs = append(errs, errors.New("getting CacheStats for CDN '"+string(cdn)+"' monitor '"+monitorFQDN+"': "+err.Error()))
 					continue
diff --git a/traffic_ops/traffic_ops_golang/cdn/capacity.go b/traffic_ops/traffic_ops_golang/cdn/capacity.go
index 32ce06e..d4bc4d2 100644
--- a/traffic_ops/traffic_ops_golang/cdn/capacity.go
+++ b/traffic_ops/traffic_ops_golang/cdn/capacity.go
@@ -105,7 +105,7 @@ func getMonitorsCapacity(tx *sql.Tx, monitors map[tc.CDNName][]string) (Capacity
 		return CapacityResp{}, errors.New("getting profile thresholds: " + err.Error())
 	}
 
-	cap, err := getCapacityData(monitors, thresholds, client, tx)
+	cap, err := getCapacityData(monitors, thresholds, client, tx, monitorForwardProxy)
 	if err != nil {
 		return CapacityResp{}, errors.New("getting capacity from monitors: " + err.Error())
 	} else if cap.Capacity == 0 {
@@ -123,7 +123,7 @@ func getMonitorsCapacity(tx *sql.Tx, monitors map[tc.CDNName][]string) (Capacity
 // getCapacityData attempts to get the CDN capacity from each monitor. If one fails, it tries the next.
 // The first monitor for which all data requests succeed is used.
 // Only if all monitors for a CDN fail is an error returned, from the last monitor tried.
-func getCapacityData(monitors map[tc.CDNName][]string, thresholds map[string]float64, client *http.Client, tx *sql.Tx) (CapData, error) {
+func getCapacityData(monitors map[tc.CDNName][]string, thresholds map[string]float64, client *http.Client, tx *sql.Tx, forwardProxyURL string) (CapData, error) {
 	cap := CapData{}
 	for cdn, monitorFQDNs := range monitors {
 		err := error(nil)
@@ -142,13 +142,16 @@ func getCapacityData(monitors map[tc.CDNName][]string, thresholds map[string]flo
 				continue
 			}
 			statsToFetch := []string{tc.StatNameKBPS, tc.StatNameMaxKBPS}
-			if cacheStats, err = monitorhlp.GetCacheStats(monitorFQDN, client, statsToFetch); err != nil {
-				err = errors.New("getting cache stats for CDN '" + string(cdn) + "' monitor '" + monitorFQDN + "': " + err.Error())
-				log.Warnln("getCapacity failed to get CacheStatsNew from cdn '" + string(cdn) + " monitor '" + monitorFQDN + "', trying CacheStats" + err.Error())
-				legacyCacheStats, err := monitorhlp.GetLegacyCacheStats(monitorFQDN, client, statsToFetch)
+			var monitorEndpoint string
+			if cacheStats, monitorEndpoint, err = monitorhlp.GetCacheStats(monitorFQDN, client, statsToFetch); err != nil {
+				proxyErr := ""
+				if forwardProxyURL != "" {
+					proxyErr = "using http proxy: " + forwardProxyURL + ", "
+				}
+				log.Warnln(proxyErr + "getCapacity failed to get '" + monitorEndpoint + "' from cdn '" + string(cdn) + "', Error: " + err.Error() + ", trying CacheStats")
+				legacyCacheStats, monitorEndpoint, err := monitorhlp.GetLegacyCacheStats(monitorFQDN, client, statsToFetch)
 				if err != nil {
-					err = errors.New("getting cache stats for CDN '" + string(cdn) + "' monitor '" + monitorFQDN + "': " + err.Error())
-					log.Warnln("getCapacity failed to get CacheStats from cdn '" + string(cdn) + " monitor '" + monitorFQDN + "', trying next monitor: " + err.Error())
+					log.Warnln(proxyErr + "getCapacity failed to get '" + monitorEndpoint + "' from cdn '" + string(cdn) + "', Error: " + err.Error())
 					continue
 				}
 				cacheStats = monitorhlp.UpgradeLegacyStats(legacyCacheStats)
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/capacity.go b/traffic_ops/traffic_ops_golang/deliveryservice/capacity.go
index 2250136..584594e 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/capacity.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/capacity.go
@@ -114,9 +114,9 @@ func getCapacity(tx *sql.Tx, ds tc.DeliveryServiceName, cdn tc.CDNName) (Capacit
 		return CapacityResp{}, errors.New("getting CRConfig for delivery service '" + string(ds) + "' monitor '" + monitorFQDN + "': " + err.Error())
 	}
 	statsoFetch := []string{tc.StatNameMaxKBPS, tc.StatNameKBPS}
-	cacheStats, err := monitorhlp.GetCacheStats(monitorFQDN, client, statsoFetch)
+	cacheStats, _, err := monitorhlp.GetCacheStats(monitorFQDN, client, statsoFetch)
 	if err != nil {
-		legacyCacheStats, err := monitorhlp.GetLegacyCacheStats(monitorFQDN, client, statsoFetch)
+		legacyCacheStats, _, err := monitorhlp.GetLegacyCacheStats(monitorFQDN, client, statsoFetch)
 		if err != nil {
 			return CapacityResp{}, errors.New("getting CacheStats for delivery service '" + string(ds) + "' monitor '" + monitorFQDN + "': " + err.Error())
 		}
diff --git a/traffic_ops/traffic_ops_golang/util/monitorhlp/monitorhlp.go b/traffic_ops/traffic_ops_golang/util/monitorhlp/monitorhlp.go
index 3c43a97..3859509 100644
--- a/traffic_ops/traffic_ops_golang/util/monitorhlp/monitorhlp.go
+++ b/traffic_ops/traffic_ops_golang/util/monitorhlp/monitorhlp.go
@@ -34,9 +34,14 @@ import (
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
 )
 
-const MonitorProxyParameter = "tm.traffic_mon_fwd_proxy"
-const MonitorRequestTimeout = time.Second * 10
-const MonitorOnlineStatus = "ONLINE"
+const (
+	MonitorProxyParameter = "tm.traffic_mon_fwd_proxy"
+	MonitorRequestTimeout = time.Second * 10
+	MonitorOnlineStatus   = "ONLINE"
+
+	TrafficMonitorCacheStatsPath       = "/publish/CacheStatsNew"
+	TrafficMonitorLegacyCacheStatsPath = "/publish/CacheStats"
+)
 
 // GetClient returns the http.Client for making requests to the Traffic Monitor. This should always be used, rather than creating a default http.Client, to ensure any monitor forward proxy parameter is used correctly.
 func GetClient(tx *sql.Tx) (*http.Client, error) {
@@ -124,40 +129,42 @@ func GetCRConfig(monitorFQDN string, client *http.Client) (tc.CRConfig, error) {
 
 // GetCacheStats gets the cache stats from the given monitor. The stats parameters is which stats to get;
 // if stats is empty or nil, all stats are fetched.
-func GetCacheStats(monitorFQDN string, client *http.Client, stats []string) (tc.Stats, error) {
-	path := `/publish/CacheStatsNew?hc=1`
+func GetCacheStats(monitorFQDN string, client *http.Client, stats []string) (tc.Stats, string, error) {
+	path := TrafficMonitorCacheStatsPath + "?hc=1"
 	if len(stats) > 0 {
 		path += `&stats=` + strings.Join(stats, `,`)
 	}
-	resp, err := client.Get("http://" + monitorFQDN + path)
+	path = "http://" + monitorFQDN + path
+	resp, err := client.Get(path)
 	if err != nil {
-		return tc.Stats{}, errors.New("getting CacheStatsNew from Monitor '" + monitorFQDN + "': " + err.Error())
+		return tc.Stats{}, path, errors.New("getting CacheStatsNew from Monitor '" + monitorFQDN + "': " + err.Error())
 	}
 	defer resp.Body.Close()
 	cacheStats := tc.Stats{}
 	if err := json.NewDecoder(resp.Body).Decode(&cacheStats); err != nil {
-		return tc.Stats{}, errors.New("decoding CacheStatsNew from monitor '" + monitorFQDN + "': " + err.Error())
+		return tc.Stats{}, path, errors.New("decoding CacheStatsNew from monitor '" + monitorFQDN + "': " + err.Error())
 	}
-	return cacheStats, nil
+	return cacheStats, path, nil
 }
 
 // GetLegacyCacheStats gets the pre ATCv5.0 cache stats from the given monitor. The stats parameters is which stats to
 // get; if stats is empty or nil, all stats are fetched.
-func GetLegacyCacheStats(monitorFQDN string, client *http.Client, stats []string) (tc.LegacyStats, error) {
-	path := `/publish/CacheStats?hc=1`
+func GetLegacyCacheStats(monitorFQDN string, client *http.Client, stats []string) (tc.LegacyStats, string, error) {
+	path := TrafficMonitorLegacyCacheStatsPath + "?hc=1"
 	if len(stats) > 0 {
 		path += `&stats=` + strings.Join(stats, `,`)
 	}
-	resp, err := client.Get("http://" + monitorFQDN + path)
+	path = "http://" + monitorFQDN + path
+	resp, err := client.Get(path)
 	if err != nil {
-		return tc.LegacyStats{}, errors.New("getting CacheStats from Monitor '" + monitorFQDN + "': " + err.Error())
+		return tc.LegacyStats{}, path, errors.New("getting CacheStats from Monitor '" + monitorFQDN + "': " + err.Error())
 	}
 	defer resp.Body.Close()
 	cacheStats := tc.LegacyStats{}
 	if err := json.NewDecoder(resp.Body).Decode(&cacheStats); err != nil {
-		return tc.LegacyStats{}, errors.New("decoding CacheStats from monitor '" + monitorFQDN + "': " + err.Error())
+		return tc.LegacyStats{}, path, errors.New("decoding CacheStats from monitor '" + monitorFQDN + "': " + err.Error())
 	}
-	return cacheStats, nil
+	return cacheStats, path, nil
 }
 
 // UpgradeLegacyStats will take LegacyStats and transform them to Stats. It assumes all stats that go in