Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data source HTTP metrics #1130

Merged
merged 10 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions backend/endpoint.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package backend

import "context"
import (
"context"

endpointctx "github.com/grafana/grafana-plugin-sdk-go/backend/internal/endpointctx"
)

// Endpoint used for defining names for endpoints/handlers.
type Endpoint string
Expand All @@ -14,18 +18,14 @@ func (e Endpoint) String() string {
return string(e)
}

type endpointCtxKeyType struct{}

var endpointCtxKey = endpointCtxKeyType{}

// WithEndpoint adds endpoint to ctx.
func WithEndpoint(ctx context.Context, endpoint Endpoint) context.Context {
return context.WithValue(ctx, endpointCtxKey, endpoint)
return context.WithValue(ctx, endpointctx.EndpointCtxKey, endpoint)
}

// EndpointFromContext extracts [Endpoint] from ctx if available, otherwise empty [Endpoint].
func EndpointFromContext(ctx context.Context) Endpoint {
if ep := ctx.Value(endpointCtxKey); ep != nil {
if ep := ctx.Value(endpointctx.EndpointCtxKey); ep != nil {
return ep.(Endpoint)
}

Expand Down
162 changes: 162 additions & 0 deletions backend/httpclient/datasource_metrics_middleware.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
package httpclient

import (
"errors"
"fmt"
"net/http"
"strconv"
"strings"
"time"

endpointctx "github.com/grafana/grafana-plugin-sdk-go/backend/internal/endpointctx"
"github.com/grafana/grafana-plugin-sdk-go/backend/log"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/client_golang/prometheus/promhttp"
)

const kb = 1024
const mb = kb * kb
const gb = mb * kb

var (
datasourceRequestCounter = promauto.NewCounterVec(
prometheus.CounterOpts{
Namespace: "plugins",
Name: "datasource_request_total",
Help: "A counter for outgoing requests for an external data source",
},
[]string{"datasource", "datasource_type", "code", "method", "secure_socks_ds_proxy_enabled", "endpoint"},
)

datasourceRequestHistogram = promauto.NewHistogramVec(
prometheus.HistogramOpts{
Namespace: "plugins",
Name: "datasource_request_duration_seconds",
Help: "histogram of durations of outgoing external data source requests sent from Grafana",
Buckets: []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10, 25, 50, 100},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to consider native histogram here as well if it works for datasource_response_size_bytes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
This doesn't look like a native histogram response :/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about

histogram_quantile(0.99, sum(rate(plugins_datasource_response_size_bytes{datasource_type="$plugin"}[$__rate_interval])) by (le))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope 😞 definitely shows native histograms aren't supported atm

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created an issue to investigate this further #1132

FYI @wbrowne

}, []string{"datasource", "datasource_type", "code", "method", "secure_socks_ds_proxy_enabled", "endpoint"},
)

datasourceResponseHistogram = promauto.NewHistogramVec(
prometheus.HistogramOpts{
Namespace: "plugins",
Name: "datasource_response_size_bytes",
Help: "histogram of external data source response sizes returned to Grafana",
Buckets: []float64{128, 256, 512, 1 * kb, 2 * kb, 4 * kb, 8 * kb, 16 * kb, 32 * kb, 64 * kb, 128 * kb, 256 * kb, 512 * kb, 1 * mb,
2 * mb, 4 * mb, 8 * mb, 16 * mb, 32 * mb, 64 * mb, 128 * mb, 256 * mb, 512 * mb, 1 * gb,
2 * gb, 4 * gb, 8 * gb},
NativeHistogramBucketFactor: 1.1,
NativeHistogramMaxBucketNumber: 100,
NativeHistogramMinResetDuration: time.Hour,
Comment on lines +49 to +51
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to verify this works as expected for an external datasource plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this doesn't look like it works atm, maybe best to remove?

}, []string{"datasource", "datasource_type", "secure_socks_ds_proxy_enabled", "endpoint"},
)

datasourceRequestsInFlight = promauto.NewGaugeVec(
prometheus.GaugeOpts{
Namespace: "plugins",
Name: "datasource_request_in_flight",
Help: "A gauge of outgoing external data source requests currently being sent by Grafana",
},
[]string{"datasource", "datasource_type", "secure_socks_ds_proxy_enabled", "endpoint"},
)
)

// sanitizeLabelName removes all invalid chars from the label name.
// If the label name is empty or contains only invalid chars, it
// will return an error.
func sanitizeLabelName(name string) (string, error) {
if len(name) == 0 {
return "", errors.New("label name cannot be empty")
}

out := strings.Builder{}
for i, b := range name {
if (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || (b >= '0' && b <= '9' && i > 0) {
out.WriteRune(b)
} else if b == ' ' {
out.WriteRune('_')
}
}

if out.Len() == 0 {
return "", fmt.Errorf("label name only contains invalid chars: %q", name)
}

return out.String(), nil
}

const DataSourceMetricsMiddlewareName = "datasource_metrics"

var executeMiddlewareFunc = executeMiddleware
aangelisc marked this conversation as resolved.
Show resolved Hide resolved

func DataSourceMetricsMiddleware() Middleware {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if this could be re-used by Grafana itself here https://github.com/grafana/grafana/blob/f062c66f89b09fca7716438ac488a76bf81ed7f8/pkg/infra/httpclient/httpclientprovider/http_client_provider.go#L28, perhaps a follow up. But then it would require an option to configure the namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed although I'm not sure how to change the namespace without m moving the creation of the metrics into a function which then causes duplicate register errors.

return NamedMiddlewareFunc(DataSourceMetricsMiddlewareName, func(opts Options, next http.RoundTripper) http.RoundTripper {
if opts.Labels == nil {
return next
}

datasourceName, exists := opts.Labels["datasource_name"]
if !exists {
return next
}

datasourceLabelName, err := sanitizeLabelName(datasourceName)
// if the datasource named cannot be turned into a prometheus
// label we will skip instrumenting these metrics.
if err != nil {
log.DefaultLogger.Error("failed to sanitize datasource name", "error", err)
return next
aangelisc marked this conversation as resolved.
Show resolved Hide resolved
}

datasourceType, exists := opts.Labels["datasource_type"]
if !exists {
return next
}
datasourceLabelType, err := sanitizeLabelName(datasourceType)
// if the datasource type cannot be turned into a prometheus
// label we will skip instrumenting these metrics.
if err != nil {
log.DefaultLogger.Error("failed to sanitize datasource type", "error", err)
return next
}

labels := prometheus.Labels{
"datasource": datasourceLabelName,
"datasource_type": datasourceLabelType,
"secure_socks_ds_proxy_enabled": strconv.FormatBool(opts.ProxyOptions != nil && opts.ProxyOptions.Enabled),
}

return executeMiddlewareFunc(next, labels)
})
}

func executeMiddleware(next http.RoundTripper, labels prometheus.Labels) http.RoundTripper {
return RoundTripperFunc(func(r *http.Request) (*http.Response, error) {
ctx := r.Context()
labels["endpoint"] = ""
if ep := ctx.Value(endpointctx.EndpointCtxKey); ep != nil {
labels["endpoint"] = ep.(string)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this is not working as expected, sorry. I'm seeing some panics in my local instance:

logger=plugin.example-httpbackend-datasource t=2024-11-06T12:46:05.012205+01:00 level=error msg="panic triggered" error="interface conversion: interface {} is backend.Endpoint, not string"

Can you verify this?

}
requestCounter := datasourceRequestCounter.MustCurryWith(labels)
requestHistogram := datasourceRequestHistogram.MustCurryWith(labels)
requestInFlight := datasourceRequestsInFlight.With(labels)
responseSizeHistogram := datasourceResponseHistogram.With(labels)

res, err := promhttp.InstrumentRoundTripperDuration(requestHistogram,
promhttp.InstrumentRoundTripperCounter(requestCounter,
promhttp.InstrumentRoundTripperInFlight(requestInFlight, next))).
RoundTrip(r)
if err != nil {
return nil, err
}

if res != nil && res.StatusCode != http.StatusSwitchingProtocols {
aangelisc marked this conversation as resolved.
Show resolved Hide resolved
res.Body = CountBytesReader(res.Body, func(bytesRead int64) {
responseSizeHistogram.Observe(float64(bytesRead))
})
}

return res, nil
})
}
158 changes: 158 additions & 0 deletions backend/httpclient/datasource_metrics_middleware_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
package httpclient

import (
"net/http"
"testing"

"github.com/grafana/grafana-plugin-sdk-go/backend/proxy"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/require"
)

func TestDataSourceMetricsMiddleware(t *testing.T) {
t.Run("Without label options set should return next http.RoundTripper", func(t *testing.T) {
origExecuteMiddlewareFunc := executeMiddlewareFunc
executeMiddlewareCalled := false
middlewareCalled := false
executeMiddlewareFunc = func(next http.RoundTripper, _ prometheus.Labels) http.RoundTripper {
executeMiddlewareCalled = true
return RoundTripperFunc(func(r *http.Request) (*http.Response, error) {
middlewareCalled = true
return next.RoundTrip(r)
})
}
t.Cleanup(func() {
executeMiddlewareFunc = origExecuteMiddlewareFunc
})

ctx := &testContext{}
finalRoundTripper := ctx.createRoundTripper("finalrt")
mw := DataSourceMetricsMiddleware()
rt := mw.CreateMiddleware(Options{}, finalRoundTripper)
require.NotNil(t, rt)
middlewareName, ok := mw.(MiddlewareName)
require.True(t, ok)
require.Equal(t, DataSourceMetricsMiddlewareName, middlewareName.MiddlewareName())

req, err := http.NewRequest(http.MethodGet, "http://", nil)
require.NoError(t, err)
res, err := rt.RoundTrip(req)
require.NoError(t, err)
require.NotNil(t, res)
if res.Body != nil {
require.NoError(t, res.Body.Close())
}
require.Len(t, ctx.callChain, 1)
require.ElementsMatch(t, []string{"finalrt"}, ctx.callChain)
require.False(t, executeMiddlewareCalled)
require.False(t, middlewareCalled)
})

t.Run("Without data source name label options set should return next http.RoundTripper", func(t *testing.T) {
origExecuteMiddlewareFunc := executeMiddlewareFunc
executeMiddlewareCalled := false
middlewareCalled := false
executeMiddlewareFunc = func(next http.RoundTripper, _ prometheus.Labels) http.RoundTripper {
executeMiddlewareCalled = true
return RoundTripperFunc(func(r *http.Request) (*http.Response, error) {
middlewareCalled = true
return next.RoundTrip(r)
})
}
t.Cleanup(func() {
executeMiddlewareFunc = origExecuteMiddlewareFunc
})

ctx := &testContext{}
finalRoundTripper := ctx.createRoundTripper("finalrt")
mw := DataSourceMetricsMiddleware()
rt := mw.CreateMiddleware(Options{Labels: map[string]string{"test": "test"}}, finalRoundTripper)
require.NotNil(t, rt)
middlewareName, ok := mw.(MiddlewareName)
require.True(t, ok)
require.Equal(t, DataSourceMetricsMiddlewareName, middlewareName.MiddlewareName())

req, err := http.NewRequest(http.MethodGet, "http://", nil)
require.NoError(t, err)
res, err := rt.RoundTrip(req)
require.NoError(t, err)
require.NotNil(t, res)
if res.Body != nil {
require.NoError(t, res.Body.Close())
}
require.Len(t, ctx.callChain, 1)
require.ElementsMatch(t, []string{"finalrt"}, ctx.callChain)
require.False(t, executeMiddlewareCalled)
require.False(t, middlewareCalled)
})

t.Run("With datasource name label options set should execute middleware", func(t *testing.T) {
origExecuteMiddlewareFunc := executeMiddlewareFunc
executeMiddlewareCalled := false
labels := prometheus.Labels{}
middlewareCalled := false
executeMiddlewareFunc = func(next http.RoundTripper, datasourceLabel prometheus.Labels) http.RoundTripper {
executeMiddlewareCalled = true
labels = datasourceLabel
return RoundTripperFunc(func(r *http.Request) (*http.Response, error) {
middlewareCalled = true
return next.RoundTrip(r)
})
}
t.Cleanup(func() {
executeMiddlewareFunc = origExecuteMiddlewareFunc
})

testCases := []struct {
description string
httpClientOptions Options
expectedSecureSocksDSProxyEnabled string
}{
{
description: "secure socks ds proxy is disabled",
httpClientOptions: Options{
Labels: map[string]string{"datasource_name": "My Data Source 123", "datasource_type": "prometheus"},
},
expectedSecureSocksDSProxyEnabled: "false",
},
{
description: "secure socks ds proxy is enabled",
httpClientOptions: Options{
Labels: map[string]string{"datasource_name": "My Data Source 123", "datasource_type": "prometheus"},
ProxyOptions: &proxy.Options{Enabled: true},
},
expectedSecureSocksDSProxyEnabled: "true",
},
}

for _, tt := range testCases {
t.Run(tt.description, func(t *testing.T) {
ctx := &testContext{}
finalRoundTripper := ctx.createRoundTripper("finalrt")
mw := DataSourceMetricsMiddleware()
rt := mw.CreateMiddleware(tt.httpClientOptions, finalRoundTripper)
require.NotNil(t, rt)
middlewareName, ok := mw.(MiddlewareName)
require.True(t, ok)
require.Equal(t, DataSourceMetricsMiddlewareName, middlewareName.MiddlewareName())

req, err := http.NewRequest(http.MethodGet, "http://", nil)
require.NoError(t, err)
res, err := rt.RoundTrip(req)
require.NoError(t, err)
require.NotNil(t, res)
if res.Body != nil {
require.NoError(t, res.Body.Close())
}
require.Len(t, ctx.callChain, 1)
require.ElementsMatch(t, []string{"finalrt"}, ctx.callChain)
require.True(t, executeMiddlewareCalled)
require.Len(t, labels, 3)
require.Equal(t, "My_Data_Source_123", labels["datasource"])
require.Equal(t, "prometheus", labels["datasource_type"])
require.Equal(t, tt.expectedSecureSocksDSProxyEnabled, labels["secure_socks_ds_proxy_enabled"])
require.True(t, middlewareCalled)
})
}
})
}
1 change: 1 addition & 0 deletions backend/httpclient/http_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ type ConfigureMiddlewareFunc func(opts Options, existingMiddleware []Middleware)
func DefaultMiddlewares() []Middleware {
return []Middleware{
TracingMiddleware(nil),
DataSourceMetricsMiddleware(),
BasicAuthenticationMiddleware(),
CustomHeadersMiddleware(),
ContextualMiddleware(),
Expand Down
11 changes: 6 additions & 5 deletions backend/httpclient/http_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,13 @@ func TestNewClient(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, client)

require.Len(t, usedMiddlewares, 5)
require.Len(t, usedMiddlewares, 6)
require.Equal(t, TracingMiddlewareName, usedMiddlewares[0].(MiddlewareName).MiddlewareName())
require.Equal(t, BasicAuthenticationMiddlewareName, usedMiddlewares[1].(MiddlewareName).MiddlewareName())
require.Equal(t, CustomHeadersMiddlewareName, usedMiddlewares[2].(MiddlewareName).MiddlewareName())
require.Equal(t, ContextualMiddlewareName, usedMiddlewares[3].(MiddlewareName).MiddlewareName())
require.Equal(t, ErrorSourceMiddlewareName, usedMiddlewares[4].(MiddlewareName).MiddlewareName())
require.Equal(t, DataSourceMetricsMiddlewareName, usedMiddlewares[1].(MiddlewareName).MiddlewareName())
require.Equal(t, BasicAuthenticationMiddlewareName, usedMiddlewares[2].(MiddlewareName).MiddlewareName())
require.Equal(t, CustomHeadersMiddlewareName, usedMiddlewares[3].(MiddlewareName).MiddlewareName())
require.Equal(t, ContextualMiddlewareName, usedMiddlewares[4].(MiddlewareName).MiddlewareName())
require.Equal(t, ErrorSourceMiddlewareName, usedMiddlewares[5].(MiddlewareName).MiddlewareName())
})

t.Run("New() with opts middleware should return expected http.Client", func(t *testing.T) {
Expand Down
Loading