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
pull/95265/head
ismail simsek 8 months ago committed by GitHub
parent 0872eb2791
commit 78a00d09cd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 2
      packages/grafana-prometheus/src/datasource.ts
  2. 17
      pkg/promlib/client/client.go
  3. 10
      pkg/promlib/client/client_test.go
  4. 12
      pkg/promlib/querydata/request.go
  5. 5
      pkg/promlib/resource/resource.go

@ -97,7 +97,6 @@ export class PrometheusDatasource
basicAuth: any; basicAuth: any;
withCredentials: boolean; withCredentials: boolean;
interval: string; interval: string;
queryTimeout: string | undefined;
httpMethod: string; httpMethod: string;
languageProvider: PrometheusLanguageProvider; languageProvider: PrometheusLanguageProvider;
exemplarTraceIdDestinations: ExemplarTraceIdDestination[] | undefined; exemplarTraceIdDestinations: ExemplarTraceIdDestination[] | undefined;
@ -127,7 +126,6 @@ export class PrometheusDatasource
this.basicAuth = instanceSettings.basicAuth; this.basicAuth = instanceSettings.basicAuth;
this.withCredentials = Boolean(instanceSettings.withCredentials); this.withCredentials = Boolean(instanceSettings.withCredentials);
this.interval = instanceSettings.jsonData.timeInterval || '15s'; this.interval = instanceSettings.jsonData.timeInterval || '15s';
this.queryTimeout = instanceSettings.jsonData.queryTimeout;
this.httpMethod = instanceSettings.jsonData.httpMethod || 'GET'; this.httpMethod = instanceSettings.jsonData.httpMethod || 'GET';
this.exemplarTraceIdDestinations = instanceSettings.jsonData.exemplarTraceIdDestinations; this.exemplarTraceIdDestinations = instanceSettings.jsonData.exemplarTraceIdDestinations;
this.hasIncrementalQuery = instanceSettings.jsonData.incrementalQuerying ?? false; this.hasIncrementalQuery = instanceSettings.jsonData.incrementalQuerying ?? false;

@ -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 // 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. // client we can parse response directly into DataFrame.
type Client struct { type Client struct {
doer doer doer doer
method string method string
baseUrl string baseUrl string
queryTimeout string
} }
func NewClient(d doer, method, baseUrl string) *Client { func NewClient(d doer, method, baseUrl, queryTimeout string) *Client {
return &Client{doer: d, method: method, baseUrl: baseUrl} return &Client{doer: d, method: method, baseUrl: baseUrl, queryTimeout: queryTimeout}
} }
func (c *Client) QueryRange(ctx context.Context, q *models.Query) (*http.Response, error) { 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), "end": formatTime(tr.End),
"step": strconv.FormatFloat(tr.Step.Seconds(), 'f', -1, 64), "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) req, err := c.createQueryRequest(ctx, "api/v1/query_range", qv)
if err != nil { 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. // Instead of aligning we use time point directly.
// https://prometheus.io/docs/prometheus/latest/querying/api/#instant-queries // https://prometheus.io/docs/prometheus/latest/querying/api/#instant-queries
qv := map[string]string{"query": q.Expr, "time": formatTime(q.End)} 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) req, err := c.createQueryRequest(ctx, "api/v1/query", qv)
if err != nil { if err != nil {
return nil, err return nil, err

@ -30,7 +30,7 @@ func TestClient(t *testing.T) {
t.Run("QueryResource", func(t *testing.T) { t.Run("QueryResource", func(t *testing.T) {
doer := &MockDoer{} doer := &MockDoer{}
// The method here does not really matter for resource calls // 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) { t.Run("sends correct POST request", func(t *testing.T) {
req := &backend.CallResourceRequest{ req := &backend.CallResourceRequest{
@ -86,7 +86,7 @@ func TestClient(t *testing.T) {
doer := &MockDoer{} doer := &MockDoer{}
t.Run("sends correct POST query", func(t *testing.T) { 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{ req := &models.Query{
Expr: "rate(ALERTS{job=\"test\" [$__rate_interval]})", Expr: "rate(ALERTS{job=\"test\" [$__rate_interval]})",
Start: time.Unix(0, 0), 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")) require.Equal(t, "application/x-www-form-urlencoded", doer.Req.Header.Get("Content-Type"))
body, err := io.ReadAll(doer.Req.Body) body, err := io.ReadAll(doer.Req.Body)
require.NoError(t, err) 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()) require.Equal(t, "http://localhost:9090/api/v1/query_range", doer.Req.URL.String())
}) })
t.Run("sends correct GET query", func(t *testing.T) { 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{ req := &models.Query{
Expr: "rate(ALERTS{job=\"test\" [$__rate_interval]})", Expr: "rate(ALERTS{job=\"test\" [$__rate_interval]})",
Start: time.Unix(0, 0), Start: time.Unix(0, 0),
@ -135,7 +135,7 @@ func TestClient(t *testing.T) {
body, err := io.ReadAll(doer.Req.Body) body, err := io.ReadAll(doer.Req.Body)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, []byte{}, body) 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())
}) })
}) })
} }

@ -57,17 +57,21 @@ func New(
return nil, err return nil, err
} }
httpMethod, _ := maputil.GetStringOptional(jsonData, "httpMethod") httpMethod, _ := maputil.GetStringOptional(jsonData, "httpMethod")
if httpMethod == "" {
httpMethod = http.MethodPost
}
timeInterval, err := maputil.GetStringOptional(jsonData, "timeInterval") timeInterval, err := maputil.GetStringOptional(jsonData, "timeInterval")
if err != nil { if err != nil {
return nil, err return nil, err
} }
if httpMethod == "" { queryTimeout, err := maputil.GetStringOptional(jsonData, "queryTimeout")
httpMethod = http.MethodPost 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 // standard deviation sampler is the default for backwards compatibility
exemplarSampler := exemplar.NewStandardDeviationSampler 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, func (s *QueryData) fetch(traceCtx context.Context, client *client.Client, q *models.Query,
enablePrometheusDataplane, hasPrometheusRunQueriesInParallel bool) *backend.DataResponse { enablePrometheusDataplane, hasPrometheusRunQueriesInParallel bool) *backend.DataResponse {
logger := s.log.FromContext(traceCtx) 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{ dr := &backend.DataResponse{
Frames: data.Frames{}, Frames: data.Frames{},

@ -41,8 +41,9 @@ func New(
} }
return &Resource{ return &Resource{
log: plog, log: plog,
promClient: client.NewClient(httpClient, httpMethod, settings.URL), // we don't use queryTimeout for resource calls
promClient: client.NewClient(httpClient, httpMethod, settings.URL, ""),
}, nil }, nil
} }

Loading…
Cancel
Save