From 7cf08ebaf91ec334ce399d56a6f344be9659acc6 Mon Sep 17 00:00:00 2001 From: "grafana-delivery-bot[bot]" <132647405+grafana-delivery-bot[bot]@users.noreply.github.com> Date: Wed, 23 Oct 2024 17:02:18 +0200 Subject: [PATCH] [v11.3.x] Prometheus: Fix passing query timeout to upstream queries (#95264) Prometheus: Fix passing query timeout to upstream queries (#95104) * remove queryTimeout from constructor * use queryTimeout for range and instant queries * remove comment * remove default query timeout * fix linting (cherry picked from commit 78a00d09cdefe618078e844b3deab84a93ed754d) Co-authored-by: ismail simsek --- packages/grafana-prometheus/src/datasource.ts | 2 -- pkg/promlib/client/client.go | 17 ++++++++++++----- pkg/promlib/client/client_test.go | 10 +++++----- pkg/promlib/querydata/request.go | 12 ++++++++---- pkg/promlib/resource/resource.go | 5 +++-- 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/packages/grafana-prometheus/src/datasource.ts b/packages/grafana-prometheus/src/datasource.ts index 29b53f51902..6f3d383d07e 100644 --- a/packages/grafana-prometheus/src/datasource.ts +++ b/packages/grafana-prometheus/src/datasource.ts @@ -97,7 +97,6 @@ export class PrometheusDatasource basicAuth: any; withCredentials: boolean; interval: string; - queryTimeout: string | undefined; httpMethod: string; languageProvider: PrometheusLanguageProvider; exemplarTraceIdDestinations: ExemplarTraceIdDestination[] | undefined; @@ -126,7 +125,6 @@ export class PrometheusDatasource this.basicAuth = instanceSettings.basicAuth; this.withCredentials = Boolean(instanceSettings.withCredentials); this.interval = instanceSettings.jsonData.timeInterval || '15s'; - this.queryTimeout = instanceSettings.jsonData.queryTimeout; this.httpMethod = instanceSettings.jsonData.httpMethod || 'GET'; this.exemplarTraceIdDestinations = instanceSettings.jsonData.exemplarTraceIdDestinations; this.hasIncrementalQuery = instanceSettings.jsonData.incrementalQuerying ?? false; diff --git a/pkg/promlib/client/client.go b/pkg/promlib/client/client.go index b23196b6f5b..accdbc00051 100644 --- a/pkg/promlib/client/client.go +++ b/pkg/promlib/client/client.go @@ -24,13 +24,14 @@ type doer interface { // objects, we have to go through them and then serialize again into DataFrame which isn't very efficient. Using custom // client we can parse response directly into DataFrame. type Client struct { - doer doer - method string - baseUrl string + doer doer + method string + baseUrl string + queryTimeout string } -func NewClient(d doer, method, baseUrl string) *Client { - return &Client{doer: d, method: method, baseUrl: baseUrl} +func NewClient(d doer, method, baseUrl, queryTimeout string) *Client { + return &Client{doer: d, method: method, baseUrl: baseUrl, queryTimeout: queryTimeout} } func (c *Client) QueryRange(ctx context.Context, q *models.Query) (*http.Response, error) { @@ -41,6 +42,9 @@ func (c *Client) QueryRange(ctx context.Context, q *models.Query) (*http.Respons "end": formatTime(tr.End), "step": strconv.FormatFloat(tr.Step.Seconds(), 'f', -1, 64), } + if c.queryTimeout != "" { + qv["timeout"] = c.queryTimeout + } req, err := c.createQueryRequest(ctx, "api/v1/query_range", qv) if err != nil { @@ -58,6 +62,9 @@ func (c *Client) QueryInstant(ctx context.Context, q *models.Query) (*http.Respo // Instead of aligning we use time point directly. // https://prometheus.io/docs/prometheus/latest/querying/api/#instant-queries qv := map[string]string{"query": q.Expr, "time": formatTime(q.End)} + if c.queryTimeout != "" { + qv["timeout"] = c.queryTimeout + } req, err := c.createQueryRequest(ctx, "api/v1/query", qv) if err != nil { return nil, err diff --git a/pkg/promlib/client/client_test.go b/pkg/promlib/client/client_test.go index 16dc76fb270..96bbc784408 100644 --- a/pkg/promlib/client/client_test.go +++ b/pkg/promlib/client/client_test.go @@ -30,7 +30,7 @@ func TestClient(t *testing.T) { t.Run("QueryResource", func(t *testing.T) { doer := &MockDoer{} // The method here does not really matter for resource calls - client := NewClient(doer, http.MethodGet, "http://localhost:9090") + client := NewClient(doer, http.MethodGet, "http://localhost:9090", "60s") t.Run("sends correct POST request", func(t *testing.T) { req := &backend.CallResourceRequest{ @@ -86,7 +86,7 @@ func TestClient(t *testing.T) { doer := &MockDoer{} t.Run("sends correct POST query", func(t *testing.T) { - client := NewClient(doer, http.MethodPost, "http://localhost:9090") + client := NewClient(doer, http.MethodPost, "http://localhost:9090", "60s") req := &models.Query{ Expr: "rate(ALERTS{job=\"test\" [$__rate_interval]})", Start: time.Unix(0, 0), @@ -108,12 +108,12 @@ func TestClient(t *testing.T) { require.Equal(t, "application/x-www-form-urlencoded", doer.Req.Header.Get("Content-Type")) body, err := io.ReadAll(doer.Req.Body) require.NoError(t, err) - require.Equal(t, []byte("end=1234&query=rate%28ALERTS%7Bjob%3D%22test%22+%5B%24__rate_interval%5D%7D%29&start=0&step=1"), body) + require.Equal(t, []byte("end=1234&query=rate%28ALERTS%7Bjob%3D%22test%22+%5B%24__rate_interval%5D%7D%29&start=0&step=1&timeout=60s"), body) require.Equal(t, "http://localhost:9090/api/v1/query_range", doer.Req.URL.String()) }) t.Run("sends correct GET query", func(t *testing.T) { - client := NewClient(doer, http.MethodGet, "http://localhost:9090") + client := NewClient(doer, http.MethodGet, "http://localhost:9090", "60s") req := &models.Query{ Expr: "rate(ALERTS{job=\"test\" [$__rate_interval]})", Start: time.Unix(0, 0), @@ -135,7 +135,7 @@ func TestClient(t *testing.T) { body, err := io.ReadAll(doer.Req.Body) require.NoError(t, err) require.Equal(t, []byte{}, body) - require.Equal(t, "http://localhost:9090/api/v1/query_range?end=1234&query=rate%28ALERTS%7Bjob%3D%22test%22+%5B%24__rate_interval%5D%7D%29&start=0&step=1", doer.Req.URL.String()) + require.Equal(t, "http://localhost:9090/api/v1/query_range?end=1234&query=rate%28ALERTS%7Bjob%3D%22test%22+%5B%24__rate_interval%5D%7D%29&start=0&step=1&timeout=60s", doer.Req.URL.String()) }) }) } diff --git a/pkg/promlib/querydata/request.go b/pkg/promlib/querydata/request.go index d28f13903c4..625113a69c9 100644 --- a/pkg/promlib/querydata/request.go +++ b/pkg/promlib/querydata/request.go @@ -57,17 +57,21 @@ func New( return nil, err } httpMethod, _ := maputil.GetStringOptional(jsonData, "httpMethod") + if httpMethod == "" { + httpMethod = http.MethodPost + } timeInterval, err := maputil.GetStringOptional(jsonData, "timeInterval") if err != nil { return nil, err } - if httpMethod == "" { - httpMethod = http.MethodPost + queryTimeout, err := maputil.GetStringOptional(jsonData, "queryTimeout") + if err != nil { + return nil, err } - promClient := client.NewClient(httpClient, httpMethod, settings.URL) + promClient := client.NewClient(httpClient, httpMethod, settings.URL, queryTimeout) // standard deviation sampler is the default for backwards compatibility exemplarSampler := exemplar.NewStandardDeviationSampler @@ -153,7 +157,7 @@ func (s *QueryData) handleQuery(ctx context.Context, bq backend.DataQuery, fromA func (s *QueryData) fetch(traceCtx context.Context, client *client.Client, q *models.Query, enablePrometheusDataplane, hasPrometheusRunQueriesInParallel bool) *backend.DataResponse { logger := s.log.FromContext(traceCtx) - logger.Debug("Sending query", "start", q.Start, "end", q.End, "step", q.Step, "query", q.Expr) + logger.Debug("Sending query", "start", q.Start, "end", q.End, "step", q.Step, "query", q.Expr /*, "queryTimeout", s.QueryTimeout*/) dr := &backend.DataResponse{ Frames: data.Frames{}, diff --git a/pkg/promlib/resource/resource.go b/pkg/promlib/resource/resource.go index 8635c618dc7..2c9705ef80f 100644 --- a/pkg/promlib/resource/resource.go +++ b/pkg/promlib/resource/resource.go @@ -41,8 +41,9 @@ func New( } return &Resource{ - log: plog, - promClient: client.NewClient(httpClient, httpMethod, settings.URL), + log: plog, + // we don't use queryTimeout for resource calls + promClient: client.NewClient(httpClient, httpMethod, settings.URL, ""), }, nil }