Instrumentation: Add examplars for request histograms (#29357)

Signed-off-by: bergquist <carl.bergquist@gmail.com>

Instrumentation: Add examplars for request histograms (#29357)
Signed-off-by: bergquist <carl.bergquist@gmail.com>
b7aa6fed · Carl Bergquist · GitHub · fb622650 · b7aa6fed · b7aa6fed
Commit b7aa6fed authored Dec 01, 2020 by Carl Bergquist Committed by GitHub Dec 01, 2020
Expand all Hide whitespace changes
Inline Side-by-side

Showing with 53 additions and 28 deletions

go.mod
+3 -2

go.sum
+22 -0

pkg/api/http_server.go
+1 -1

pkg/middleware/logger.go
+2 -17

pkg/middleware/request_metrics.go
+24 -7

pkg/server/server.go
+1 -1

No files found.
--- a/go.mod
+++ b/go.mod
@@ -62,9 +62,9 @@ require (
 	github.com/opentracing/opentracing-go v1.2.0
 	github.com/patrickmn/go-cache v2.1.0+incompatible
 	github.com/pkg/errors v0.9.1
-	github.com/prometheus/client_golang v1.7.1
+	github.com/prometheus/client_golang v1.8.0
 	github.com/prometheus/client_model v0.2.0
-	github.com/prometheus/common v0.10.0
+	github.com/prometheus/common v0.14.0
 	github.com/rainycape/unidecode v0.0.0-20150907023854-cb7f23ec59be // indirect
 	github.com/robfig/cron v0.0.0-20180505203441-b41be1df6967
 	github.com/robfig/cron/v3 v3.0.0
@@ -77,6 +77,7 @@ require (
 	github.com/uber/jaeger-client-go v2.25.0+incompatible
 	github.com/unknwon/com v1.0.1
 	github.com/urfave/cli/v2 v2.1.1
+	github.com/weaveworks/common v0.0.0-20201119133501-0619918236ec
 	github.com/xorcare/pointer v1.1.0
 	github.com/yudai/gojsondiff v1.0.0
 	golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a

--- a/go.sum
+++ b/go.sum
--- a/pkg/api/http_server.go
+++ b/pkg/api/http_server.go
@@ -367,7 +367,7 @@ func (hs *HTTPServer) metricsEndpoint(ctx *macaron.Context) {
 	}
 	promhttp.
-		HandlerFor(prometheus.DefaultGatherer, promhttp.HandlerOpts{}).
+		HandlerFor(prometheus.DefaultGatherer, promhttp.HandlerOpts{EnableOpenMetrics: true}).
 		ServeHTTP(ctx.Resp, ctx.Req.Request)
 }

--- a/pkg/middleware/logger.go
+++ b/pkg/middleware/logger.go
@@ -16,15 +16,13 @@
 package middleware
 import (
-	"context"
 	"net/http"
 	"time"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/setting"
-	opentracing "github.com/opentracing/opentracing-go"
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/uber/jaeger-client-go"
+	cw "github.com/weaveworks/common/middleware"
 	"gopkg.in/macaron.v1"
 )
@@ -63,7 +61,7 @@ func Logger() macaron.Handler {
 				"referer", req.Referer(),
 			}
-			traceID, exist := extractTraceID(ctxTyped.Req.Request.Context())
+			traceID, exist := cw.ExtractTraceID(ctxTyped.Req.Request.Context())
 			if exist {
 				logParams = append(logParams, "traceID", traceID)
 			}
@@ -76,16 +74,3 @@ func Logger() macaron.Handler {
 		}
 	}
 }
-func extractTraceID(ctx context.Context) (string, bool) {
-	sp := opentracing.SpanFromContext(ctx)
-	if sp == nil {
-		return "", false
-	}
-	sctx, ok := sp.Context().(jaeger.SpanContext)
-	if !ok {
-		return "", false
-	}
-	return sctx.TraceID().String(), true
-}
--- a/pkg/middleware/request_metrics.go
+++ b/pkg/middleware/request_metrics.go
@@ -9,12 +9,17 @@ import (
 	"github.com/grafana/grafana/pkg/infra/metrics"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/prometheus/client_golang/prometheus"
+	cw "github.com/weaveworks/common/middleware"
 	"gopkg.in/macaron.v1"
 )
 var (
 	httpRequestsInFlight         prometheus.Gauge
 	httpRequestDurationHistogram *prometheus.HistogramVec
+	// DefBuckets are histogram buckets for the response time (in seconds)
+	// of a network service, including one that is responding very slowly.
+	defBuckets = []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5}
 )
 func init() {
@@ -30,9 +35,9 @@ func init() {
 			Namespace: "grafana",
 			Name:      "http_request_duration_seconds",
 			Help:      "Histogram of latencies for HTTP requests.",
-			Buckets:   []float64{.1, .2, .4, 1, 3, 8, 20, 60, 120},
+			Buckets:   defBuckets,
 		},
-		[]string{"handler"},
+		[]string{"handler", "code", "method"},
 	)
 	prometheus.MustRegister(httpRequestsInFlight, httpRequestDurationHistogram)
@@ -52,14 +57,26 @@ func RequestMetrics(cfg *setting.Cfg) func(handler string) macaron.Handler {
 			code := sanitizeCode(status)
 			method := sanitizeMethod(req.Method)
-			metrics.MHttpRequestTotal.WithLabelValues(handler, code, method).Inc()
-			duration := time.Since(now).Nanoseconds() / int64(time.Millisecond)
-			// enable histogram and disable summaries for http requests.
+			// enable histogram and disable summaries + counters for http requests.
 			if cfg.IsHTTPRequestHistogramEnabled() {
-				httpRequestDurationHistogram.WithLabelValues(handler).Observe(float64(duration))
+				// avoiding the sanitize functions for in the new instrumentation
+				// since they dont make much sense. We should remove them later.
+				histogram := httpRequestDurationHistogram.
+					WithLabelValues(handler, strconv.Itoa(rw.Status()), req.Method)
+				if traceID, ok := cw.ExtractSampledTraceID(c.Req.Context()); ok {
+					// Need to type-convert the Observer to an
+					// ExemplarObserver. This will always work for a
+					// HistogramVec.
+					histogram.(prometheus.ExemplarObserver).ObserveWithExemplar(
+						time.Since(now).Seconds(), prometheus.Labels{"traceID": traceID},
+					)
+					return
+				}
+				histogram.Observe(time.Since(now).Seconds())
 			} else {
+				duration := time.Since(now).Nanoseconds() / int64(time.Millisecond)
+				metrics.MHttpRequestTotal.WithLabelValues(handler, code, method).Inc()
 				metrics.MHttpRequestSummary.WithLabelValues(handler, code, method).Observe(float64(duration))
 			}

--- a/pkg/server/server.go
+++ b/pkg/server/server.go
@@ -273,7 +273,7 @@ func (s *Server) buildServiceGraph(services []*registry.Descriptor) error {
 	objs := []interface{}{
 		bus.GetBus(),
 		s.cfg,
-		routing.NewRouteRegister(middleware.RequestMetrics(s.cfg), middleware.RequestTracing),
+		routing.NewRouteRegister(middleware.RequestTracing, middleware.RequestMetrics(s.cfg)),
 		localcache.New(5*time.Minute, 10*time.Minute),
 		s,
 	}