Merge pull request #302 from prometheus/beorn7/http

Improve promhttp middleware

prometheus/promhttp/instrument_client_1_8_test.go

@@ -28,6 +28,8 @@ func TestClientMiddlewareAPI(t *testing.T) {
 	client := http.DefaultClient
 	client.Timeout = 1 * time.Second
 
+	reg := prometheus.NewRegistry()
+
 	inFlightGauge := prometheus.NewGauge(prometheus.GaugeOpts{
 		Name: "client_in_flight_requests",
 		Help: "A gauge of in-flight requests for the wrapped client.",
@@ -68,7 +70,7 @@ func TestClientMiddlewareAPI(t *testing.T) {
 		[]string{"method"},
 	)
 
-	prometheus.MustRegister(counter, tlsLatencyVec, dnsLatencyVec, histVec, inFlightGauge)
+	reg.MustRegister(counter, tlsLatencyVec, dnsLatencyVec, histVec, inFlightGauge)
 
 	trace := &InstrumentTrace{
 		DNSStart: func(t float64) {

prometheus/promhttp/instrument_server.go

@@ -27,6 +27,9 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
+// magicString is used for the hacky label test in checkLabels. Remove once fixed.
+const magicString = "zZgWfBxLqvG8kc8IMv3POi2Bb0tZI3vAnBx+gBaFi9FyPzB/CzKUer1yufDa"
+
 // InstrumentHandlerInFlight is a middleware that wraps the provided
 // http.Handler. It sets the provided prometheus.Gauge to the number of
 // requests currently handled by the wrapped http.Handler.
@@ -191,37 +194,46 @@ func checkLabels(c prometheus.Collector) (code bool, method bool) {
 
 	if _, err := prometheus.NewConstMetric(desc, prometheus.UntypedValue, 0); err == nil {
 		return
-	} else if m, err := prometheus.NewConstMetric(desc, prometheus.UntypedValue, 0, ""); err == nil {
+	}
+	if m, err := prometheus.NewConstMetric(desc, prometheus.UntypedValue, 0, magicString); err == nil {
 		if err := m.Write(&pm); err != nil {
 			panic("error checking metric for labels")
 		}
-
+		for _, label := range pm.Label {
-		name := *pm.Label[0].Name
+			name, value := label.GetName(), label.GetValue()
-		if name == "code" {
+			if value != magicString {
+				continue
+			}
+			switch name {
+			case "code":
 				code = true
-		} else if name == "method" {
+			case "method":
 				method = true
-		} else {
+			default:
 				panic("metric partitioned with non-supported labels")
 			}
 			return
-	} else if m, err := prometheus.NewConstMetric(desc, prometheus.UntypedValue, 0, "", ""); err == nil {
+		}
+		panic("previously set label not found – this must never happen")
+	}
+	if m, err := prometheus.NewConstMetric(desc, prometheus.UntypedValue, 0, magicString, magicString); err == nil {
 		if err := m.Write(&pm); err != nil {
 			panic("error checking metric for labels")
 		}
-
 		for _, label := range pm.Label {
-			if *label.Name == "code" || *label.Name == "method" {
+			name, value := label.GetName(), label.GetValue()
+			if value != magicString {
+				continue
+			}
+			if name == "code" || name == "method" {
 				continue
 			}
 			panic("metric partitioned with non-supported labels")
 		}
-
 		code = true
 		method = true
 		return
 	}
-
 	panic("metric partitioned with non-supported labels")
 }
 

prometheus/promhttp/instrument_server_test.go

@@ -23,6 +23,8 @@ import (
 )
 
 func TestMiddlewareAPI(t *testing.T) {
+	reg := prometheus.NewRegistry()
+
 	inFlightGauge := prometheus.NewGauge(prometheus.GaugeOpts{
 		Name: "in_flight_requests",
 		Help: "A gauge of requests currently being served by the wrapped handler.",
@@ -41,6 +43,7 @@ func TestMiddlewareAPI(t *testing.T) {
 			Name:        "response_duration_seconds",
 			Help:        "A histogram of request latencies.",
 			Buckets:     prometheus.DefBuckets,
+			ConstLabels: prometheus.Labels{"handler": "api"},
 		},
 		[]string{"method"},
 	)
@@ -58,7 +61,7 @@ func TestMiddlewareAPI(t *testing.T) {
 		w.Write([]byte("OK"))
 	})
 
-	prometheus.MustRegister(inFlightGauge, counter, histVec, responseSize)
+	reg.MustRegister(inFlightGauge, counter, histVec, responseSize)
 
 	chain := InstrumentHandlerInFlight(inFlightGauge,
 		InstrumentHandlerCounter(counter,
@@ -87,29 +90,25 @@ func ExampleInstrumentHandlerDuration() {
 		[]string{"code", "method"},
 	)
 
-	// pushVec is partitioned by the HTTP method and uses custom buckets based on
+	// pushVec and pullVec are partitioned by the HTTP method and use custom
-	// the expected request duration. It uses ConstLabels to set a handler label
+	// buckets based on the expected request duration. ConstLabels are used
-	// marking pushVec as tracking the durations for pushes.
+	// to set a handler label to mark pushVec as tracking the durations for
-	pushVec := prometheus.NewHistogramVec(
+	// pushes and pullVec as tracking the durations for pulls. Note that
-		prometheus.HistogramOpts{
+	// Name, Help, and Buckets need to be the same for consistency, so we
+	// use the same HistogramOpts after just modifying the ConstLabels.
+	histogramOpts := prometheus.HistogramOpts{
 		Name:        "request_duration_seconds",
-			Help:        "A histogram of latencies for requests to the push handler.",
+		Help:        "A histogram of latencies for requests.",
 		Buckets:     []float64{.25, .5, 1, 2.5, 5, 10},
 		ConstLabels: prometheus.Labels{"handler": "push"},
-		},
+	}
+	pushVec := prometheus.NewHistogramVec(
+		histogramOpts,
 		[]string{"method"},
 	)
-
+	histogramOpts.ConstLabels = prometheus.Labels{"handler": "pull"}
-	// pullVec is also partitioned by the HTTP method but uses custom buckets
-	// different from those for pushVec. It also has a different value for the
-	// constant "handler" label.
 	pullVec := prometheus.NewHistogramVec(
-		prometheus.HistogramOpts{
+		histogramOpts,
-			Name:        "request_duration_seconds",
-			Help:        "A histogram of latencies for requests to the pull handler.",
-			Buckets:     []float64{.005, .01, .025, .05},
-			ConstLabels: prometheus.Labels{"handler": "pull"},
-		},
 		[]string{"method"},
 	)