Merge pull request #1994 from codysoyland/prometheus

Add Prometheus tests, refactor http stats as middleware, minor fixes

go.mod

@@ -23,6 +23,7 @@ require (
 	github.com/pelletier/go-toml v1.2.0
 	github.com/pkg/errors v0.8.1
 	github.com/prometheus/client_golang v0.9.3
+	github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90
 	github.com/remyoudompheng/bigfft v0.0.0-20190321074620-2f0d2b0e0001 // indirect
 	github.com/satori/go.uuid v1.2.0
 	github.com/shirou/gopsutil v2.18.12+incompatible

http/handler.go

@@ -235,6 +235,41 @@ func (h *Handler) extractTracing(next http.Handler) http.Handler {
 	})
 }
 
+func (h *Handler) collectStats(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t := time.Now()
+		next.ServeHTTP(w, r)
+		dur := time.Since(t)
+
+		statsTags := make([]string, 0, 5)
+
+		longQueryTime := h.api.LongQueryTime()
+		if longQueryTime > 0 && dur > longQueryTime {
+			h.logger.Printf("%s %s %v", r.Method, r.URL.String(), dur)
+			statsTags = append(statsTags, "slow_query")
+		}
+
+		pathParts := strings.Split(r.URL.Path, "/")
+		if externalPrefixFlag[pathParts[1]] {
+			statsTags = append(statsTags, "external")
+		}
+
+		statsTags = append(statsTags, "useragent:"+r.UserAgent())
+
+		path, err := mux.CurrentRoute(r).GetPathTemplate()
+		if err == nil {
+			statsTags = append(statsTags, "path:"+path)
+		}
+
+		statsTags = append(statsTags, "method:"+r.Method)
+
+		stats := h.api.StatsWithTags(statsTags)
+		if stats != nil {
+			stats.Timing("http.request", dur, 0.1)
+		}
+	})
+}
+
 // newRouter creates a new mux http router.
 func newRouter(handler *Handler) *mux.Router {
 	router := mux.NewRouter()
@@ -280,6 +315,7 @@ func newRouter(handler *Handler) *mux.Router {
 
 	router.Use(handler.queryArgValidator)
 	router.Use(handler.extractTracing)
+	router.Use(handler.collectStats)
 	return router
 }
 
@@ -295,32 +331,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 	}()
 
-	t := time.Now()
 	h.Handler.ServeHTTP(w, r)
-	dif := time.Since(t)
-
-	// Calculate per request StatsD metrics when the handler is fully configured.
-	statsTags := make([]string, 0, 4)
-
-	pathParts := strings.Split(r.URL.Path, "/")
-	longQueryTime := h.api.LongQueryTime()
-	if longQueryTime > 0 && dif > longQueryTime {
-		h.logger.Printf("%s %s %v", r.Method, r.URL.String(), dif)
-		statsTags = append(statsTags, "slow_query")
-	}
-
-	statsTags = append(statsTags, "url:"+r.URL.Path)
-
-	if externalPrefixFlag[pathParts[1]] {
-		statsTags = append(statsTags, "external")
-	}
-
-	// useragent tag identifies internal/external endpoints
-	statsTags = append(statsTags, "useragent:"+r.UserAgent())
-	stats := h.api.StatsWithTags(statsTags)
-	if stats != nil {
-		stats.Histogram("http.request", float64(dif), 0.1)
-	}
 }
 
 // successResponse is a general success/error struct for http responses.

prometheus/prometheus.go

@@ -177,6 +177,7 @@ func (c *prometheusClient) Gauge(name string, value float64, rate float64) {
 		gauge, err = gaugeVec.GetMetricWith(labels)
 		if err != nil {
 			c.logger.Printf("gaugeVec.GetMetricWith error: %s", err)
+			return
 		}
 	}
 	gauge.Set(float64(value))
@@ -219,6 +220,7 @@ func (c *prometheusClient) Histogram(name string, value float64, rate float64) {
 		observer, err = summaryVec.GetMetricWith(labels)
 		if err != nil {
 			c.logger.Printf("summaryVec.GetMetricWith error: %s", err)
+			return
 		}
 	}
 	observer.Observe(value)
@@ -231,7 +233,7 @@ func (c *prometheusClient) Set(name string, value string, rate float64) {
 
 // Timing tracks timing information for a metric.
 func (c *prometheusClient) Timing(name string, value time.Duration, rate float64) {
-	durationMs := value / time.Millisecond
+	durationMs := value / time.Second
 	c.Histogram(name, float64(durationMs), rate)
 }
 

prometheus/prometheus_test.go

@@ -0,0 +1,82 @@
+// Copyright 2017 Pilosa Corp.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package prometheus_test
+
+import (
+	"reflect"
+	"testing"
+	"time"
+
+	pilosaPrometheus "github.com/pilosa/pilosa/prometheus"
+	"github.com/prometheus/client_golang/prometheus"
+	io_prometheus_client "github.com/prometheus/client_model/go"
+)
+
+func TestPrometheusClient_WithTags(t *testing.T) {
+	// Create a new client.
+	c, err := pilosaPrometheus.NewPrometheusClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer c.Close()
+
+	// Create a new client with additional tags.
+	c1 := c.WithTags("foo", "bar")
+	if tags := c1.Tags(); !reflect.DeepEqual(tags, []string{"bar", "foo"}) {
+		t.Fatalf("unexpected tags: %+v", tags)
+	}
+
+	// Create a new client from the clone with more tags.
+	c2 := c1.WithTags("bar", "baz")
+	if tags := c2.Tags(); !reflect.DeepEqual(tags, []string{"bar", "baz", "foo"}) {
+		t.Fatalf("unexpected tags: %+v", tags)
+	}
+}
+
+func TestPrometheusClient_Methods(t *testing.T) {
+	// Create a new client.
+	c, err := pilosaPrometheus.NewPrometheusClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer c.Close()
+
+	dur, _ := time.ParseDuration("123us")
+	c.CountWithCustomTags("ct", 1, 1.0, []string{"foo:bar"})
+	c.Count("cc", 1, 1.0)
+	c.Gauge("gg", 10, 1.0)
+	c.Histogram("hh", 1, 1.0)
+	c.Timing("tt", dur, 1.0)
+
+	metricFams, err := prometheus.DefaultGatherer.Gather()
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, metricName := range []string{"pilosa_ct", "pilosa_cc", "pilosa_gg", "pilosa_hh", "pilosa_tt"} {
+		if metricExists(metricName, metricFams) {
+			continue
+		}
+		t.Fatalf("Metric was not recorded: %s", metricName)
+	}
+}
+
+func metricExists(metricName string, metricFams []*io_prometheus_client.MetricFamily) bool {
+	for _, metricFam := range metricFams {
+		if metricFam.GetName() == metricName {
+			return true
+		}
+	}
+	return false
+}