-
Notifications
You must be signed in to change notification settings - Fork 65
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
Data source HTTP metrics #1130
Changes from all commits
65b0f43
26666de
8dd27ed
0c5c47d
2efbb13
b6c52a5
777e9c0
c94e9b4
6eb4630
ae33eca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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}, | ||
}, []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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to verify this works as expected for an external datasource plugin. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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 | ||
}) | ||
} |
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) | ||
}) | ||
} | ||
}) | ||
} |
There was a problem hiding this comment.
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
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look like a native histogram response :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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