feat: using metrics registry to get all metrics (#121)

* feat: using metrics registry to get all metrics

Author: pawels-optimizely

cmd/main.go

@@ -30,6 +30,7 @@ import (
 	"github.com/spf13/viper"
 
 	"github.com/optimizely/agent/config"
+	"github.com/optimizely/agent/pkg/metrics"
 	"github.com/optimizely/agent/pkg/optimizely"
 	"github.com/optimizely/agent/pkg/routers"
 	"github.com/optimizely/agent/pkg/server"
@@ -98,9 +99,12 @@ func main() {
 	conf := loadConfig(v)
 	initLogging(conf.Log)
 
+	agentMetricsRegistry := metrics.NewRegistry()
+	sdkMetricsRegistry := optimizely.NewRegistry(agentMetricsRegistry)
+
 	ctx, cancel := context.WithCancel(context.Background()) // Create default service context
 	sg := server.NewGroup(ctx, conf.Server)                 // Create a new server group to manage the individual http listeners
-	optlyCache := optimizely.NewCache(ctx, conf.Optly)
+	optlyCache := optimizely.NewCache(ctx, conf.Optly, sdkMetricsRegistry)
 
 	// goroutine to check for signals to gracefully shutdown listeners
 	go func() {
@@ -114,7 +118,7 @@ func main() {
 	}()
 
 	log.Info().Str("version", conf.Admin.Version).Msg("Starting services.")
-	sg.GoListenAndServe("api", conf.API.Port, routers.NewDefaultAPIRouter(optlyCache, conf.API))
+	sg.GoListenAndServe("api", conf.API.Port, routers.NewDefaultAPIRouter(optlyCache, conf.API, agentMetricsRegistry))
 	sg.GoListenAndServe("webhook", conf.Webhook.Port, routers.NewWebhookRouter(optlyCache, conf.Webhook))
 	sg.GoListenAndServe("admin", conf.Admin.Port, routers.NewAdminRouter(conf.Admin)) // Admin should be added last.
 

pkg/metrics/metrics.go

@@ -20,44 +20,83 @@ package metrics
 import (
 	"sync"
 
-	"github.com/optimizely/go-sdk/pkg/metrics"
+	go_kit_metrics "github.com/go-kit/kit/metrics"
 
 	go_kit_expvar "github.com/go-kit/kit/metrics/expvar"
 	"github.com/rs/zerolog/log"
 )
 
+// CounterPrefix stores the prefix for Counter
 const (
-	counterPrefix = "counter"
-	gaugePrefix   = "gauge"
+	CounterPrefix = "counter"
+	GaugePrefix   = "gauge"
+	TimerPrefix   = "timer"
 )
 
+// Timer is the collection of some timers
+type Timer struct {
+	hits      go_kit_metrics.Counter
+	totalTime go_kit_metrics.Counter
+	histogram go_kit_metrics.Histogram
+}
+
+// NewTimer constructs Timer
+func (m *Registry) NewTimer(key string) *Timer {
+	if key == "" {
+		log.Warn().Msg("metrics timer key is empty")
+		return nil
+	}
+	combinedKey := TimerPrefix + "." + key
+
+	m.timerLock.Lock()
+	defer m.timerLock.Unlock()
+	if val, ok := m.metricsTimerVars[combinedKey]; ok {
+		return val
+	}
+
+	return m.createTimer(combinedKey)
+}
+
+// Update timer components
+func (t *Timer) Update(delta float64) {
+	t.hits.Add(1)
+	t.totalTime.Add(delta)
+	t.histogram.Observe(delta)
+}
+
 // Registry initializes expvar metrics registry
 type Registry struct {
-	metricsCounterVars map[string]*go_kit_expvar.Counter
-	metricsGaugeVars   map[string]*go_kit_expvar.Gauge
-
-	gaugeLock   sync.RWMutex
-	counterLock sync.RWMutex
+	metricsCounterVars   map[string]go_kit_metrics.Counter
+	metricsGaugeVars     map[string]go_kit_metrics.Gauge
+	metricsHistogramVars map[string]go_kit_metrics.Histogram
+	metricsTimerVars     map[string]*Timer
+
+	gaugeLock     sync.RWMutex
+	counterLock   sync.RWMutex
+	histogramLock sync.RWMutex
+	timerLock     sync.RWMutex
 }
 
 // NewRegistry initializes metrics registry
 func NewRegistry() *Registry {
 
 	return &Registry{
-		metricsCounterVars: map[string]*go_kit_expvar.Counter{},
-		metricsGaugeVars:   map[string]*go_kit_expvar.Gauge{},
+		metricsCounterVars:   map[string]go_kit_metrics.Counter{},
+		metricsGaugeVars:     map[string]go_kit_metrics.Gauge{},
+		metricsHistogramVars: map[string]go_kit_metrics.Histogram{},
+		metricsTimerVars:     map[string]*Timer{},
 	}
 }
 
 // GetCounter gets go-kit expvar Counter
-func (m *Registry) GetCounter(key string) metrics.Counter {
+func (m *Registry) GetCounter(key string) go_kit_metrics.Counter {
 
 	if key == "" {
 		log.Warn().Msg("metrics counter key is empty")
-		return &metrics.NoopCounter{}
+		return nil
 	}
 
-	combinedKey := counterPrefix + "." + key
+	combinedKey := CounterPrefix + "." + key
 
 	m.counterLock.Lock()
 	defer m.counterLock.Unlock()
@@ -69,14 +108,14 @@ func (m *Registry) GetCounter(key string) metrics.Counter {
 }
 
 // GetGauge gets go-kit expvar Gauge
-func (m *Registry) GetGauge(key string) metrics.Gauge {
+func (m *Registry) GetGauge(key string) go_kit_metrics.Gauge {
 
 	if key == "" {
 		log.Warn().Msg("metrics gauge key is empty")
-		return &metrics.NoopGauge{}
+		return nil
 	}
 
-	combinedKey := gaugePrefix + "." + key
+	combinedKey := GaugePrefix + "." + key
 
 	m.gaugeLock.Lock()
 	defer m.gaugeLock.Unlock()
@@ -86,6 +125,22 @@ func (m *Registry) GetGauge(key string) metrics.Gauge {
 	return m.createGauge(combinedKey)
 }
 
+// GetHistogram gets go-kit Histogram
+func (m *Registry) GetHistogram(key string) go_kit_metrics.Histogram {
+
+	if key == "" {
+		log.Warn().Msg("metrics gauge key is empty")
+		return nil
+	}
+
+	m.histogramLock.Lock()
+	defer m.histogramLock.Unlock()
+	if val, ok := m.metricsHistogramVars[key]; ok {
+		return val
+	}
+	return m.createHistogram(key)
+}
+
 func (m *Registry) createGauge(key string) *go_kit_expvar.Gauge {
 	gaugeVar := go_kit_expvar.NewGauge(key)
 	m.metricsGaugeVars[key] = gaugeVar
@@ -99,3 +154,22 @@ func (m *Registry) createCounter(key string) *go_kit_expvar.Counter {
 	return counterVar
 
 }
+
+func (m *Registry) createHistogram(key string) *go_kit_expvar.Histogram {
+	histogramVar := go_kit_expvar.NewHistogram(key, 50)
+	m.metricsHistogramVars[key] = histogramVar
+	return histogramVar
+
+}
+
+func (m *Registry) createTimer(key string) *Timer {
+	timerVar := &Timer{
+		hits:      m.createCounter(key + ".hits"),
+		totalTime: m.createCounter(key + ".responseTime"),
+		histogram: m.createHistogram(key + ".responseTimeHist"),
+	}
+
+	m.metricsTimerVars[key] = timerVar
+	return timerVar
+
+}

pkg/metrics/metrics_test.go

@@ -23,15 +23,11 @@ import (
 	"net/http/httptest"
 	"testing"
 
-	"github.com/optimizely/go-sdk/pkg/metrics"
-
 	"github.com/stretchr/testify/assert"
 )
 
 type JSON map[string]interface{}
 
-var metricPrefix = "prefix"
-
 func TestCounterValid(t *testing.T) {
 
 	rec := httptest.NewRecorder()
@@ -77,7 +73,7 @@ func TestCounterEmptyKey(t *testing.T) {
 	metricsRegistry := NewRegistry()
 	counter := metricsRegistry.GetCounter("")
 
-	assert.Equal(t, &metrics.NoopCounter{}, counter)
+	assert.Nil(t, counter)
 
 }
 
@@ -126,6 +122,100 @@ func TestGaugeEmptyKey(t *testing.T) {
 	metricsRegistry := NewRegistry()
 	gauge := metricsRegistry.GetGauge("")
 
-	assert.Equal(t, &metrics.NoopGauge{}, gauge)
+	assert.Nil(t, gauge)
+
+}
+
+func TestHistorgramValid(t *testing.T) {
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/", nil)
+
+	metricsRegistry := NewRegistry()
+	histogram := metricsRegistry.GetHistogram("metrics")
+	histogram.Observe(12)
+	histogram.Observe(23)
+
+	expvar.Handler().ServeHTTP(rec, req)
+
+	var expVarMap JSON
+	err := json.Unmarshal(rec.Body.Bytes(), &expVarMap)
+	assert.Nil(t, err)
+	assert.Equal(t, 12.0, expVarMap["metrics.p50"])
+	assert.Equal(t, 23.0, expVarMap["metrics.p99"])
+
+}
+
+func TestHistogramMultipleRetrievals(t *testing.T) {
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/", nil)
+
+	metricsRegistry := NewRegistry()
+	histogramKey := "next_histogram_metrics"
+	histogram := metricsRegistry.GetHistogram(histogramKey)
+	histogram.Observe(12)
+	nextGauge := metricsRegistry.GetHistogram(histogramKey)
+	nextGauge.Observe(23)
+
+	expvar.Handler().ServeHTTP(rec, req)
+
+	var expVarMap JSON
+	err := json.Unmarshal(rec.Body.Bytes(), &expVarMap)
+	assert.Nil(t, err)
+	assert.Equal(t, 12.0, expVarMap["next_histogram_metrics.p50"])
+	assert.Equal(t, 23.0, expVarMap["next_histogram_metrics.p99"])
+
+}
+
+func TestHistogramEmptyKey(t *testing.T) {
+
+	metricsRegistry := NewRegistry()
+	histogram := metricsRegistry.GetHistogram("")
+
+	assert.Nil(t, histogram)
+
+}
+func TestTimerValid(t *testing.T) {
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/", nil)
+
+	metricsRegistry := NewRegistry()
+	timer := metricsRegistry.NewTimer("metrics")
+	timer.Update(12)
+	timer.Update(23)
+
+	expvar.Handler().ServeHTTP(rec, req)
+
+	var expVarMap JSON
+	err := json.Unmarshal(rec.Body.Bytes(), &expVarMap)
+	assert.Nil(t, err)
+	assert.Equal(t, 2.0, expVarMap["timer.metrics.hits"])
+	assert.Equal(t, 35.0, expVarMap["timer.metrics.responseTime"])
+	assert.Equal(t, 12.0, expVarMap["timer.metrics.responseTimeHist.p50"])
+	assert.Equal(t, 23.0, expVarMap["timer.metrics.responseTimeHist.p99"])
+}
+
+func TestTimerMultipleRetrievals(t *testing.T) {
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/", nil)
+
+	metricsRegistry := NewRegistry()
+	timerKey := "next_timer_metrics"
+	timer := metricsRegistry.NewTimer(timerKey)
+	timer.Update(12)
+	nextTimer := metricsRegistry.NewTimer(timerKey)
+	nextTimer.Update(23)
+
+	expvar.Handler().ServeHTTP(rec, req)
+
+	var expVarMap JSON
+	err := json.Unmarshal(rec.Body.Bytes(), &expVarMap)
+	assert.Nil(t, err)
+	assert.Equal(t, 2.0, expVarMap["timer.next_timer_metrics.hits"])
+	assert.Equal(t, 12.0, expVarMap["timer.next_timer_metrics.responseTimeHist.p50"])
+	assert.Equal(t, 23.0, expVarMap["timer.next_timer_metrics.responseTimeHist.p99"])
 
 }

pkg/middleware/metrics.go

@@ -22,53 +22,28 @@ import (
 	"net/http"
 	"time"
 
-	"github.com/go-kit/kit/metrics"
-	"github.com/go-kit/kit/metrics/expvar"
+	"github.com/optimizely/agent/pkg/metrics"
 )
 
-const metricPrefix = "timers."
-
 type contextString string
 
 const responseTime = contextString("responseTime")
 
-// Metrics struct contains url hit counts, response time and its histogram
-type Metrics struct {
-	HitCounts             metrics.Counter
-	ResponseTime          metrics.Counter
-	ResponseTimeHistogram metrics.Histogram
-}
-
-// NewMetrics initialized metrics
-func NewMetrics(key string) *Metrics {
-
-	uniqueName := metricPrefix + key
-
-	return &Metrics{
-		HitCounts:             expvar.NewCounter(uniqueName + ".counts"),
-		ResponseTime:          expvar.NewCounter(uniqueName + ".responseTime"),
-		ResponseTimeHistogram: expvar.NewHistogram(uniqueName+".responseTimeHist", 50),
-	}
-}
-
 // Metricize updates counts, total response time, and response time histogram
 // for each URL hit, key being a combination of a method and route pattern
-func Metricize(key string) func(http.Handler) http.Handler {
-	singleMetric := NewMetrics(key)
+func Metricize(key string, metricsRegistry *metrics.Registry) func(http.Handler) http.Handler {
+	singleMetric := metricsRegistry.NewTimer(key)
 
 	f := func(h http.Handler) http.Handler {
 
 		fn := func(w http.ResponseWriter, r *http.Request) {
-
-			singleMetric.HitCounts.Add(1)
 			ctx := r.Context()
 			startTime, ok := ctx.Value(responseTime).(time.Time)
 			if ok {
 				defer func() {
 					endTime := time.Now()
 					timeDiff := endTime.Sub(startTime).Seconds() * 1000.0 // display time in milliseconds
-					singleMetric.ResponseTime.Add(timeDiff)
-					singleMetric.ResponseTimeHistogram.Observe(timeDiff)
+					singleMetric.Update(timeDiff)
 				}()
 			}