Remove namedMetric pointer and improve agent log messages for access & error log parsing

Author: dhurley

src/core/metrics/sources/common.go

@@ -34,10 +34,10 @@ type namedMetric struct {
 }
 
 func (n *namedMetric) label(name string) string {
-	log.Tracef("Creating label for namespace %s, group %s, name %s", n.namespace, n.group, name)
 	if name == "" {
 		return ""
 	}
+
 	switch {
 	case n.namespace != "" && n.group != "":
 		return strings.Join([]string{n.namespace, n.group, name}, ".")
@@ -46,6 +46,7 @@ func (n *namedMetric) label(name string) string {
 	case n.group != "":
 		return strings.Join([]string{n.group, name}, ".")
 	}
+
 	return name
 }
 

src/core/metrics/sources/nginx_access_log.go

@@ -62,13 +62,13 @@ var logVarRegex = regexp.MustCompile(`\$([a-zA-Z]+[_[a-zA-Z]+]*)`)
 // This metrics source is used to tail the NGINX access logs to retrieve metrics.
 
 type NginxAccessLog struct {
-	baseDimensions *metrics.CommonDim
-	*namedMetric
+	baseDimensions     *metrics.CommonDim
 	mu                 *sync.Mutex
 	logFormats         map[string]string
 	logs               map[string]context.CancelFunc
 	binary             core.NginxBinary
 	nginxType          string
+	namespace          string
 	collectionInterval time.Duration
 	buf                []*metrics.StatsEntityWrapper
 	logger             *MetricSourceLogger
@@ -85,12 +85,12 @@ func NewNginxAccessLog(
 
 	nginxAccessLog := &NginxAccessLog{
 		baseDimensions,
-		&namedMetric{namespace: namespace},
 		&sync.Mutex{},
 		make(map[string]string),
 		make(map[string]context.CancelFunc),
 		binary,
 		nginxType,
+		namespace,
 		collectionInterval,
 		[]*metrics.StatsEntityWrapper{},
 		NewMetricSourceLogger(),
@@ -203,8 +203,8 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string
 
 	logPattern = replaceCustomLogVars(logPattern)
 
-	log.Debugf("Collecting from: %s using format: %s", logFile, logFormat)
-	log.Debugf("Pattern used for tailing logs: %s", logPattern)
+	log.Debugf("Collecting from %s using format %s", logFile, logFormat)
+	log.Debugf("Pattern used for tailing logs, %s", logPattern)
 
 	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{}
@@ -214,19 +214,21 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string
 	if logPattern == "ltsv" {
 		t, err := tailer.NewLTSVTailer(logFile)
 		if err != nil {
-			log.Errorf("unable to tail %q: %v", logFile, err)
+			log.Errorf("Unable to tail %q: %v", logFile, err)
 			return
 		}
 		go t.Tail(ctx, data)
 	} else {
 		t, err := tailer.NewPatternTailer(logFile, map[string]string{"DEFAULT": logPattern})
 		if err != nil {
-			log.Errorf("unable to tail %q: %v", logFile, err)
+			log.Errorf("Unable to tail %q: %v", logFile, err)
 			return
 		}
 		go t.Tail(ctx, data)
 	}
 
+	accessLogNamedMetric := namedMetric{namespace: c.namespace}
+
 	tick := time.NewTicker(c.collectionInterval)
 	defer tick.Stop()
 	for {
@@ -295,7 +297,7 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string
 							upstreamCounters[n] = 1
 						}
 					} else {
-						log.Debugf("Error getting upstream status value from access logs, %v", err)
+						log.Debugf("Error getting upstream status value from access log %s, %v", logFile, err)
 					}
 				}
 
@@ -330,6 +332,8 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string
 		case <-tick.C:
 			mu.Lock()
 
+			log.Tracef("Collecting metrics from access log: %s", logFile)
+
 			c.baseDimensions.NginxType = c.nginxType
 			c.baseDimensions.PublishedAPI = logFile
 
@@ -361,24 +365,23 @@ func (c *NginxAccessLog) logStats(ctx context.Context, logFile, logFormat string
 				upstreamCounters["upstream.response.length"] = getAverageMetricValue(upstreamResponseLength)
 			}
 
-			log.Tracef("%s log file: Converting httpCounters: %v", logFile, httpCounters)
-
-			c.group = "http"
-			simpleMetrics := c.convertSamplesToSimpleMetrics(httpCounters)
-
-			c.group = ""
-			simpleMetrics = append(simpleMetrics, c.convertSamplesToSimpleMetrics(upstreamCounters)...)
+			accessLogNamedMetric.group = "http"
+			log.Tracef("Converting httpCounters for %s access log file, httpCounters=%v", logFile, httpCounters)
+			simpleMetrics := accessLogNamedMetric.convertSamplesToSimpleMetrics(httpCounters)
 
-			c.group = ""
-			simpleMetrics = append(simpleMetrics, c.convertSamplesToSimpleMetrics(upstreamCacheCounters)...)
+			accessLogNamedMetric.group = ""
+			log.Tracef("Converting upstreamCounters for %s access log file, upstreamCounters=%v", logFile, upstreamCounters)
+			simpleMetrics = append(simpleMetrics, accessLogNamedMetric.convertSamplesToSimpleMetrics(upstreamCounters)...)
+			log.Tracef("Converting upstreamCacheCounters for %s access log file, upstreamCacheCounters=%v", logFile, upstreamCacheCounters)
+			simpleMetrics = append(simpleMetrics, accessLogNamedMetric.convertSamplesToSimpleMetrics(upstreamCacheCounters)...)
 
-			log.Tracef("Access log metrics collected: %v", simpleMetrics)
+			log.Tracef("Access log %s metrics collected: %v", logFile, simpleMetrics)
 
 			// reset the counters
 			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{}
 
-			log.Debugf("access log stats count: %d", len(simpleMetrics))
+			log.Debugf("Access log %s stats count: %d", logFile, len(simpleMetrics))
 
 			c.buf = append(c.buf, metrics.NewStatsEntityWrapper(c.baseDimensions.ToDimensions(), simpleMetrics, proto.MetricsReport_INSTANCE))
 

src/core/metrics/sources/nginx_error_log.go

@@ -57,13 +57,13 @@ var regularExpressionErrorMap = map[string][]*re.Regexp{
 }
 
 type NginxErrorLog struct {
-	baseDimensions *metrics.CommonDim
-	*namedMetric
+	baseDimensions     *metrics.CommonDim
 	mu                 *sync.Mutex
 	logFormats         map[string]string
 	logs               map[string]context.CancelFunc
 	binary             core.NginxBinary
 	nginxType          string
+	namespace          string
 	collectionInterval time.Duration
 	buf                []*metrics.StatsEntityWrapper
 }
@@ -78,12 +78,12 @@ func NewNginxErrorLog(
 	log.Trace("Creating NewNginxErrorLog")
 	nginxErrorLog := &NginxErrorLog{
 		baseDimensions,
-		&namedMetric{namespace: namespace},
 		&sync.Mutex{},
 		make(map[string]string),
 		make(map[string]context.CancelFunc),
 		binary,
 		nginxType,
+		namespace,
 		collectionInterval,
 		[]*metrics.StatsEntityWrapper{},
 	}
@@ -111,7 +111,7 @@ func (c *NginxErrorLog) Stop() {
 		fn()
 		delete(c.logs, f)
 	}
-	log.Debugf("Stopping NginxErrorLog source for nginx id: %v", c.baseDimensions.NginxId)
+	log.Debugf("Stopping NginxErrorLog source for NGINX ID: %v", c.baseDimensions.NginxId)
 }
 
 func (c *NginxErrorLog) Update(dimensions *metrics.CommonDim, collectorConf *metrics.NginxCollectorConfig) {
@@ -189,14 +189,14 @@ func (c *NginxErrorLog) collectLogStats(_ context.Context, m chan<- *metrics.Sta
 }
 
 func (c *NginxErrorLog) logStats(ctx context.Context, logFile string) {
-	log.Debugf("Collecting from error log: %s", logFile)
+	log.Debugf("Collecting from error log %s", logFile)
 
 	counters := map[string]float64{}
 	mu := sync.Mutex{}
 
 	t, err := tailer.NewTailer(logFile)
 	if err != nil {
-		log.Errorf("Unable to tail %q: %v", logFile, err)
+		log.Errorf("Unable to tail %s, %v", logFile, err)
 		return
 	}
 	data := make(chan string, 1024)
@@ -205,6 +205,8 @@ func (c *NginxErrorLog) logStats(ctx context.Context, logFile string) {
 	tick := time.NewTicker(c.collectionInterval)
 	defer tick.Stop()
 
+	errorLogNamedMetric := namedMetric{namespace: c.namespace}
+
 	for {
 		select {
 		case d := <-data:
@@ -228,15 +230,21 @@ func (c *NginxErrorLog) logStats(ctx context.Context, logFile string) {
 
 		case <-tick.C:
 			mu.Lock()
+
+			log.Tracef("Collecting metrics from error log: %s", logFile)
+
 			c.baseDimensions.NginxType = c.nginxType
 			c.baseDimensions.PublishedAPI = logFile
 
-			simpleMetrics := c.convertSamplesToSimpleMetrics(counters)
-			log.Tracef("Error log metrics collected: %v", simpleMetrics)
+			log.Tracef("Converting counters for %s error log file, counters=%v", logFile, counters)
+			simpleMetrics := errorLogNamedMetric.convertSamplesToSimpleMetrics(counters)
+			log.Tracef("Error log %s metrics collected: %v", logFile, simpleMetrics)
 
 			// reset the counters
 			counters = map[string]float64{}
 
+			log.Debugf("Error log %s stats count: %d", logFile, len(simpleMetrics))
+
 			c.buf = append(c.buf, metrics.NewStatsEntityWrapper(c.baseDimensions.ToDimensions(), simpleMetrics, proto.MetricsReport_INSTANCE))
 
 			mu.Unlock()