From 5214377583baec0e11d88f4628606ba19c80cd9b Mon Sep 17 00:00:00 2001 From: dhurley Date: Wed, 2 Aug 2023 15:03:06 +0100 Subject: [PATCH] Update nginx access & error log metric sources to only report metrics that are available instead of returning zero for metric that are missing. --- src/core/metrics/sources/nginx_access_log.go | 167 ++-- .../metrics/sources/nginx_access_log_test.go | 740 +----------------- src/core/metrics/sources/nginx_error_log.go | 21 +- .../core/metrics/sources/nginx_access_log.go | 167 ++-- .../core/metrics/sources/nginx_error_log.go | 21 +- 5 files changed, 166 insertions(+), 950 deletions(-) diff --git a/src/core/metrics/sources/nginx_access_log.go b/src/core/metrics/sources/nginx_access_log.go index cb5156378..5cf9b19c8 100644 --- a/src/core/metrics/sources/nginx_access_log.go +++ b/src/core/metrics/sources/nginx_access_log.go @@ -208,7 +208,7 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string log.Debugf("Collecting from: %s using format: %s", logFile, logFormat) log.Debugf("Pattern used for tailing logs: %s", logPattern) - httpCounters, upstreamCounters, upstreamCacheCounters := getDefaultCounters() + httpCounters, upstreamCounters, upstreamCacheCounters := map[string]float64{}, map[string]float64{}, map[string]float64{} gzipRatios, requestLengths, requestTimes, upstreamResponseLength, upstreamResponseTimes, upstreamConnectTimes, upstreamHeaderTimes := []float64{}, []float64{}, []float64{}, []float64{}, []float64{}, []float64{}, []float64{} mu := sync.Mutex{} @@ -267,7 +267,13 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string if isOtherMethod(n) { n = "method.others" } - httpCounters[n] = httpCounters[n] + 1 + + existingValue, ok := httpCounters[n] + if ok { + httpCounters[n] = existingValue + 1 + } else { + httpCounters[n] = 1 + } if access.ServerProtocol == "" { calculateServerProtocol(protocol, httpCounters) @@ -284,7 +290,12 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string for _, value := range statusValues { if v, err := strconv.Atoi(value); err == nil { n := fmt.Sprintf("upstream.status.%dxx", v/100) - upstreamCounters[n] = upstreamCounters[n] + 1 + existingValue, ok := upstreamCounters[n] + if ok { + upstreamCounters[n] = existingValue + 1 + } else { + upstreamCounters[n] = 1 + } } else { log.Debugf("Error getting upstream status value from access logs, %v", err) } @@ -307,8 +318,13 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string upstreamRequest, upstreamCounters = calculateUpstreamNextCount(upstreamTimes, upstreamCounters) } - if upstreamRequest == true { - upstreamCounters["upstream.request.count"] = upstreamCounters["upstream.request.count"] + 1 + if upstreamRequest { + existingValue, ok := upstreamCounters["upstream.request.count"] + if ok { + upstreamCounters["upstream.request.count"] = existingValue + 1 + } else { + upstreamCounters["upstream.request.count"] = 1 + } } mu.Unlock() @@ -346,6 +362,7 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string if len(upstreamResponseLength) > 0 { upstreamCounters["upstream.response.length"] = getAverageMetricValue(upstreamResponseLength) } + c.group = "http" simpleMetrics := c.convertSamplesToSimpleMetrics(httpCounters) @@ -358,7 +375,7 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string log.Tracef("Access log metrics collected: %v", simpleMetrics) // reset the counters - httpCounters, upstreamCounters, upstreamCacheCounters = getDefaultCounters() + httpCounters, upstreamCounters, upstreamCacheCounters = map[string]float64{}, map[string]float64{}, map[string]float64{} gzipRatios, requestLengths, requestTimes, upstreamResponseLength, upstreamResponseTimes, upstreamConnectTimes, upstreamHeaderTimes = []float64{}, []float64{}, []float64{}, []float64{}, []float64{}, []float64{}, []float64{} c.buf = append(c.buf, metrics.NewStatsEntityWrapper(c.baseDimensions.ToDimensions(), simpleMetrics, proto.MetricsReport_INSTANCE)) @@ -383,7 +400,12 @@ func calculateUpstreamNextCount(metricValues []string, upstreamCounters map[stri upstreamRequest = true times := strings.Split(upstreamTimes, ", ") if len(times) > 1 { - upstreamCounters["upstream.next.count"] = upstreamCounters["upstream.next.count"] + (float64(len(times)) - 1) + existingValue, ok := upstreamCounters["upstream.next.count"] + if ok { + upstreamCounters["upstream.next.count"] = existingValue + (float64(len(times)) - 1) + } else { + upstreamCounters["upstream.next.count"] = (float64(len(times)) - 1) + } return upstreamRequest, upstreamCounters } } @@ -424,7 +446,12 @@ func (c *NginxAccessLog) parseAccessLogUpstream(metricName string, metric string func (c *NginxAccessLog) parseAccessLogFloatCounters(metricName string, metric string, counters map[string]float64) map[string]float64 { if metric != "" { if v, err := strconv.ParseFloat(metric, 64); err == nil { - counters[metricName] = v + counters[metricName] + existingValue, ok := counters[metricName] + if ok { + counters[metricName] = v + existingValue + } else { + counters[metricName] = v + } return counters } else { c.logger.Log(fmt.Sprintf("Error getting %s value from access logs, %v", metricName, err)) @@ -438,7 +465,13 @@ func calculateServerProtocol(protocol string, counters map[string]float64) { httpProtocolVersion := strings.Split(protocol, "/")[1] httpProtocolVersion = strings.ReplaceAll(httpProtocolVersion, ".", "_") n := fmt.Sprintf("v%s", httpProtocolVersion) - counters[n] = counters[n] + 1 + + existingValue, ok := counters[n] + if ok { + counters[n] = existingValue + 1 + } else { + counters[n] = 1 + } } } @@ -500,17 +533,39 @@ func getAverageMetricValue(metricValues []float64) float64 { func (c *NginxAccessLog) calculateHttpStatus(status string, counter map[string]float64) { if v, err := strconv.Atoi(status); err == nil { n := fmt.Sprintf("status.%dxx", v/100) - counter[n] = counter[n] + 1 + + existingValue, ok := counter[n] + if ok { + counter[n] = existingValue + 1 + } else { + counter[n] = 1 + } + switch v { case 403, 404, 500, 502, 503, 504: n := fmt.Sprintf("status.%d", v) - counter[n] = counter[n] + 1 + existingValue, ok := counter[n] + if ok { + counter[n] = existingValue + 1 + } else { + counter[n] = 1 + } case 499: n := "status.discarded" - counter[n] = counter[n] + 1 + existingValue, ok := counter[n] + if ok { + counter[n] = existingValue + 1 + } else { + counter[n] = 1 + } case 400: n := "request.malformed" - counter[n] = counter[n] + 1 + existingValue, ok := counter[n] + if ok { + counter[n] = existingValue + 1 + } else { + counter[n] = 1 + } } } else { c.logger.Log(fmt.Sprintf("Error getting status value from access logs, %v", err)) @@ -522,7 +577,12 @@ func calculateUpstreamCacheStatus(status string, counter map[string]float64) { switch status { case "BYPASS", "EXPIRED", "HIT", "MISS", "REVALIDATED", "STALE", "UPDATING": - counter[n] = counter[n] + 1 + existingValue, ok := counter[n] + if ok { + counter[n] = existingValue + 1 + } else { + counter[n] = 1 + } return } } @@ -537,11 +597,8 @@ func calculateTimeMetricsMap(metricName string, times []float64, counter map[str } for metric := range timeMetrics { - metricType := metric[strings.LastIndex(metric, ".")+1:] - counter[metric] = metrics.GetTimeMetrics(times, metricType) - } } @@ -554,82 +611,6 @@ func isOtherMethod(method string) bool { method != "method.options" } -func getDefaultCounters() (map[string]float64, map[string]float64, map[string]float64) { - httpCounters := map[string]float64{ - "gzip.ratio": 0, - "method.delete": 0, - "method.get": 0, - "method.head": 0, - "method.options": 0, - "method.post": 0, - "method.put": 0, - "method.others": 0, - "request.body_bytes_sent": 0, - "request.bytes_sent": 0, - "request.length": 0, - "request.malformed": 0, - "request.time": 0, - "request.time.count": 0, - "request.time.max": 0, - "request.time.median": 0, - "request.time.pctl95": 0, - "status.403": 0, - "status.404": 0, - "status.500": 0, - "status.502": 0, - "status.503": 0, - "status.504": 0, - "status.discarded": 0, - "status.1xx": 0, - "status.2xx": 0, - "status.3xx": 0, - "status.4xx": 0, - "status.5xx": 0, - "v0_9": 0, - "v1_0": 0, - "v1_1": 0, - "v2": 0, - } - - upstreamCounters := map[string]float64{ - "upstream.connect.time": 0, - "upstream.connect.time.count": 0, - "upstream.connect.time.max": 0, - "upstream.connect.time.median": 0, - "upstream.connect.time.pctl95": 0, - "upstream.header.time": 0, - "upstream.header.time.count": 0, - "upstream.header.time.max": 0, - "upstream.header.time.median": 0, - "upstream.header.time.pctl95": 0, - "upstream.request.count": 0, - "upstream.next.count": 0, - "upstream.response.time": 0, - "upstream.response.time.count": 0, - "upstream.response.time.max": 0, - "upstream.response.time.median": 0, - "upstream.response.time.pctl95": 0, - "upstream.response.length": 0, - "upstream.status.1xx": 0, - "upstream.status.2xx": 0, - "upstream.status.3xx": 0, - "upstream.status.4xx": 0, - "upstream.status.5xx": 0, - } - - upstreamCacheCounters := map[string]float64{ - "cache.bypass": 0, - "cache.expired": 0, - "cache.hit": 0, - "cache.miss": 0, - "cache.revalidated": 0, - "cache.stale": 0, - "cache.updating": 0, - } - - return httpCounters, upstreamCounters, upstreamCacheCounters -} - // For all the variables in the log format that are not present in the logVarMap // replace them with the %{DATA:.*} format func replaceCustomLogVars(logPattern string) string { diff --git a/src/core/metrics/sources/nginx_access_log_test.go b/src/core/metrics/sources/nginx_access_log_test.go index fd9bba953..7ddc72a89 100644 --- a/src/core/metrics/sources/nginx_access_log_test.go +++ b/src/core/metrics/sources/nginx_access_log_test.go @@ -753,258 +753,22 @@ func TestAccessLogStats(t *testing.T) { }, &proto.StatsEntity{ Simplemetrics: []*proto.SimpleMetric{ - { - Name: "nginx.http.gzip.ratio", - Value: 0, - }, - { - Name: "nginx.http.request.time", - Value: 0, - }, - { - Name: "nginx.http.request.time.count", - Value: 0, - }, - { - Name: "nginx.http.request.time.median", - Value: 0, - }, - { - Name: "nginx.http.request.time.max", - Value: 0, - }, - { - Name: "nginx.http.request.time.pctl95", - Value: 0, - }, { Name: "nginx.http.request.body_bytes_sent", Value: 196, }, - { - Name: "nginx.http.request.bytes_sent", - Value: 0, - }, - { - Name: "nginx.http.request.length", - Value: 0, - }, - { - Name: "nginx.http.request.malformed", - Value: 0, - }, - { - Name: "nginx.http.method.post", - Value: 0, - }, { Name: "nginx.http.method.get", Value: 2, }, - { - Name: "nginx.http.method.delete", - Value: 0, - }, - { - Name: "nginx.http.method.put", - Value: 0, - }, - { - Name: "nginx.http.method.head", - Value: 0, - }, - { - Name: "nginx.http.method.options", - Value: 0, - }, - { - Name: "nginx.http.method.others", - Value: 0, - }, - { - Name: "nginx.http.status.1xx", - Value: 0, - }, { Name: "nginx.http.status.2xx", Value: 2, }, - { - Name: "nginx.http.status.3xx", - Value: 0, - }, - { - Name: "nginx.http.status.4xx", - Value: 0, - }, - { - Name: "nginx.http.status.5xx", - Value: 0, - }, - { - Name: "nginx.http.status.403", - Value: 0, - }, - { - Name: "nginx.http.status.404", - Value: 0, - }, - { - Name: "nginx.http.status.500", - Value: 0, - }, - { - Name: "nginx.http.status.502", - Value: 0, - }, - { - Name: "nginx.http.status.503", - Value: 0, - }, - { - Name: "nginx.http.status.504", - Value: 0, - }, - { - Name: "nginx.http.status.discarded", - Value: 0, - }, - { - Name: "nginx.http.v0_9", - Value: 0, - }, - { - Name: "nginx.http.v1_0", - Value: 0, - }, { Name: "nginx.http.v1_1", Value: 2, }, - { - Name: "nginx.http.v2", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.count", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.max", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.median", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.pctl95", - Value: 0, - }, - { - Name: "nginx.upstream.header.time", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.count", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.max", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.median", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.pctl95", - Value: 0, - }, - { - Name: "nginx.upstream.request.count", - Value: 0, - }, - { - Name: "nginx.upstream.next.count", - Value: 0, - }, - { - Name: "nginx.upstream.response.length", - Value: 0, - }, - { - Name: "nginx.upstream.response.time", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.count", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.max", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.median", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.pctl95", - Value: 0, - }, - { - Name: "nginx.upstream.status.1xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.2xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.3xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.4xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.5xx", - Value: 0, - }, - { - Name: "nginx.cache.bypass", - Value: 0, - }, - { - Name: "nginx.cache.expired", - Value: 0, - }, - { - Name: "nginx.cache.hit", - Value: 0, - }, - { - Name: "nginx.cache.miss", - Value: 0, - }, - { - Name: "nginx.cache.revalidated", - Value: 0, - }, - { - Name: "nginx.cache.stale", - Value: 0, - }, - { - Name: "nginx.cache.updating", - Value: 0, - }, }, }, }, @@ -1017,257 +781,25 @@ func TestAccessLogStats(t *testing.T) { }, &proto.StatsEntity{ Simplemetrics: []*proto.SimpleMetric{ - { - Name: "nginx.http.gzip.ratio", - Value: 0, - }, - { - Name: "nginx.http.request.time", - Value: 0, - }, - { - Name: "nginx.http.request.time.count", - Value: 0, - }, - { - Name: "nginx.http.request.time.median", - Value: 0, - }, - { - Name: "nginx.http.request.time.max", - Value: 0, - }, - { - Name: "nginx.http.request.time.pctl95", - Value: 0, - }, { Name: "nginx.http.request.body_bytes_sent", Value: 196, }, - { - Name: "nginx.http.request.bytes_sent", - Value: 0, - }, - { - Name: "nginx.http.request.length", - Value: 0, - }, - { - Name: "nginx.http.request.malformed", - Value: 0, - }, - { - Name: "nginx.http.method.post", - Value: 0, - }, { Name: "nginx.http.method.get", Value: 1, }, - { - Name: "nginx.http.method.delete", - Value: 0, - }, - { - Name: "nginx.http.method.put", - Value: 0, - }, - { - Name: "nginx.http.method.head", - Value: 0, - }, - { - Name: "nginx.http.method.options", - Value: 0, - }, { Name: "nginx.http.method.others", Value: 1, }, - { - Name: "nginx.http.status.1xx", - Value: 0, - }, { Name: "nginx.http.status.2xx", Value: 2, }, { - Name: "nginx.http.status.3xx", - Value: 0, - }, - { - Name: "nginx.http.status.4xx", - Value: 0, - }, - { - Name: "nginx.http.status.5xx", - Value: 0, - }, - { - Name: "nginx.http.status.403", - Value: 0, - }, - { - Name: "nginx.http.status.404", - Value: 0, - }, - { - Name: "nginx.http.status.500", - Value: 0, - }, - { - Name: "nginx.http.status.502", - Value: 0, - }, - { - Name: "nginx.http.status.503", - Value: 0, - }, - { - Name: "nginx.http.status.504", - Value: 0, - }, - { - Name: "nginx.http.status.discarded", - Value: 0, - }, - { - Name: "nginx.http.v0_9", - Value: 0, - }, - { - Name: "nginx.http.v1_0", - Value: 0, - }, - { - Name: "nginx.http.v1_1", - Value: 1, - }, - { - Name: "nginx.http.v2", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.count", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.max", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.median", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.pctl95", - Value: 0, - }, - { - Name: "nginx.upstream.header.time", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.count", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.max", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.median", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.pctl95", - Value: 0, - }, - { - Name: "nginx.upstream.next.count", - Value: 0, - }, - { - Name: "nginx.upstream.request.count", - Value: 0, - }, - { - Name: "nginx.upstream.response.time", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.count", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.max", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.median", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.pctl95", - Value: 0, - }, - { - Name: "nginx.upstream.response.length", - Value: 0, - }, - { - Name: "nginx.upstream.status.1xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.2xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.3xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.4xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.5xx", - Value: 0, - }, - { - Name: "nginx.cache.bypass", - Value: 0, - }, - { - Name: "nginx.cache.expired", - Value: 0, - }, - { - Name: "nginx.cache.hit", - Value: 0, - }, - { - Name: "nginx.cache.miss", - Value: 0, - }, - { - Name: "nginx.cache.revalidated", - Value: 0, - }, - { - Name: "nginx.cache.stale", - Value: 0, - }, - { - Name: "nginx.cache.updating", - Value: 0, + Name: "nginx.http.v1_1", + Value: 1, }, }, }, @@ -1361,18 +893,10 @@ func TestAccessLogStats(t *testing.T) { Name: "nginx.http.method.others", Value: 2, }, - { - Name: "nginx.http.status.1xx", - Value: 0, - }, { Name: "nginx.http.status.2xx", Value: 5, }, - { - Name: "nginx.http.status.3xx", - Value: 0, - }, { Name: "nginx.http.status.4xx", Value: 4, @@ -1497,18 +1021,10 @@ func TestAccessLogStats(t *testing.T) { Name: "nginx.upstream.response.length", Value: 23.444444444444443, }, - { - Name: "nginx.upstream.status.1xx", - Value: 0, - }, { Name: "nginx.upstream.status.2xx", Value: 6, }, - { - Name: "nginx.upstream.status.3xx", - Value: 0, - }, { Name: "nginx.upstream.status.4xx", Value: 3, @@ -1517,14 +1033,6 @@ func TestAccessLogStats(t *testing.T) { Name: "nginx.upstream.status.5xx", Value: 4, }, - { - Name: "nginx.cache.bypass", - Value: 0, - }, - { - Name: "nginx.cache.expired", - Value: 0, - }, { Name: "nginx.cache.hit", Value: 11, @@ -1533,14 +1041,6 @@ func TestAccessLogStats(t *testing.T) { Name: "nginx.cache.miss", Value: 2, }, - { - Name: "nginx.cache.revalidated", - Value: 0, - }, - { - Name: "nginx.cache.stale", - Value: 0, - }, { Name: "nginx.cache.updating", Value: 1, @@ -1557,258 +1057,22 @@ func TestAccessLogStats(t *testing.T) { }, &proto.StatsEntity{ Simplemetrics: []*proto.SimpleMetric{ - { - Name: "nginx.http.gzip.ratio", - Value: 0, - }, - { - Name: "nginx.http.request.time", - Value: 0, - }, - { - Name: "nginx.http.request.time.count", - Value: 0, - }, - { - Name: "nginx.http.request.time.median", - Value: 0, - }, - { - Name: "nginx.http.request.time.max", - Value: 0, - }, - { - Name: "nginx.http.request.time.pctl95", - Value: 0, - }, { Name: "nginx.http.request.body_bytes_sent", Value: 196, }, - { - Name: "nginx.http.request.bytes_sent", - Value: 0, - }, - { - Name: "nginx.http.request.length", - Value: 0, - }, - { - Name: "nginx.http.request.malformed", - Value: 0, - }, - { - Name: "nginx.http.method.post", - Value: 0, - }, { Name: "nginx.http.method.get", Value: 2, }, - { - Name: "nginx.http.method.delete", - Value: 0, - }, - { - Name: "nginx.http.method.put", - Value: 0, - }, - { - Name: "nginx.http.method.head", - Value: 0, - }, - { - Name: "nginx.http.method.options", - Value: 0, - }, - { - Name: "nginx.http.method.others", - Value: 0, - }, - { - Name: "nginx.http.status.1xx", - Value: 0, - }, { Name: "nginx.http.status.2xx", Value: 2, }, - { - Name: "nginx.http.status.3xx", - Value: 0, - }, - { - Name: "nginx.http.status.4xx", - Value: 0, - }, - { - Name: "nginx.http.status.5xx", - Value: 0, - }, - { - Name: "nginx.http.status.403", - Value: 0, - }, - { - Name: "nginx.http.status.404", - Value: 0, - }, - { - Name: "nginx.http.status.500", - Value: 0, - }, - { - Name: "nginx.http.status.502", - Value: 0, - }, - { - Name: "nginx.http.status.503", - Value: 0, - }, - { - Name: "nginx.http.status.504", - Value: 0, - }, - { - Name: "nginx.http.status.discarded", - Value: 0, - }, - { - Name: "nginx.http.v0_9", - Value: 0, - }, - { - Name: "nginx.http.v1_0", - Value: 0, - }, { Name: "nginx.http.v1_1", Value: 2, }, - { - Name: "nginx.http.v2", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.count", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.max", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.median", - Value: 0, - }, - { - Name: "nginx.upstream.connect.time.pctl95", - Value: 0, - }, - { - Name: "nginx.upstream.header.time", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.count", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.max", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.median", - Value: 0, - }, - { - Name: "nginx.upstream.header.time.pctl95", - Value: 0, - }, - { - Name: "nginx.upstream.request.count", - Value: 0, - }, - { - Name: "nginx.upstream.next.count", - Value: 0, - }, - { - Name: "nginx.upstream.response.length", - Value: 0, - }, - { - Name: "nginx.upstream.response.time", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.count", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.max", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.median", - Value: 0, - }, - { - Name: "nginx.upstream.response.time.pctl95", - Value: 0, - }, - { - Name: "nginx.upstream.status.1xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.2xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.3xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.4xx", - Value: 0, - }, - { - Name: "nginx.upstream.status.5xx", - Value: 0, - }, - { - Name: "nginx.cache.bypass", - Value: 0, - }, - { - Name: "nginx.cache.expired", - Value: 0, - }, - { - Name: "nginx.cache.hit", - Value: 0, - }, - { - Name: "nginx.cache.miss", - Value: 0, - }, - { - Name: "nginx.cache.revalidated", - Value: 0, - }, - { - Name: "nginx.cache.stale", - Value: 0, - }, - { - Name: "nginx.cache.updating", - Value: 0, - }, }, }, }, diff --git a/src/core/metrics/sources/nginx_error_log.go b/src/core/metrics/sources/nginx_error_log.go index 8b823f5c3..d985d78b5 100644 --- a/src/core/metrics/sources/nginx_error_log.go +++ b/src/core/metrics/sources/nginx_error_log.go @@ -193,12 +193,7 @@ func (c *NginxErrorLog) collectLogStats(ctx context.Context, m chan<- *metrics.S func (c *NginxErrorLog) logStats(ctx context.Context, logFile string) { log.Debugf("Collecting from error log: %s", logFile) - counters := map[string]float64{ - HttpRequestBufferedMetricName: 0, - UpstreamResponseBufferedMetricName: 0, - UpstreamRequestFailedMetricName: 0, - UpstreamResponseFailedMetricName: 0, - } + counters := map[string]float64{} mu := sync.Mutex{} t, err := tailer.NewTailer(logFile) @@ -220,7 +215,12 @@ func (c *NginxErrorLog) logStats(ctx context.Context, logFile string) { for metricName, regularExpressionList := range regularExpressionErrorMap { for _, re := range regularExpressionList { if re.MatchString(d) { - counters[metricName] = counters[metricName] + 1 + existingValue, ok := counters[metricName] + if ok { + counters[metricName] = existingValue + 1 + } else { + counters[metricName] = 1 + } break } } @@ -237,12 +237,7 @@ func (c *NginxErrorLog) logStats(ctx context.Context, logFile string) { log.Tracef("Error log metrics collected: %v", simpleMetrics) // reset the counters - counters = map[string]float64{ - HttpRequestBufferedMetricName: 0, - UpstreamResponseBufferedMetricName: 0, - UpstreamRequestFailedMetricName: 0, - UpstreamResponseFailedMetricName: 0, - } + counters = map[string]float64{} c.buf = append(c.buf, metrics.NewStatsEntityWrapper(c.baseDimensions.ToDimensions(), simpleMetrics, proto.MetricsReport_INSTANCE)) diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_access_log.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_access_log.go index cb5156378..5cf9b19c8 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_access_log.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_access_log.go @@ -208,7 +208,7 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string log.Debugf("Collecting from: %s using format: %s", logFile, logFormat) log.Debugf("Pattern used for tailing logs: %s", logPattern) - httpCounters, upstreamCounters, upstreamCacheCounters := getDefaultCounters() + httpCounters, upstreamCounters, upstreamCacheCounters := map[string]float64{}, map[string]float64{}, map[string]float64{} gzipRatios, requestLengths, requestTimes, upstreamResponseLength, upstreamResponseTimes, upstreamConnectTimes, upstreamHeaderTimes := []float64{}, []float64{}, []float64{}, []float64{}, []float64{}, []float64{}, []float64{} mu := sync.Mutex{} @@ -267,7 +267,13 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string if isOtherMethod(n) { n = "method.others" } - httpCounters[n] = httpCounters[n] + 1 + + existingValue, ok := httpCounters[n] + if ok { + httpCounters[n] = existingValue + 1 + } else { + httpCounters[n] = 1 + } if access.ServerProtocol == "" { calculateServerProtocol(protocol, httpCounters) @@ -284,7 +290,12 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string for _, value := range statusValues { if v, err := strconv.Atoi(value); err == nil { n := fmt.Sprintf("upstream.status.%dxx", v/100) - upstreamCounters[n] = upstreamCounters[n] + 1 + existingValue, ok := upstreamCounters[n] + if ok { + upstreamCounters[n] = existingValue + 1 + } else { + upstreamCounters[n] = 1 + } } else { log.Debugf("Error getting upstream status value from access logs, %v", err) } @@ -307,8 +318,13 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string upstreamRequest, upstreamCounters = calculateUpstreamNextCount(upstreamTimes, upstreamCounters) } - if upstreamRequest == true { - upstreamCounters["upstream.request.count"] = upstreamCounters["upstream.request.count"] + 1 + if upstreamRequest { + existingValue, ok := upstreamCounters["upstream.request.count"] + if ok { + upstreamCounters["upstream.request.count"] = existingValue + 1 + } else { + upstreamCounters["upstream.request.count"] = 1 + } } mu.Unlock() @@ -346,6 +362,7 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string if len(upstreamResponseLength) > 0 { upstreamCounters["upstream.response.length"] = getAverageMetricValue(upstreamResponseLength) } + c.group = "http" simpleMetrics := c.convertSamplesToSimpleMetrics(httpCounters) @@ -358,7 +375,7 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string log.Tracef("Access log metrics collected: %v", simpleMetrics) // reset the counters - httpCounters, upstreamCounters, upstreamCacheCounters = getDefaultCounters() + httpCounters, upstreamCounters, upstreamCacheCounters = map[string]float64{}, map[string]float64{}, map[string]float64{} gzipRatios, requestLengths, requestTimes, upstreamResponseLength, upstreamResponseTimes, upstreamConnectTimes, upstreamHeaderTimes = []float64{}, []float64{}, []float64{}, []float64{}, []float64{}, []float64{}, []float64{} c.buf = append(c.buf, metrics.NewStatsEntityWrapper(c.baseDimensions.ToDimensions(), simpleMetrics, proto.MetricsReport_INSTANCE)) @@ -383,7 +400,12 @@ func calculateUpstreamNextCount(metricValues []string, upstreamCounters map[stri upstreamRequest = true times := strings.Split(upstreamTimes, ", ") if len(times) > 1 { - upstreamCounters["upstream.next.count"] = upstreamCounters["upstream.next.count"] + (float64(len(times)) - 1) + existingValue, ok := upstreamCounters["upstream.next.count"] + if ok { + upstreamCounters["upstream.next.count"] = existingValue + (float64(len(times)) - 1) + } else { + upstreamCounters["upstream.next.count"] = (float64(len(times)) - 1) + } return upstreamRequest, upstreamCounters } } @@ -424,7 +446,12 @@ func (c *NginxAccessLog) parseAccessLogUpstream(metricName string, metric string func (c *NginxAccessLog) parseAccessLogFloatCounters(metricName string, metric string, counters map[string]float64) map[string]float64 { if metric != "" { if v, err := strconv.ParseFloat(metric, 64); err == nil { - counters[metricName] = v + counters[metricName] + existingValue, ok := counters[metricName] + if ok { + counters[metricName] = v + existingValue + } else { + counters[metricName] = v + } return counters } else { c.logger.Log(fmt.Sprintf("Error getting %s value from access logs, %v", metricName, err)) @@ -438,7 +465,13 @@ func calculateServerProtocol(protocol string, counters map[string]float64) { httpProtocolVersion := strings.Split(protocol, "/")[1] httpProtocolVersion = strings.ReplaceAll(httpProtocolVersion, ".", "_") n := fmt.Sprintf("v%s", httpProtocolVersion) - counters[n] = counters[n] + 1 + + existingValue, ok := counters[n] + if ok { + counters[n] = existingValue + 1 + } else { + counters[n] = 1 + } } } @@ -500,17 +533,39 @@ func getAverageMetricValue(metricValues []float64) float64 { func (c *NginxAccessLog) calculateHttpStatus(status string, counter map[string]float64) { if v, err := strconv.Atoi(status); err == nil { n := fmt.Sprintf("status.%dxx", v/100) - counter[n] = counter[n] + 1 + + existingValue, ok := counter[n] + if ok { + counter[n] = existingValue + 1 + } else { + counter[n] = 1 + } + switch v { case 403, 404, 500, 502, 503, 504: n := fmt.Sprintf("status.%d", v) - counter[n] = counter[n] + 1 + existingValue, ok := counter[n] + if ok { + counter[n] = existingValue + 1 + } else { + counter[n] = 1 + } case 499: n := "status.discarded" - counter[n] = counter[n] + 1 + existingValue, ok := counter[n] + if ok { + counter[n] = existingValue + 1 + } else { + counter[n] = 1 + } case 400: n := "request.malformed" - counter[n] = counter[n] + 1 + existingValue, ok := counter[n] + if ok { + counter[n] = existingValue + 1 + } else { + counter[n] = 1 + } } } else { c.logger.Log(fmt.Sprintf("Error getting status value from access logs, %v", err)) @@ -522,7 +577,12 @@ func calculateUpstreamCacheStatus(status string, counter map[string]float64) { switch status { case "BYPASS", "EXPIRED", "HIT", "MISS", "REVALIDATED", "STALE", "UPDATING": - counter[n] = counter[n] + 1 + existingValue, ok := counter[n] + if ok { + counter[n] = existingValue + 1 + } else { + counter[n] = 1 + } return } } @@ -537,11 +597,8 @@ func calculateTimeMetricsMap(metricName string, times []float64, counter map[str } for metric := range timeMetrics { - metricType := metric[strings.LastIndex(metric, ".")+1:] - counter[metric] = metrics.GetTimeMetrics(times, metricType) - } } @@ -554,82 +611,6 @@ func isOtherMethod(method string) bool { method != "method.options" } -func getDefaultCounters() (map[string]float64, map[string]float64, map[string]float64) { - httpCounters := map[string]float64{ - "gzip.ratio": 0, - "method.delete": 0, - "method.get": 0, - "method.head": 0, - "method.options": 0, - "method.post": 0, - "method.put": 0, - "method.others": 0, - "request.body_bytes_sent": 0, - "request.bytes_sent": 0, - "request.length": 0, - "request.malformed": 0, - "request.time": 0, - "request.time.count": 0, - "request.time.max": 0, - "request.time.median": 0, - "request.time.pctl95": 0, - "status.403": 0, - "status.404": 0, - "status.500": 0, - "status.502": 0, - "status.503": 0, - "status.504": 0, - "status.discarded": 0, - "status.1xx": 0, - "status.2xx": 0, - "status.3xx": 0, - "status.4xx": 0, - "status.5xx": 0, - "v0_9": 0, - "v1_0": 0, - "v1_1": 0, - "v2": 0, - } - - upstreamCounters := map[string]float64{ - "upstream.connect.time": 0, - "upstream.connect.time.count": 0, - "upstream.connect.time.max": 0, - "upstream.connect.time.median": 0, - "upstream.connect.time.pctl95": 0, - "upstream.header.time": 0, - "upstream.header.time.count": 0, - "upstream.header.time.max": 0, - "upstream.header.time.median": 0, - "upstream.header.time.pctl95": 0, - "upstream.request.count": 0, - "upstream.next.count": 0, - "upstream.response.time": 0, - "upstream.response.time.count": 0, - "upstream.response.time.max": 0, - "upstream.response.time.median": 0, - "upstream.response.time.pctl95": 0, - "upstream.response.length": 0, - "upstream.status.1xx": 0, - "upstream.status.2xx": 0, - "upstream.status.3xx": 0, - "upstream.status.4xx": 0, - "upstream.status.5xx": 0, - } - - upstreamCacheCounters := map[string]float64{ - "cache.bypass": 0, - "cache.expired": 0, - "cache.hit": 0, - "cache.miss": 0, - "cache.revalidated": 0, - "cache.stale": 0, - "cache.updating": 0, - } - - return httpCounters, upstreamCounters, upstreamCacheCounters -} - // For all the variables in the log format that are not present in the logVarMap // replace them with the %{DATA:.*} format func replaceCustomLogVars(logPattern string) string { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_error_log.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_error_log.go index 8b823f5c3..d985d78b5 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_error_log.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_error_log.go @@ -193,12 +193,7 @@ func (c *NginxErrorLog) collectLogStats(ctx context.Context, m chan<- *metrics.S func (c *NginxErrorLog) logStats(ctx context.Context, logFile string) { log.Debugf("Collecting from error log: %s", logFile) - counters := map[string]float64{ - HttpRequestBufferedMetricName: 0, - UpstreamResponseBufferedMetricName: 0, - UpstreamRequestFailedMetricName: 0, - UpstreamResponseFailedMetricName: 0, - } + counters := map[string]float64{} mu := sync.Mutex{} t, err := tailer.NewTailer(logFile) @@ -220,7 +215,12 @@ func (c *NginxErrorLog) logStats(ctx context.Context, logFile string) { for metricName, regularExpressionList := range regularExpressionErrorMap { for _, re := range regularExpressionList { if re.MatchString(d) { - counters[metricName] = counters[metricName] + 1 + existingValue, ok := counters[metricName] + if ok { + counters[metricName] = existingValue + 1 + } else { + counters[metricName] = 1 + } break } } @@ -237,12 +237,7 @@ func (c *NginxErrorLog) logStats(ctx context.Context, logFile string) { log.Tracef("Error log metrics collected: %v", simpleMetrics) // reset the counters - counters = map[string]float64{ - HttpRequestBufferedMetricName: 0, - UpstreamResponseBufferedMetricName: 0, - UpstreamRequestFailedMetricName: 0, - UpstreamResponseFailedMetricName: 0, - } + counters = map[string]float64{} c.buf = append(c.buf, metrics.NewStatsEntityWrapper(c.baseDimensions.ToDimensions(), simpleMetrics, proto.MetricsReport_INSTANCE))