refactoring metrics

flow/flow.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/northwesternmutual/kanali/controller"
+	"github.com/northwesternmutual/kanali/metrics"
 	"github.com/opentracing/opentracing-go"
 )
 
@@ -40,11 +41,11 @@ func (f *Flow) Add(steps ...Step) {
 }
 
 // Play executes all step in a flow in the order they were added.
-func (f *Flow) Play(ctx context.Context, ctlr *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error {
+func (f *Flow) Play(ctx context.Context, metrics *metrics.Metrics, ctlr *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error {
 	logrus.Infof("a flow with %d step is about to play", len(*f))
 	for _, step := range *f {
 		logrus.Infof("playing step %s", step.GetName())
-		if err := step.Do(ctx, ctlr, w, r, resp, trace); err != nil {
+		if err := step.Do(ctx, metrics, ctlr, w, r, resp, trace); err != nil {
 			trace.SetTag("error", true)
 			trace.LogKV(
 				"event", "error",

flow/flow_test.go

@@ -27,6 +27,7 @@ import (
 	"testing"
 
 	"github.com/northwesternmutual/kanali/controller"
+	"github.com/northwesternmutual/kanali/metrics"
 	opentracing "github.com/opentracing/opentracing-go"
 	"github.com/stretchr/testify/assert"
 )
@@ -37,7 +38,7 @@ func (s mockStep) GetName() string {
 	return "mock step"
 }
 
-func (s mockStep) Do(ctx context.Context, ctlr *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error {
+func (s mockStep) Do(ctx context.Context, m *metrics.Metrics, ctlr *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error {
 	return nil
 }
 
@@ -47,7 +48,7 @@ func (s mockErrorStep) GetName() string {
 	return "mock error step"
 }
 
-func (s mockErrorStep) Do(ctx context.Context, ctlr *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error {
+func (s mockErrorStep) Do(ctx context.Context, m *metrics.Metrics, ctlr *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error {
 	return errors.New("forced error")
 }
 
@@ -58,14 +59,14 @@ func TestAdd(t *testing.T) {
 	assert.Equal(t, len(*f), 3)
 	for _, step := range *f {
 		assert.Equal(t, step.GetName(), "mock step")
-		assert.Nil(t, step.Do(context.Background(), nil, nil, nil, nil, opentracing.StartSpan("test span")))
+		assert.Nil(t, step.Do(context.Background(), nil, nil, nil, nil, nil, opentracing.StartSpan("test span")))
 	}
 }
 
 func TestPlay(t *testing.T) {
 	f := &Flow{}
 	f.Add(mockStep{})
-	assert.Nil(t, f.Play(context.Background(), nil, nil, nil, nil, opentracing.StartSpan("test span")))
+	assert.Nil(t, f.Play(context.Background(), nil, nil, nil, nil, nil, opentracing.StartSpan("test span")))
 	f.Add(mockErrorStep{})
-	assert.Error(t, f.Play(context.Background(), nil, nil, nil, nil, opentracing.StartSpan("test span")))
+	assert.Error(t, f.Play(context.Background(), nil, nil, nil, nil, nil, opentracing.StartSpan("test span")))
 }

flow/step.go

@@ -25,11 +25,12 @@ import (
 	"net/http"
 
 	"github.com/northwesternmutual/kanali/controller"
+	"github.com/northwesternmutual/kanali/metrics"
 	"github.com/opentracing/opentracing-go"
 )
 
 // Step is an interface that represents a step to be used in a flow
 type Step interface {
 	GetName() string
-	Do(ctx context.Context, ctlr *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error
+	Do(ctx context.Context, metrics *metrics.Metrics, ctlr *controller.Controller, w http.ResponseWriter, r *http.Request, resp *http.Response, trace opentracing.Span) error
 }

handlers/handler.go

@@ -30,7 +30,7 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/northwesternmutual/kanali/config"
 	"github.com/northwesternmutual/kanali/controller"
-	"github.com/northwesternmutual/kanali/monitor"
+	"github.com/northwesternmutual/kanali/metrics"
 	"github.com/northwesternmutual/kanali/utils"
 	"github.com/opentracing/opentracing-go"
 	"github.com/spf13/viper"
@@ -39,8 +39,8 @@ import (
 // Handler is used to provide additional parameters to an HTTP handler
 type Handler struct {
 	*controller.Controller
-	context.Context
-	H func(ctx context.Context, c *controller.Controller, w http.ResponseWriter, r *http.Request, trace opentracing.Span) error
+	*metrics.Metrics
+	H func(ctx context.Context, m *metrics.Metrics, c *controller.Controller, w http.ResponseWriter, r *http.Request, trace opentracing.Span) error
 }
 
 func (h Handler) serveHTTP(w http.ResponseWriter, r *http.Request) {
@@ -72,7 +72,7 @@ func (h Handler) serveHTTP(w http.ResponseWriter, r *http.Request) {
 
 	defer sp.Finish()
 
-	err = h.H(h.Context, h.Controller, w, r, sp)
+	err = h.H(context.Background(), h.Metrics, h.Controller, w, r, sp)
 
 	// handle request errors
 	if err != nil {
@@ -92,7 +92,7 @@ func (h Handler) serveHTTP(w http.ResponseWriter, r *http.Request) {
 				"uri":    r.RequestURI,
 			}).Error(e.Error())
 
-			h.Context = monitor.AddCtxMetric(h.Context, "http_response_code", strconv.Itoa(e.Status()))
+			h.Metrics.Add(metrics.Metric{"http_response_code", strconv.Itoa(e.Status()), true})
 
 			errStatus, err := json.Marshal(utils.JSONErr{Code: e.Status(), Msg: e.Error()})
 			if err != nil {
@@ -119,7 +119,7 @@ func (h Handler) serveHTTP(w http.ResponseWriter, r *http.Request) {
 				"uri":    r.RequestURI,
 			}).Error("unknown error")
 
-			h.Context = monitor.AddCtxMetric(h.Context, "http_response_code", strconv.Itoa(http.StatusInternalServerError))
+			h.Metrics.Add(metrics.Metric{"http_response_code", strconv.Itoa(http.StatusInternalServerError), true})
 
 			errStatus, err := json.Marshal(utils.JSONErr{Code: http.StatusInternalServerError, Msg: "unknown error"})
 			if err != nil {

handlers/incoming.go

@@ -27,14 +27,15 @@ import (
 	"github.com/northwesternmutual/kanali/config"
 	"github.com/northwesternmutual/kanali/controller"
 	"github.com/northwesternmutual/kanali/flow"
+	"github.com/northwesternmutual/kanali/metrics"
 	"github.com/northwesternmutual/kanali/spec"
 	"github.com/northwesternmutual/kanali/steps"
 	"github.com/opentracing/opentracing-go"
 	"github.com/spf13/viper"
 )
 
 // IncomingRequest orchestrates the logic that occurs for every incoming request
-func IncomingRequest(ctx context.Context, ctlr *controller.Controller, w http.ResponseWriter, r *http.Request, trace opentracing.Span) error {
+func IncomingRequest(ctx context.Context, m *metrics.Metrics, ctlr *controller.Controller, w http.ResponseWriter, r *http.Request, trace opentracing.Span) error {
 
 	// this is a handler to our future proxy pass response
 	// maybe there's a better way to do this... seems misplaced
@@ -57,7 +58,7 @@ func IncomingRequest(ctx context.Context, ctlr *controller.Controller, w http.Re
 		steps.WriteResponseStep{},
 	)
 
-	err := f.Play(ctx, ctlr, w, r, futureResponse, trace)
+	err := f.Play(ctx, m, ctlr, w, r, futureResponse, trace)
 
 	return err
 

handlers/logger.go

@@ -21,13 +21,13 @@
 package handlers
 
 import (
-	"context"
 	"net/http"
 	"strconv"
 	"strings"
 	"time"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/northwesternmutual/kanali/metrics"
 	"github.com/northwesternmutual/kanali/monitor"
 )
 
@@ -38,7 +38,7 @@ func Logger(influxCtlr *monitor.InfluxController, inner Handler) http.Handler {
 
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 
-		inner.Context = context.WithValue(context.Background(), monitor.MetricsKey, monitor.New())
+		inner.Metrics = &metrics.Metrics{}
 
 		t0 := time.Now()
 		inner.serveHTTP(w, r)
@@ -51,12 +51,14 @@ func Logger(influxCtlr *monitor.InfluxController, inner Handler) http.Handler {
 			"totalTime": int(t1.Sub(t0) / time.Millisecond),
 		}).Info("request details")
 
-		inner.Context = monitor.AddCtxMetric(inner.Context, "total_time", strconv.Itoa(int(t1.Sub(t0)/time.Millisecond)))
-		inner.Context = monitor.AddCtxMetric(inner.Context, "http_method", r.Method)
-		inner.Context = monitor.AddCtxMetric(inner.Context, "http_uri", r.RequestURI)
-		inner.Context = monitor.AddCtxMetric(inner.Context, "client_ip", strings.Split(r.RemoteAddr, ":")[0])
+		inner.Metrics.Add(
+			metrics.Metric{"total_time", strconv.Itoa(int(t1.Sub(t0) / time.Millisecond)), false},
+			metrics.Metric{"http_method", r.Method, true},
+			metrics.Metric{"http_uri", r.RequestURI, false},
+			metrics.Metric{"client_ip", strings.Split(r.RemoteAddr, ":")[0], false},
+		)
 
-		if err := influxCtlr.WriteRequestData(inner.Context); err != nil {
+		if err := influxCtlr.WriteRequestData(inner.Metrics); err != nil {
 			logrus.Warnf(err.Error())
 		} else {
 			logrus.Infof("successfully wrote request details to influxdb")

metrics/metrics.go

@@ -0,0 +1,18 @@
+package metrics
+
+// Metric represent a single request metric
+type Metric struct {
+	Name  string
+	Value string
+	Index bool
+}
+
+// Metrics represents a list of Metrics associated with a request
+type Metrics []Metric
+
+// Add appends a metric to the list of metrics
+func (m *Metrics) Add(metrics ...Metric) {
+	for _, metric := range metrics {
+		*m = append(*m, metric)
+	}
+}

metrics/metrics_test.go

@@ -0,0 +1,23 @@
+package metrics
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAdd(t *testing.T) {
+	f := &Metrics{}
+	f.Add(Metric{"nameOne", "valueOne", false})
+	f.Add(Metric{"nameTwo", "valueTwo", true}, Metric{"nameThree", "valueThree", false})
+	assert.Equal(t, len(*f), 3)
+	assert.False(t, (*f)[0].Index)
+	assert.True(t, (*f)[1].Index)
+	assert.False(t, (*f)[2].Index)
+	assert.Equal(t, (*f)[0].Name, "nameOne")
+	assert.Equal(t, (*f)[1].Name, "nameTwo")
+	assert.Equal(t, (*f)[2].Name, "nameThree")
+	assert.Equal(t, (*f)[0].Value, "valueOne")
+	assert.Equal(t, (*f)[1].Value, "valueTwo")
+	assert.Equal(t, (*f)[2].Value, "valueThree")
+}

monitor/influxdb.go

@@ -21,13 +21,13 @@
 package monitor
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"time"
 
 	influx "github.com/influxdata/influxdb/client/v2"
 	"github.com/northwesternmutual/kanali/config"
+	"github.com/northwesternmutual/kanali/metrics"
 	"github.com/spf13/viper"
 )
 
@@ -60,7 +60,7 @@ func NewInfluxdbController() (*InfluxController, error) {
 }
 
 // WriteRequestData writes contextual request metrics to Influxdb
-func (c *InfluxController) WriteRequestData(ctx context.Context) (err error) {
+func (c *InfluxController) WriteRequestData(m *metrics.Metrics) (err error) {
 
 	defer func() {
 		if r := recover(); r != nil {
@@ -76,7 +76,17 @@ func (c *InfluxController) WriteRequestData(ctx context.Context) (err error) {
 		return err
 	}
 
-	pt, err := influx.NewPoint("request_details", getTags(ctx), getFields(ctx), time.Now())
+	tags := make(map[string]string)
+	fields := make(map[string]interface{})
+
+	for _, metric := range *m {
+		fields[metric.Name] = metric.Value
+		if metric.Index {
+			tags[metric.Name] = metric.Value
+		}
+	}
+
+	pt, err := influx.NewPoint("request_details", tags, fields, time.Now())
 	if err != nil {
 		return err
 	}
@@ -86,31 +96,3 @@ func (c *InfluxController) WriteRequestData(ctx context.Context) (err error) {
 	return c.Client.Write(bp)
 
 }
-
-func getTags(ctx context.Context) map[string]string {
-
-	keyName := GetCtxMetric(ctx, "api_key_name")
-	if keyName == "" {
-		keyName = "none"
-	}
-
-	return map[string]string{
-		"proxyName":      GetCtxMetric(ctx, "proxy_name"),
-		"responseCode":   GetCtxMetric(ctx, "http_response_code"),
-		"method":         GetCtxMetric(ctx, "http_method"),
-		"keyName":        keyName,
-		"proxyNamespace": GetCtxMetric(ctx, "proxy_namespace"),
-	}
-
-}
-
-func getFields(ctx context.Context) map[string]interface{} {
-
-	return map[string]interface{}{
-		"totalTime":    GetCtxMetric(ctx, "total_time"),
-		"clientIP":     GetCtxMetric(ctx, "client_ip"),
-		"responseCode": GetCtxMetric(ctx, "http_response_code"),
-		"uri":          GetCtxMetric(ctx, "http_uri"),
-	}
-
-}