From 2998735bf39d41a2f573ec84dad312ae25a0653b Mon Sep 17 00:00:00 2001 From: Prawn Date: Thu, 27 Aug 2020 20:08:51 +1200 Subject: [PATCH] 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 --- metrics/logging/reporter.go | 20 ++++++++++---------- metrics/logging/reporter_test.go | 9 --------- metrics/prometheus/reporter_test.go | 22 +++++++++++++++++++--- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/metrics/logging/reporter.go b/metrics/logging/reporter.go index 8ce513d9..d1d3e479 100644 --- a/metrics/logging/reporter.go +++ b/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 } diff --git a/metrics/logging/reporter_test.go b/metrics/logging/reporter_test.go index 70bf6776..0a7d439b 100644 --- a/metrics/logging/reporter_test.go +++ b/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]") } diff --git a/metrics/prometheus/reporter_test.go b/metrics/prometheus/reporter_test.go index 4b7f87a9..74b2f452 100644 --- a/metrics/prometheus/reporter_test.go +++ b/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`) }