cleanup: remove <requestType>Errors metrics in favour of status_codes…

….* ones as more reliable. In our clusters FindErrors erroneously showed zero while there were lots of http404s. In general those metrics are less reliable than status_codes.* ones since they depend on having code to increment it on each error scenario which is easy to miss.
go-graphite · Feb 8, 2023 · 0c92755 · 0c92755
1 parent d23b5b1
commit 0c92755
Show file tree

Hide file tree

Showing 7 changed files with 0 additions and 41 deletions.
diff --git a/carbonserver/carbonserver.go b/carbonserver/carbonserver.go
@@ -67,19 +67,13 @@ import (
 
 type metricStruct struct {
 	RenderRequests               uint64
-	RenderErrors                 uint64
 	NotFound                     uint64
 	FindRequests                 uint64
-	FindErrors                   uint64
 	FindZero                     uint64
 	InfoRequests                 uint64
-	InfoErrors                   uint64
 	ListRequests                 uint64
-	ListErrors                   uint64
 	ListQueryRequests            uint64
-	ListQueryErrors              uint64
 	DetailsRequests              uint64
-	DetailsErrors                uint64
 	CacheHit                     uint64
 	CacheMiss                    uint64
 	CacheRequestsTotal           uint64
@@ -1496,15 +1490,11 @@ func (listener *CarbonserverListener) Stat(send helper.StatCallback) {
 	numGC := uint64(m.NumGC)
 
 	sender("render_requests", &listener.metrics.RenderRequests, send)
-	sender("render_errors", &listener.metrics.RenderErrors, send)
 	sender("notfound", &listener.metrics.NotFound, send)
 	sender("find_requests", &listener.metrics.FindRequests, send)
-	sender("find_errors", &listener.metrics.FindErrors, send)
 	sender("find_zero", &listener.metrics.FindZero, send)
 	sender("list_requests", &listener.metrics.ListRequests, send)
-	sender("list_errors", &listener.metrics.ListErrors, send)
 	sender("details_requests", &listener.metrics.DetailsRequests, send)
-	sender("details_errors", &listener.metrics.DetailsErrors, send)
 	sender("cache_hit", &listener.metrics.CacheHit, send)
 	sender("cache_miss", &listener.metrics.CacheMiss, send)
 	sender("cache_work_time_ns", &listener.metrics.CacheWorkTimeNS, send)

diff --git a/carbonserver/details.go b/carbonserver/details.go
@@ -44,7 +44,6 @@ func (listener *CarbonserverListener) detailsHandler(wr http.ResponseWriter, req
 
 	formatCode, ok := knownFormats[format]
 	if !ok || formatCode == pickleFormat {
-		atomic.AddUint64(&listener.metrics.DetailsErrors, 1)
 		accessLogger.Error("details failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "unsupported format"),
@@ -58,7 +57,6 @@ func (listener *CarbonserverListener) detailsHandler(wr http.ResponseWriter, req
 
 	fidx := listener.CurrentFileIndex()
 	if fidx == nil {
-		atomic.AddUint64(&listener.metrics.DetailsErrors, 1)
 		accessLogger.Error("details failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "can't fetch metrics list"),
@@ -104,7 +102,6 @@ func (listener *CarbonserverListener) detailsHandler(wr http.ResponseWriter, req
 	}
 
 	if err != nil {
-		atomic.AddUint64(&listener.metrics.ListErrors, 1)
 		accessLogger.Error("details failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "response encode failed"),

diff --git a/carbonserver/fetchsinglemetric.go b/carbonserver/fetchsinglemetric.go
@@ -116,7 +116,6 @@ func (listener *CarbonserverListener) fetchSingleMetric(metric string, pathExpre
 	case err == nil:
 		// Should never happen, because we have a check for proper archive now
 		if m.Timeseries == nil {
-			atomic.AddUint64(&listener.metrics.RenderErrors, 1)
 			logger.Warn("metric time range not found")
 			return response{}, errors.New("time range not found")
 		}
@@ -154,7 +153,6 @@ func (listener *CarbonserverListener) fetchSingleMetric(metric string, pathExpre
 		}
 		cacheData, cerr := listener.fetchFromCache(metric, fromTime, untilTime, &resp)
 		if cerr != nil {
-			atomic.AddUint64(&listener.metrics.RenderErrors, 1)
 			logger.Warn("metric not found even in Cache", zap.Error(err))
 			// Metric has no Whisper file and/or has no datapoints in Cache
 			return response{}, cerr
@@ -166,7 +164,6 @@ func (listener *CarbonserverListener) fetchSingleMetric(metric string, pathExpre
 
 		return resp, nil
 	default:
-		atomic.AddUint64(&listener.metrics.RenderErrors, 1)
 		logger.Warn("failed to fetch points", zap.Error(err))
 		return response{}, err
 	}

diff --git a/carbonserver/find.go b/carbonserver/find.go
@@ -71,7 +71,6 @@ func (listener *CarbonserverListener) findHandler(wr http.ResponseWriter, req *h
 
 	formatCode, ok := knownFormats[format]
 	if !ok {
-		atomic.AddUint64(&listener.metrics.FindErrors, 1)
 		accessLogger.Error("find failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "unsupported format"),
@@ -115,7 +114,6 @@ func (listener *CarbonserverListener) findHandler(wr http.ResponseWriter, req *h
 	)
 
 	if len(query) == 0 {
-		atomic.AddUint64(&listener.metrics.FindErrors, 1)
 		accessLogger.Error("find failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "empty query"),
@@ -268,8 +266,6 @@ func (listener *CarbonserverListener) findMetrics(ctx context.Context, logger *z
 		}
 
 		if err != nil {
-			atomic.AddUint64(&listener.metrics.FindErrors, 1)
-
 			logger.Error("find failed",
 				zap.Duration("runtime_seconds", time.Since(t0)),
 				zap.String("reason", "response encode failed"),
@@ -295,8 +291,6 @@ func (listener *CarbonserverListener) findMetrics(ctx context.Context, logger *z
 		listener.populateMetricsFoundStat(expandedGlobs)
 
 		if err != nil {
-			atomic.AddUint64(&listener.metrics.FindErrors, 1)
-
 			logger.Error("find failed",
 				zap.Duration("runtime_seconds", time.Since(t0)),
 				zap.String("reason", "response encode failed"),
@@ -376,8 +370,6 @@ GATHER:
 	}
 
 	if len(errors) > 0 {
-		atomic.AddUint64(&listener.metrics.FindErrors, uint64(len(errors)))
-
 		if len(errors) == len(names) {
 			logger.Error("find failed",
 				zap.Duration("runtime_seconds", time.Since(t0)),

diff --git a/carbonserver/info.go b/carbonserver/info.go
@@ -50,7 +50,6 @@ func (listener *CarbonserverListener) infoHandler(wr http.ResponseWriter, req *h
 
 	formatCode, ok := knownFormats[format]
 	if !ok || formatCode == pickleFormat {
-		atomic.AddUint64(&listener.metrics.InfoErrors, 1)
 		accessLogger.Error("info failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "unsupported format"),
@@ -135,7 +134,6 @@ func (listener *CarbonserverListener) infoHandler(wr http.ResponseWriter, req *h
 	}
 
 	if len(response.Metrics) == 0 {
-		atomic.AddUint64(&listener.metrics.InfoErrors, 1)
 		accessLogger.Error("info failed",
 			zap.String("reason", "Not Found"),
 			zap.Int("http_code", http.StatusNotFound),
@@ -172,7 +170,6 @@ func (listener *CarbonserverListener) infoHandler(wr http.ResponseWriter, req *h
 	}
 
 	if err != nil {
-		atomic.AddUint64(&listener.metrics.InfoErrors, 1)
 		accessLogger.Error("info failed",
 			zap.String("reason", "response encode failed"),
 			zap.Int("http_code", http.StatusInternalServerError),

diff --git a/carbonserver/list.go b/carbonserver/list.go
@@ -72,7 +72,6 @@ func (listener *CarbonserverListener) listHandler(wr http.ResponseWriter, req *h
 
 	formatCode, ok := knownFormats[format]
 	if !ok || formatCode == pickleFormat {
-		atomic.AddUint64(&listener.metrics.ListErrors, 1)
 		accessLogger.Error("list failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "unsupported format"),
@@ -86,7 +85,6 @@ func (listener *CarbonserverListener) listHandler(wr http.ResponseWriter, req *h
 
 	metrics, err := listener.getMetricsList()
 	if err != nil {
-		atomic.AddUint64(&listener.metrics.ListErrors, 1)
 		accessLogger.Error("list failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "can't fetch metrics list"),
@@ -113,7 +111,6 @@ func (listener *CarbonserverListener) listHandler(wr http.ResponseWriter, req *h
 	}
 
 	if err != nil {
-		atomic.AddUint64(&listener.metrics.ListErrors, 1)
 		accessLogger.Error("list failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "response encode failed"),
@@ -229,7 +226,6 @@ func (listener *CarbonserverListener) listQueryHandler(wr http.ResponseWriter, r
 		var err error
 		limit, err = strconv.Atoi(req.FormValue("limit"))
 		if err != nil {
-			atomic.AddUint64(&listener.metrics.ListQueryErrors, 1)
 			accessLogger.Error("list failed",
 				zap.Duration("runtime_seconds", time.Since(t0)),
 				zap.String("reason", "faield to parse limit"),
@@ -254,7 +250,6 @@ func (listener *CarbonserverListener) listQueryHandler(wr http.ResponseWriter, r
 
 	formatCode, ok := knownFormats[format]
 	if !ok || formatCode != jsonFormat {
-		atomic.AddUint64(&listener.metrics.ListQueryErrors, 1)
 		accessLogger.Error("list_query failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "unsupported format"),
@@ -266,7 +261,6 @@ func (listener *CarbonserverListener) listQueryHandler(wr http.ResponseWriter, r
 
 	result, err := listener.queryMetricsList(target, limit, leafOnly, statsOnly)
 	if err != nil {
-		atomic.AddUint64(&listener.metrics.ListQueryErrors, 1)
 		accessLogger.Error("list_query failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "can't fetch metrics list"),
@@ -287,7 +281,6 @@ func (listener *CarbonserverListener) listQueryHandler(wr http.ResponseWriter, r
 	}
 
 	if err != nil {
-		atomic.AddUint64(&listener.metrics.ListQueryErrors, 1)
 		accessLogger.Error("list_query failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "response encode failed"),

diff --git a/carbonserver/render.go b/carbonserver/render.go
@@ -179,7 +179,6 @@ func (listener *CarbonserverListener) renderHandler(wr http.ResponseWriter, req
 
 	format, err := getFormat(req)
 	if err != nil {
-		atomic.AddUint64(&listener.metrics.RenderErrors, 1)
 		accessLogger.Error("fetch failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", err.Error()),
@@ -195,7 +194,6 @@ func (listener *CarbonserverListener) renderHandler(wr http.ResponseWriter, req
 
 	targets, err := getTargets(req, format)
 	if err != nil {
-		atomic.AddUint64(&listener.metrics.RenderErrors, 1)
 		accessLogger.Error("fetch failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", err.Error()),
@@ -250,7 +248,6 @@ func (listener *CarbonserverListener) renderHandler(wr http.ResponseWriter, req
 
 	wr.Header().Set("Content-Type", response.contentType)
 	if err != nil {
-		atomic.AddUint64(&listener.metrics.RenderErrors, 1)
 		accessLogger.Error("fetch failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "failed to read data"),
@@ -262,7 +259,6 @@ func (listener *CarbonserverListener) renderHandler(wr http.ResponseWriter, req
 	}
 
 	if response.metricsFetched == 0 && !listener.emptyResultOk {
-		atomic.AddUint64(&listener.metrics.RenderErrors, 1)
 		accessLogger.Error("fetch failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "no metrics found"),
@@ -445,7 +441,6 @@ func (listener *CarbonserverListener) prepareDataStream(ctx context.Context, for
 					}
 				}
 				if err != nil {
-					atomic.AddUint64(&listener.metrics.RenderErrors, 1)
 					listener.accessLogger.Error("error while fetching the data",
 						zap.Error(err),
 					)
@@ -775,7 +770,6 @@ func (listener *CarbonserverListener) Render(req *protov2.MultiFetchRequest, str
 	tle.MetricsFetched = metricsFetched
 	tle.ValuesFetched = valuesFetched
 	if err != nil {
-		atomic.AddUint64(&listener.metrics.RenderErrors, 1)
 		accessLogger.Error("fetch failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "stream send failed"),
@@ -789,7 +783,6 @@ func (listener *CarbonserverListener) Render(req *protov2.MultiFetchRequest, str
 	}
 
 	if metricsFetched == 0 && !listener.emptyResultOk {
-		atomic.AddUint64(&listener.metrics.RenderErrors, 1)
 		accessLogger.Error("fetch failed",
 			zap.Duration("runtime_seconds", time.Since(t0)),
 			zap.String("reason", "no metrics found"),