public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Plugins: Add status_source label to plugin request metrics (#76236)

Browse Source 
* Plugins: Chore: Renamed instrumentation middleware to metrics middleware

* Removed repeated logger attributes in middleware and contextual logger

* renamed loggerParams to logParams

* PR review suggestion

* Add pluginsInstrumentationStatusSource feature toggle

* Plugin error source prometheus metrics

* Add error_source to logs

* re-generate feature toggles

* fix compilation issues

* remove unwanted changes

* Removed logger middleware changes, implement error source using context

* Renamed pluginmeta to pluginrequestmeta, changed some method names

* Fix comment

* pluginrequestmeta.go -> plugin_request_meta.go

* Replaced plugin request meta with status source

* Add tests for pluginrequestmeta status source

* Fix potential nil pointer dereference in instrmentation middleware

* Add metrics middleware tests

* Sort imports in clienttest.go

* Add StatusSourceFromContext test

* Add error_source label to plugin_request_duration_seconds

* Re-generate feature flags

* lint

* Use StatusSourcePlugin by default

* re-generate feature flags
This commit is contained in:
Giuseppe Guerra
2023-10-17 10:27:45 +02:00
committed by GitHub
parent 8ebbe06377
commit f5076d1868
12 changed files with 363 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/pluginsintegration/clientmiddleware/metrics_middleware.go
+57 -10
View File
@@ -3,6 +3,7 @@ package clientmiddleware
import (
"context"
"errors"
"fmt"
"time"
"github.com/grafana/grafana-plugin-sdk-go/backend"
@@ -11,6 +12,8 @@ import (
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/manager/registry"
"github.com/grafana/grafana/pkg/plugins/pluginrequestmeta"
"github.com/grafana/grafana/pkg/services/featuremgmt"
)
// pluginMetrics contains the prometheus metrics used by the MetricsMiddleware.
@@ -26,21 +29,26 @@ type pluginMetrics struct {
type MetricsMiddleware struct {
pluginMetrics
pluginRegistry registry.Service
features featuremgmt.FeatureToggles
next plugins.Client
}
func newMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry registry.Service) *MetricsMiddleware {
func newMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry registry.Service, features featuremgmt.FeatureToggles) *MetricsMiddleware {
var additionalLabels []string
if features.IsEnabled(featuremgmt.FlagPluginsInstrumentationStatusSource) {
additionalLabels = []string{"status_source"}
}
pluginRequestCounter := prometheus.NewCounterVec(prometheus.CounterOpts{
Namespace: "grafana",
Name: "plugin_request_total",
Help: "The total amount of plugin requests",
}, []string{"plugin_id", "endpoint", "status", "target"})
}, append([]string{"plugin_id", "endpoint", "status", "target"}, additionalLabels...))
pluginRequestDuration := prometheus.NewHistogramVec(prometheus.HistogramOpts{
Namespace: "grafana",
Name: "plugin_request_duration_milliseconds",
Help: "Plugin request duration",
Buckets: []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10, 25, 50, 100},
}, []string{"plugin_id", "endpoint", "target"})
}, append([]string{"plugin_id", "endpoint", "target"}, additionalLabels...))
pluginRequestSize := prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Namespace: "grafana",
@@ -54,7 +62,7 @@ func newMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry r
Name: "plugin_request_duration_seconds",
Help: "Plugin request duration in seconds",
Buckets: []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10, 25},
}, []string{"source", "plugin_id", "endpoint", "status", "target"})
}, append([]string{"source", "plugin_id", "endpoint", "status", "target"}, additionalLabels...))
promRegisterer.MustRegister(
pluginRequestCounter,
pluginRequestDuration,
@@ -69,12 +77,13 @@ func newMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry r
pluginRequestDurationSeconds: pluginRequestDurationSeconds,
},
pluginRegistry: pluginRegistry,
features: features,
}
}
// NewMetricsMiddleware returns a new MetricsMiddleware.
func NewMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry registry.Service) plugins.ClientMiddleware {
imw := newMetricsMiddleware(promRegisterer, pluginRegistry)
func NewMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry registry.Service, features featuremgmt.FeatureToggles) plugins.ClientMiddleware {
imw := newMetricsMiddleware(promRegisterer, pluginRegistry, features)
return plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {
imw.next = next
return imw
@@ -117,12 +126,21 @@ func (m *MetricsMiddleware) instrumentPluginRequest(ctx context.Context, pluginC
status = statusCancelled
}
}
elapsed := time.Since(start)
pluginRequestDurationWithLabels := m.pluginRequestDuration.WithLabelValues(pluginCtx.PluginID, endpoint, target)
pluginRequestCounterWithLabels := m.pluginRequestCounter.WithLabelValues(pluginCtx.PluginID, endpoint, status, target)
pluginRequestDurationSecondsWithLabels := m.pluginRequestDurationSeconds.WithLabelValues("grafana-backend", pluginCtx.PluginID, endpoint, status, target)
pluginRequestDurationLabels := []string{pluginCtx.PluginID, endpoint, target}
pluginRequestCounterLabels := []string{pluginCtx.PluginID, endpoint, status, target}
pluginRequestDurationSecondsLabels := []string{"grafana-backend", pluginCtx.PluginID, endpoint, status, target}
if m.features.IsEnabled(featuremgmt.FlagPluginsInstrumentationStatusSource) {
statusSource := pluginrequestmeta.StatusSourceFromContext(ctx)
pluginRequestDurationLabels = append(pluginRequestDurationLabels, string(statusSource))
pluginRequestCounterLabels = append(pluginRequestCounterLabels, string(statusSource))
pluginRequestDurationSecondsLabels = append(pluginRequestDurationSecondsLabels, string(statusSource))
}
pluginRequestDurationWithLabels := m.pluginRequestDuration.WithLabelValues(pluginRequestDurationLabels...)
pluginRequestCounterWithLabels := m.pluginRequestCounter.WithLabelValues(pluginRequestCounterLabels...)
pluginRequestDurationSecondsWithLabels := m.pluginRequestDurationSeconds.WithLabelValues(pluginRequestDurationSecondsLabels...)
if traceID := tracing.TraceIDFromContext(ctx, true); traceID != "" {
pluginRequestDurationWithLabels.(prometheus.ExemplarObserver).ObserveWithExemplar(
@@ -142,6 +160,9 @@ func (m *MetricsMiddleware) instrumentPluginRequest(ctx context.Context, pluginC
}
func (m *MetricsMiddleware) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
// Setup plugin request status source
ctx = pluginrequestmeta.WithStatusSource(ctx, pluginrequestmeta.StatusSourcePlugin)
var requestSize float64
for _, v := range req.Queries {
requestSize += float64(len(v.JSON))
@@ -152,6 +173,32 @@ func (m *MetricsMiddleware) QueryData(ctx context.Context, req *backend.QueryDat
var resp *backend.QueryDataResponse
err := m.instrumentPluginRequest(ctx, req.PluginContext, endpointQueryData, func(ctx context.Context) (innerErr error) {
resp, innerErr = m.next.QueryData(ctx, req)
if resp == nil || resp.Responses == nil || !m.features.IsEnabled(featuremgmt.FlagPluginsInstrumentationStatusSource) {
return innerErr
}
// Set downstream status source in the context if there's at least one response with downstream status source,
// and if there's no plugin error
var hasPluginError bool
var hasDownstreamError bool
for _, r := range resp.Responses {
if r.Error == nil {
continue
}
if r.ErrorSource == backend.ErrorSourceDownstream {
hasDownstreamError = true
} else {
hasPluginError = true
}
}
// A plugin error has higher priority than a downstream error,
// so set to downstream only if there's no plugin error
if hasDownstreamError && !hasPluginError {
if err := pluginrequestmeta.WithDownstreamStatusSource(ctx); err != nil {
return fmt.Errorf("failed to set downstream status source: %w", err)
}
}
return innerErr
})
return resp, err
pkg/services/pluginsintegration/clientmiddleware/metrics_middleware_test.go
+194 -11
View File
@@ -16,20 +16,21 @@ import (
"github.com/grafana/grafana/pkg/plugins/backendplugin"
"github.com/grafana/grafana/pkg/plugins/manager/client/clienttest"
"github.com/grafana/grafana/pkg/plugins/manager/fakes"
"github.com/grafana/grafana/pkg/plugins/pluginrequestmeta"
"github.com/grafana/grafana/pkg/services/featuremgmt"
)
const (
pluginID = "plugin-id"
metricRequestTotal = "grafana_plugin_request_total"
metricRequestDurationMs = "grafana_plugin_request_duration_milliseconds"
metricRequestDurationS = "grafana_plugin_request_duration_seconds"
metricRequestSize = "grafana_plugin_request_size_bytes"
)
func TestInstrumentationMiddleware(t *testing.T) {
const (
pluginID = "plugin-id"
metricRequestTotal = "grafana_plugin_request_total"
metricRequestDurationMs = "grafana_plugin_request_duration_milliseconds"
metricRequestDurationS = "grafana_plugin_request_duration_seconds"
metricRequestSize = "grafana_plugin_request_size_bytes"
)
pCtx := backend.PluginContext{PluginID: pluginID}
t.Run("should instrument requests", func(t *testing.T) {
for _, tc := range []struct {
expEndpoint string
@@ -75,7 +76,7 @@ func TestInstrumentationMiddleware(t *testing.T) {
JSONData: plugins.JSONData{ID: pluginID, Backend: true},
}))
mw := newMetricsMiddleware(promRegistry, pluginsRegistry)
mw := newMetricsMiddleware(promRegistry, pluginsRegistry, featuremgmt.WithFeatures())
cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(
plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {
mw.next = next
@@ -112,6 +113,173 @@ func TestInstrumentationMiddleware(t *testing.T) {
})
}
func TestInstrumentationMiddlewareStatusSource(t *testing.T) {
const labelStatusSource = "status_source"
queryDataCounterLabels := prometheus.Labels{
"plugin_id": pluginID,
"endpoint": endpointQueryData,
"status": statusOK,
"target": string(backendplugin.TargetUnknown),
}
downstreamErrorResponse := backend.DataResponse{
Frames: nil,
Error: errors.New("bad gateway"),
Status: 502,
ErrorSource: backend.ErrorSourceDownstream,
}
pluginErrorResponse := backend.DataResponse{
Frames: nil,
Error: errors.New("internal error"),
Status: 500,
ErrorSource: backend.ErrorSourcePlugin,
}
legacyErrorResponse := backend.DataResponse{
Frames: nil,
Error: errors.New("internal error"),
Status: 500,
ErrorSource: "",
}
okResponse := backend.DataResponse{
Frames: nil,
Error: nil,
Status: 200,
ErrorSource: "",
}
pCtx := backend.PluginContext{PluginID: pluginID}
promRegistry := prometheus.NewRegistry()
pluginsRegistry := fakes.NewFakePluginRegistry()
require.NoError(t, pluginsRegistry.Add(context.Background(), &plugins.Plugin{
JSONData: plugins.JSONData{ID: pluginID, Backend: true},
}))
features := featuremgmt.WithFeatures(featuremgmt.FlagPluginsInstrumentationStatusSource)
metricsMw := newMetricsMiddleware(promRegistry, pluginsRegistry, features)
cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(
plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {
metricsMw.next = next
return metricsMw
}),
))
t.Run("Metrics", func(t *testing.T) {
t.Run("Should ignore ErrorSource if feature flag is disabled", func(t *testing.T) {
// Use different middleware without feature flag
metricsMw := newMetricsMiddleware(prometheus.NewRegistry(), pluginsRegistry, featuremgmt.WithFeatures())
cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(
plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {
metricsMw.next = next
return metricsMw
}),
))
cdt.TestClient.QueryDataFunc = func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
return &backend.QueryDataResponse{Responses: map[string]backend.DataResponse{"A": downstreamErrorResponse}}, nil
}
_, err := cdt.Decorator.QueryData(context.Background(), &backend.QueryDataRequest{PluginContext: pCtx})
require.NoError(t, err)
counter, err := metricsMw.pluginMetrics.pluginRequestCounter.GetMetricWith(newLabels(queryDataCounterLabels, nil))
require.NoError(t, err)
require.Equal(t, 1.0, testutil.ToFloat64(counter))
// error_source should not be defined at all
_, err = metricsMw.pluginMetrics.pluginRequestCounter.GetMetricWith(newLabels(
queryDataCounterLabels,
prometheus.Labels{
labelStatusSource: string(backend.ErrorSourceDownstream),
}),
)
require.Error(t, err)
require.ErrorContains(t, err, "inconsistent label cardinality")
})
t.Run("Should add error_source label if feature flag is enabled", func(t *testing.T) {
metricsMw.pluginMetrics.pluginRequestCounter.Reset()
cdt.TestClient.QueryDataFunc = func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
return &backend.QueryDataResponse{Responses: map[string]backend.DataResponse{"A": downstreamErrorResponse}}, nil
}
_, err := cdt.Decorator.QueryData(context.Background(), &backend.QueryDataRequest{PluginContext: pCtx})
require.NoError(t, err)
counter, err := metricsMw.pluginMetrics.pluginRequestCounter.GetMetricWith(newLabels(
queryDataCounterLabels,
prometheus.Labels{
labelStatusSource: string(backend.ErrorSourceDownstream),
}),
)
require.NoError(t, err)
require.Equal(t, 1.0, testutil.ToFloat64(counter))
})
})
t.Run("Priority", func(t *testing.T) {
for _, tc := range []struct {
name string
responses map[string]backend.DataResponse
expStatusSource pluginrequestmeta.StatusSource
}{
{
"Default status source for ok responses should be plugin",
map[string]backend.DataResponse{"A": okResponse},
pluginrequestmeta.StatusSourcePlugin,
},
{
"Plugin errors should have higher priority than downstream errors",
map[string]backend.DataResponse{
"A": pluginErrorResponse,
"B": downstreamErrorResponse,
},
pluginrequestmeta.StatusSourcePlugin,
},
{
"Errors without ErrorSource should be reported as plugin status source",
map[string]backend.DataResponse{"A": legacyErrorResponse},
pluginrequestmeta.StatusSourcePlugin,
},
{
"Downstream errors should have higher priority than ok responses",
map[string]backend.DataResponse{
"A": okResponse,
"B": downstreamErrorResponse,
},
pluginrequestmeta.StatusSourceDownstream,
},
{
"Plugin errors should have higher priority than ok responses",
map[string]backend.DataResponse{
"A": okResponse,
"B": pluginErrorResponse,
},
pluginrequestmeta.StatusSourcePlugin,
},
{
"Legacy errors should have higher priority than ok responses",
map[string]backend.DataResponse{
"A": okResponse,
"B": legacyErrorResponse,
},
pluginrequestmeta.StatusSourcePlugin,
},
} {
t.Run(tc.name, func(t *testing.T) {
t.Cleanup(func() {
cdt.QueryDataCtx = nil
cdt.QueryDataReq = nil
})
cdt.TestClient.QueryDataFunc = func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
cdt.QueryDataCtx = ctx
cdt.QueryDataReq = req
return &backend.QueryDataResponse{Responses: tc.responses}, nil
}
_, err := cdt.Decorator.QueryData(context.Background(), &backend.QueryDataRequest{PluginContext: pCtx})
require.NoError(t, err)
ctxStatusSource := pluginrequestmeta.StatusSourceFromContext(cdt.QueryDataCtx)
require.Equal(t, tc.expStatusSource, ctxStatusSource)
})
}
})
}
// checkHistogram is a utility function that checks if a histogram with the given name and label values exists
// and has been observed at least once.
func checkHistogram(promRegistry *prometheus.Registry, expMetricName string, expLabels map[string]string) error {
@@ -162,3 +330,18 @@ func checkHistogram(promRegistry *prometheus.Registry, expMetricName string, exp
}
return nil
}
// newLabels creates a new prometheus.Labels from the given initial labels and additional labels.
// The additionalLabels are merged into the initial ones, and will overwrite a value if already set in initialLabels.
func newLabels(initialLabels prometheus.Labels, additional ...prometheus.Labels) prometheus.Labels {
r := make(prometheus.Labels, len(initialLabels)+len(additional)/2)
for k, v := range initialLabels {
r[k] = v
}
for _, l := range additional {
for k, v := range l {
r[k] = v
}
}
return r
}