Plugins: Improve instrumentation by adding metrics and tracing (#61035)

* WIP: Plugins tracing * Trace ID middleware * Add prometheus metrics and tracing to plugins updater * Add TODOs * Add instrumented http client * Add tracing to grafana update checker * Goimports * Moved plugins tracing to middleware * goimports, fix tests * Removed X-Trace-Id header * Fix comment in NewTracingHeaderMiddleware * Add metrics to instrumented http client * Add instrumented http client options * Removed unused function * Switch to contextual logger * Refactoring, fix tests * Moved InstrumentedHTTPClient and PrometheusMetrics to their own package * Tracing middleware: handle errors * Report span status codes when recording errors * Add tests for tracing middleware * Moved fakeSpan and fakeTracer to pkg/infra/tracing * Add TestHTTPClientTracing * Lint * Changes after PR review * Tests: Made "ended" in FakeSpan private, allow calling End only once * Testing: panic in FakeSpan if span already ended * Refactoring: Simplify Grafana updater checks * Refactoring: Simplify plugins updater error checks and logs * Fix wrong call to checkForUpdates -> instrumentedCheckForUpdates * Tests: Fix wrong call to checkForUpdates -> instrumentedCheckForUpdates * Log update checks duration, use Info log level for check succeeded logs * Add plugin context span attributes in tracing_middleware * Refactor prometheus metrics as httpclient middleware * Fix call to ProvidePluginsService in plugins_test.go * Propagate context to update checker outgoing http requests * Plugin client tracing middleware: Removed operation name in status * Fix tests * Goimports tracing_middleware.go * Goimports * Fix imports * Changed span name to plugins client middleware * Add span name assertion in TestTracingMiddleware * Removed Prometheus metrics middleware from grafana and plugins updatechecker * Add span attributes for ds name, type, uid, panel and dashboard ids * Fix http header reading in tracing middlewares * Use contexthandler.FromContext, add X-Query-Group-Id * Add test for RunStream * Fix imports * Changes from PR review * TestTracingMiddleware: Changed assert to require for didPanic assertion * Lint * Fix imports
2023-03-28 11:01:06 +02:00
parent 0beb768427
commit a89202eab2
16 changed files with 1028 additions and 72 deletions
@@ -0,0 +1,88 @@
+package httpclientprovider
+
+import (
+	"net/http"
+	"time"
+
+	"github.com/grafana/grafana-plugin-sdk-go/backend/httpclient"
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+// PrometheusMetrics groups some metrics for a PrometheusMetricsMiddleware
+type PrometheusMetrics struct {
+	requestsCounter          prometheus.Counter
+	failureCounter           prometheus.Counter
+	durationSecondsHistogram prometheus.Histogram
+	inFlightGauge            prometheus.Gauge
+}
+
+// NewPrometheusMetricsMiddleware returns a new *PrometheusMetrics with pre-filled metrics, with the specified prefix
+func NewPrometheusMetricsMiddleware(prefix string) *PrometheusMetrics {
+	return &PrometheusMetrics{
+		requestsCounter: prometheus.NewCounter(prometheus.CounterOpts{
+			Name: prefix + "_request_total",
+		}),
+		failureCounter: prometheus.NewCounter(prometheus.CounterOpts{
+			Name: prefix + "_failure_total",
+		}),
+		durationSecondsHistogram: prometheus.NewHistogram(prometheus.HistogramOpts{
+			Name: prefix + "_request_duration_seconds",
+		}),
+		inFlightGauge: prometheus.NewGauge(prometheus.GaugeOpts{
+			Name: prefix + "_in_flight_request",
+		}),
+	}
+}
+
+// Register registers the metrics in the current PrometheusMetrics into the provided registry
+func (m *PrometheusMetrics) Register(registry prometheus.Registerer) error {
+	for _, collector := range []prometheus.Collector{
+		m.requestsCounter, m.failureCounter, m.durationSecondsHistogram, m.inFlightGauge,
+	} {
+		if err := registry.Register(collector); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// MustRegister is like Register, but, in case of failure, it panics instead of returning an error
+func (m *PrometheusMetrics) MustRegister(registry prometheus.Registerer) {
+	if err := m.Register(registry); err != nil {
+		panic(err)
+	}
+}
+
+// WithMustRegister calls MustRegister and returns itself. This is to allow to chain the method call
+// upon initialization, useful when declaring metrics in the global scope:
+//
+//	var svcMetrics = NewPrometheusMetricsMiddleware("my_client").WithMustRegister(prometheus.DefaultRegisterer)
+func (m *PrometheusMetrics) WithMustRegister(registry prometheus.Registerer) *PrometheusMetrics {
+	m.MustRegister(registry)
+	return m
+}
+
+// PrometheusMetricsMiddleware is a middleware that will mutate the in flight, requests, duration and
+// failure count on the provided *PrometheusMetrics instance. This can be used to count the number of requests,
+// successful requests and errors that go through the httpclient, as well as to track the response times.
+// For the metrics to be exposed properly, the provided *PrometheusMetrics should already be registered in a Prometheus
+// registry.
+func PrometheusMetricsMiddleware(metrics *PrometheusMetrics) httpclient.Middleware {
+	return httpclient.MiddlewareFunc(func(opts httpclient.Options, next http.RoundTripper) http.RoundTripper {
+		return httpclient.RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
+			startTime := time.Now()
+			metrics.inFlightGauge.Inc()
+
+			res, err := next.RoundTrip(req)
+
+			metrics.inFlightGauge.Dec()
+			metrics.requestsCounter.Inc()
+			metrics.durationSecondsHistogram.Observe(time.Since(startTime).Seconds())
+			if err != nil || (res != nil && !(res.StatusCode >= 200 && res.StatusCode <= 299)) {
+				metrics.failureCounter.Inc()
+			}
+
+			return res, err
+		})
+	})
+}
@@ -0,0 +1,94 @@
+package httpclientprovider
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/grafana/grafana-plugin-sdk-go/backend/httpclient"
+	"github.com/prometheus/client_golang/prometheus/testutil"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPrometheusMetricsMiddleware(t *testing.T) {
+	noOpHandlerFunc := func(writer http.ResponseWriter, request *http.Request) {}
+
+	for _, tc := range []struct {
+		name    string
+		handler http.HandlerFunc
+		assert  func(t *testing.T, metrics *PrometheusMetrics)
+	}{
+		{
+			name: "successful",
+			assert: func(t *testing.T, metrics *PrometheusMetrics) {
+				require.Equal(t, float64(0), testutil.ToFloat64(metrics.inFlightGauge))
+				require.Equal(t, float64(1), testutil.ToFloat64(metrics.requestsCounter))
+				require.Equal(t, float64(0), testutil.ToFloat64(metrics.failureCounter))
+			},
+		},
+		{
+			name: "failure",
+			handler: func(writer http.ResponseWriter, request *http.Request) {
+				writer.WriteHeader(http.StatusInternalServerError)
+			},
+			assert: func(t *testing.T, metrics *PrometheusMetrics) {
+				require.Equal(t, float64(0), testutil.ToFloat64(metrics.inFlightGauge))
+				require.Equal(t, float64(1), testutil.ToFloat64(metrics.requestsCounter))
+				require.Equal(t, float64(1), testutil.ToFloat64(metrics.failureCounter))
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			// Create metrics and make sure they are 0
+			metrics := NewPrometheusMetricsMiddleware("test")
+			require.Equal(t, float64(0), testutil.ToFloat64(metrics.inFlightGauge))
+			require.Equal(t, float64(0), testutil.ToFloat64(metrics.requestsCounter))
+			require.Equal(t, float64(0), testutil.ToFloat64(metrics.failureCounter))
+
+			// Set up test server
+			// Default to noOpHandlerFunc if it's not provided in test case
+			h := tc.handler
+			if h == nil {
+				h = noOpHandlerFunc
+			}
+			srv := httptest.NewServer(h)
+			t.Cleanup(srv.Close)
+
+			// Make request with the prometheus handling middleware
+			cl, err := httpclient.New(httpclient.Options{
+				Middlewares: []httpclient.Middleware{PrometheusMetricsMiddleware(metrics)},
+			})
+			require.NoError(t, err)
+
+			resp, err := cl.Get(srv.URL)
+			defer func() { _ = resp.Body.Close() }()
+			require.NoError(t, err)
+			require.NotNil(t, resp)
+
+			// Run test-case-specific assertions
+			tc.assert(t, metrics)
+		})
+	}
+
+	t.Run("in flight", func(t *testing.T) {
+		metrics := NewPrometheusMetricsMiddleware("test")
+		require.Equal(t, float64(0), testutil.ToFloat64(metrics.inFlightGauge))
+
+		srv := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+			// Assert in-flight requests
+			require.Equal(t, float64(1), testutil.ToFloat64(metrics.inFlightGauge), "in flight should increase during request")
+		}))
+		t.Cleanup(srv.Close)
+
+		cl, err := httpclient.New(httpclient.Options{
+			Middlewares: []httpclient.Middleware{PrometheusMetricsMiddleware(metrics)},
+		})
+		require.NoError(t, err)
+
+		resp, err := cl.Get(srv.URL)
+		defer func() { _ = resp.Body.Close() }()
+		require.NoError(t, err)
+		require.NotNil(t, resp)
+		require.Equal(t, float64(0), testutil.ToFloat64(metrics.inFlightGauge), "in flight should decrease after response")
+	})
+}
@@ -7,12 +7,13 @@ import (
 	"strconv"

 	"github.com/grafana/grafana-plugin-sdk-go/backend/httpclient"
-	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/infra/tracing"
 	"go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/trace"
+
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/infra/tracing"
 )

 const (