Tidying up the new Metrics implementations (#1974)

* Unit tests to check tagging and aggregation of Prometheus metrics

* Removing the logger output routing (because it doesn't actually work in the logger implementation)

* Emitting values with the logging reporter

Co-authored-by: chris <chris@Profanity.local>

metrics/logging/reporter.go

@@ -3,43 +3,43 @@ package logging
 import (
 	"time"
 
-	log "github.com/micro/go-micro/v3/logger"
+	"github.com/micro/go-micro/v3/logger"
 	"github.com/micro/go-micro/v3/metrics"
 )
 
+const (
+	defaultLoggingLevel = logger.TraceLevel
+)
+
 // Reporter is an implementation of metrics.Reporter:
 type Reporter struct {
-	logger  log.Logger
 	options metrics.Options
 }
 
-// New returns a configured noop reporter:
+// New returns a configured logging reporter:
 func New(opts ...metrics.Option) *Reporter {
-	options := metrics.NewOptions(opts...)
-	logger := log.NewLogger(log.WithFields(convertTags(options.DefaultTags)))
-	logger.Log(log.InfoLevel, "Metrics/Logging - metrics will be logged (at TRACE level)")
+	logger.Logf(logger.InfoLevel, "Metrics/Logging - metrics will be logged (at %s level)", defaultLoggingLevel.String())
 
 	return &Reporter{
-		logger:  logger,
 		options: metrics.NewOptions(opts...),
 	}
 }
 
 // Count implements the metrics.Reporter interface Count method:
 func (r *Reporter) Count(metricName string, value int64, tags metrics.Tags) error {
-	r.logger.Logf(log.TraceLevel, "Count metric: %s", tags)
+	logger.Logf(defaultLoggingLevel, "Count metric: (%s: %d) %s", metricName, value, tags)
 	return nil
 }
 
 // Gauge implements the metrics.Reporter interface Gauge method:
 func (r *Reporter) Gauge(metricName string, value float64, tags metrics.Tags) error {
-	r.logger.Logf(log.TraceLevel, "Gauge metric: %s", tags)
+	logger.Logf(defaultLoggingLevel, "Gauge metric: (%s: %f) %s", metricName, value, tags)
 	return nil
 }
 
 // Timing implements the metrics.Reporter interface Timing method:
 func (r *Reporter) Timing(metricName string, value time.Duration, tags metrics.Tags) error {
-	r.logger.Logf(log.TraceLevel, "Timing metric: %s", tags)
+	logger.Logf(defaultLoggingLevel, "Timing metric: (%s: %s) %s", metricName, value.String(), tags)
 	return nil
 }
 

metrics/logging/reporter_test.go

@@ -1,11 +1,9 @@
 package logging
 
 import (
-	"bytes"
 	"testing"
 	"time"
 
-	log "github.com/micro/go-micro/v3/logger"
 	"github.com/micro/go-micro/v3/metrics"
 
 	"github.com/stretchr/testify/assert"
@@ -20,10 +18,6 @@ func TestLoggingReporter(t *testing.T) {
 	assert.Equal(t, ":9000", reporter.options.Address)
 	assert.Equal(t, "/prometheus", reporter.options.Path)
 
-	// Make a log buffer and create a new logger to use it:
-	logBuffer := new(bytes.Buffer)
-	reporter.logger = log.NewLogger(log.WithLevel(log.TraceLevel), log.WithOutput(logBuffer), log.WithFields(convertTags(reporter.options.DefaultTags)))
-
 	// Check that our implementation is valid:
 	assert.Implements(t, new(metrics.Reporter), reporter)
 
@@ -45,7 +39,4 @@ func TestLoggingReporter(t *testing.T) {
 	assert.NoError(t, reporter.Gauge("test.gauge.1", 98, tags))
 	assert.NoError(t, reporter.Timing("test.timing.1", time.Second, tags))
 	assert.NoError(t, reporter.Timing("test.timing.2", time.Minute, tags))
-
-	// Test reading back the metrics from the logbuffer (doesn't seem to work because the output still goes to StdOut):
-	// assert.Contains(t, logBuffer.String(), "level=debug service=prometheus-test Count metric: map[tag1:false tag2:true]")
 }

metrics/prometheus/reporter_test.go

@@ -1,6 +1,8 @@
 package prometheus
 
 import (
+	"io/ioutil"
+	"net/http"
 	"testing"
 	"time"
 
@@ -12,11 +14,11 @@ import (
 func TestPrometheusReporter(t *testing.T) {
 
 	// Make a Reporter:
-	reporter, err := New(metrics.Path("/prometheus"), metrics.DefaultTags(map[string]string{"service": "prometheus-test"}))
+	reporter, err := New(metrics.Address(":9999"), metrics.Path("/prometheus"), metrics.DefaultTags(map[string]string{"service": "prometheus-test"}))
 	assert.NoError(t, err)
 	assert.NotNil(t, reporter)
 	assert.Equal(t, "prometheus-test", reporter.options.DefaultTags["service"])
-	assert.Equal(t, ":9000", reporter.options.Address)
+	assert.Equal(t, ":9999", reporter.options.Address)
 	assert.Equal(t, "/prometheus", reporter.options.Path)
 
 	// Check that our implementation is valid:
@@ -69,5 +71,19 @@ func TestPrometheusReporter(t *testing.T) {
 	assert.Len(t, reporter.metrics.timings, 2)
 
 	// Test reading back the metrics:
-	// This could be done by hitting the /metrics endpoint
+	rsp, err := http.Get("http://localhost:9999/prometheus")
+	assert.NoError(t, err)
+	assert.Equal(t, http.StatusOK, rsp.StatusCode)
+
+	// Read the response body and check for our metric:
+	bodyBytes, err := ioutil.ReadAll(rsp.Body)
+	assert.NoError(t, err)
+
+	// Check for appropriately aggregated metrics:
+	assert.Contains(t, string(bodyBytes), `test_counter_1{service="prometheus-test",tag1="false",tag2="true"} 11`)
+	assert.Contains(t, string(bodyBytes), `test_counter_2{service="prometheus-test",tag1="false",tag2="true"} 19`)
+	assert.Contains(t, string(bodyBytes), `test_gauge_1{service="prometheus-test",tag1="false",tag2="true"} 98`)
+	assert.Contains(t, string(bodyBytes), `test_gauge_2{service="prometheus-test",tag1="false",tag2="true"} 55`)
+	assert.Contains(t, string(bodyBytes), `test_timing_1{service="prometheus-test",tag1="false",tag2="true",quantile="0"} 1`)
+	assert.Contains(t, string(bodyBytes), `test_timing_2{service="prometheus-test",tag1="false",tag2="true",quantile="0"} 60`)
 }