Skip to content

Commit

Permalink
GCM: Time-range fix (grafana#98455)
Browse files Browse the repository at this point in the history
* Include timeRange on all query executors

* Ensure queries run against query specific time range

* Fix lint

* Improve safety of annotations queries
  • Loading branch information
aangelisc authored Jan 23, 2025
1 parent e110338 commit 4e740d8
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 22 deletions.
6 changes: 6 additions & 0 deletions pkg/tsdb/cloud-monitoring/annotation_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cloudmonitoring
import (
"context"
"encoding/json"
"errors"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -38,6 +39,11 @@ func (s *Service) executeAnnotationQuery(ctx context.Context, req *backend.Query
} `json:"timeSeriesList"`
}{}

if len(req.Queries) != 1 {
return nil, errors.New("multiple queries received in annotation-request")
}

// It's okay to use the first query for annotations as there should only be one
firstQuery := req.Queries[0]
err = json.Unmarshal(firstQuery.JSON, &tslq)
if err != nil {
Expand Down
18 changes: 10 additions & 8 deletions pkg/tsdb/cloud-monitoring/cloudmonitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,11 @@ func queryModel(query backend.DataQuery) (grafanaQuery, error) {

func (s *Service) buildQueryExecutors(logger log.Logger, req *backend.QueryDataRequest) ([]cloudMonitoringQueryExecutor, error) {
cloudMonitoringQueryExecutors := make([]cloudMonitoringQueryExecutor, 0, len(req.Queries))
startTime := req.Queries[0].TimeRange.From
endTime := req.Queries[0].TimeRange.To
durationSeconds := int(endTime.Sub(startTime).Seconds())

for _, query := range req.Queries {
for index, query := range req.Queries {
startTime := req.Queries[index].TimeRange.From
endTime := req.Queries[index].TimeRange.To
durationSeconds := int(endTime.Sub(startTime).Seconds())
q, err := queryModel(query)
if err != nil {
return nil, fmt.Errorf("could not unmarshal CloudMonitoringQuery json: %w", err)
Expand All @@ -401,8 +401,9 @@ func (s *Service) buildQueryExecutors(logger log.Logger, req *backend.QueryDataR
switch query.QueryType {
case string(dataquery.QueryTypeTIMESERIESLIST), string(dataquery.QueryTypeANNOTATION):
cmtsf := &cloudMonitoringTimeSeriesList{
refID: query.RefID,
aliasBy: q.AliasBy,
refID: query.RefID,
aliasBy: q.AliasBy,
timeRange: req.Queries[index].TimeRange,
}
if q.TimeSeriesList.View == nil || *q.TimeSeriesList.View == "" {
fullString := "FULL"
Expand All @@ -417,14 +418,15 @@ func (s *Service) buildQueryExecutors(logger log.Logger, req *backend.QueryDataR
aliasBy: q.AliasBy,
parameters: q.TimeSeriesQuery,
IntervalMS: query.Interval.Milliseconds(),
timeRange: req.Queries[0].TimeRange,
timeRange: req.Queries[index].TimeRange,
logger: logger,
}
case string(dataquery.QueryTypeSLO):
cmslo := &cloudMonitoringSLO{
refID: query.RefID,
aliasBy: q.AliasBy,
parameters: q.SloQuery,
timeRange: req.Queries[index].TimeRange,
}
cmslo.setParams(startTime, endTime, durationSeconds, query.Interval.Milliseconds())
queryInterface = cmslo
Expand All @@ -433,7 +435,7 @@ func (s *Service) buildQueryExecutors(logger log.Logger, req *backend.QueryDataR
refID: query.RefID,
aliasBy: q.AliasBy,
parameters: q.PromQLQuery,
timeRange: req.Queries[0].TimeRange,
timeRange: req.Queries[index].TimeRange,
logger: logger,
}
queryInterface = cmp
Expand Down
2 changes: 1 addition & 1 deletion pkg/tsdb/cloud-monitoring/promql_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (promQLQ *cloudMonitoringProm) run(ctx context.Context, req *backend.QueryD
return dr, backend.DataResponse{}, "", nil
}

span := traceReq(ctx, req, dsInfo, r, "")
span := traceReq(ctx, req, dsInfo, r, "", promQLQ.timeRange)
defer span.End()

requestBody := map[string]any{
Expand Down
2 changes: 1 addition & 1 deletion pkg/tsdb/cloud-monitoring/slo_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (

func (sloQ *cloudMonitoringSLO) run(ctx context.Context, req *backend.QueryDataRequest,
s *Service, dsInfo datasourceInfo, logger log.Logger) (*backend.DataResponse, any, string, error) {
return runTimeSeriesRequest(ctx, req, s, dsInfo, sloQ.parameters.ProjectName, sloQ.params, nil, logger)
return runTimeSeriesRequest(ctx, req, s, dsInfo, sloQ.parameters.ProjectName, sloQ.params, nil, logger, sloQ.timeRange)
}

func (sloQ *cloudMonitoringSLO) parseResponse(queryRes *backend.DataResponse,
Expand Down
2 changes: 1 addition & 1 deletion pkg/tsdb/cloud-monitoring/time_series_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

func (timeSeriesFilter *cloudMonitoringTimeSeriesList) run(ctx context.Context, req *backend.QueryDataRequest,
s *Service, dsInfo datasourceInfo, logger log.Logger) (*backend.DataResponse, any, string, error) {
return runTimeSeriesRequest(ctx, req, s, dsInfo, timeSeriesFilter.parameters.ProjectName, timeSeriesFilter.params, nil, logger)
return runTimeSeriesRequest(ctx, req, s, dsInfo, timeSeriesFilter.parameters.ProjectName, timeSeriesFilter.params, nil, logger, timeSeriesFilter.timeRange)
}

func parseTimeSeriesResponse(queryRes *backend.DataResponse,
Expand Down
8 changes: 4 additions & 4 deletions pkg/tsdb/cloud-monitoring/time_series_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func (timeSeriesQuery *cloudMonitoringTimeSeriesQuery) appendGraphPeriod(req *ba
if timeSeriesQuery.parameters.GraphPeriod != "disabled" {
if timeSeriesQuery.parameters.GraphPeriod == "auto" || timeSeriesQuery.parameters.GraphPeriod == "" {
intervalCalculator := gcmTime.NewCalculator(gcmTime.CalculatorOptions{})
interval := intervalCalculator.Calculate(req.Queries[0].TimeRange, time.Duration(timeSeriesQuery.IntervalMS/1000)*time.Second, req.Queries[0].MaxDataPoints)
interval := intervalCalculator.Calculate(timeSeriesQuery.timeRange, time.Duration(timeSeriesQuery.IntervalMS/1000)*time.Second, req.Queries[0].MaxDataPoints)
timeSeriesQuery.parameters.GraphPeriod = interval.Text
}
return fmt.Sprintf(" | graph_period %s", timeSeriesQuery.parameters.GraphPeriod)
Expand All @@ -29,14 +29,14 @@ func (timeSeriesQuery *cloudMonitoringTimeSeriesQuery) appendGraphPeriod(req *ba
func (timeSeriesQuery *cloudMonitoringTimeSeriesQuery) run(ctx context.Context, req *backend.QueryDataRequest,
s *Service, dsInfo datasourceInfo, logger log.Logger) (*backend.DataResponse, any, string, error) {
timeSeriesQuery.parameters.Query += timeSeriesQuery.appendGraphPeriod(req)
from := req.Queries[0].TimeRange.From
to := req.Queries[0].TimeRange.To
from := timeSeriesQuery.timeRange.From
to := timeSeriesQuery.timeRange.To
timeFormat := "2006/01/02-15:04:05"
timeSeriesQuery.parameters.Query += fmt.Sprintf(" | within d'%s', d'%s'", from.UTC().Format(timeFormat), to.UTC().Format(timeFormat))
requestBody := map[string]any{
"query": timeSeriesQuery.parameters.Query,
}
return runTimeSeriesRequest(ctx, req, s, dsInfo, timeSeriesQuery.parameters.ProjectName, nil, requestBody, logger)
return runTimeSeriesRequest(ctx, req, s, dsInfo, timeSeriesQuery.parameters.ProjectName, nil, requestBody, logger, timeSeriesQuery.timeRange)
}

func (timeSeriesQuery *cloudMonitoringTimeSeriesQuery) parseResponse(queryRes *backend.DataResponse,
Expand Down
6 changes: 4 additions & 2 deletions pkg/tsdb/cloud-monitoring/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,17 @@ type (
aliasBy string
parameters *dataquery.TimeSeriesList
// Processed properties
params url.Values
timeRange backend.TimeRange
params url.Values
}
// cloudMonitoringSLO is used to build time series with a filter but for the SLO case
cloudMonitoringSLO struct {
refID string
aliasBy string
parameters *dataquery.SLOQuery
// Processed properties
params url.Values
timeRange backend.TimeRange
params url.Values
}

// cloudMonitoringProm is used to build a promQL queries
Expand Down
10 changes: 5 additions & 5 deletions pkg/tsdb/cloud-monitoring/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,11 @@ func doRequestWithPagination(ctx context.Context, r *http.Request, dsInfo dataso
return d, nil
}

func traceReq(ctx context.Context, req *backend.QueryDataRequest, dsInfo datasourceInfo, _ *http.Request, target string) trace.Span {
func traceReq(ctx context.Context, req *backend.QueryDataRequest, dsInfo datasourceInfo, _ *http.Request, target string, timeRange backend.TimeRange) trace.Span {
_, span := tracing.DefaultTracer().Start(ctx, "cloudMonitoring query", trace.WithAttributes(
attribute.String("target", target),
attribute.String("from", req.Queries[0].TimeRange.From.String()),
attribute.String("until", req.Queries[0].TimeRange.To.String()),
attribute.String("from", timeRange.From.String()),
attribute.String("until", timeRange.To.String()),
attribute.Int64("datasource_id", dsInfo.id),
attribute.Int64("org_id", req.PluginContext.OrgID),
))
Expand All @@ -137,7 +137,7 @@ func traceReq(ctx context.Context, req *backend.QueryDataRequest, dsInfo datasou
}

func runTimeSeriesRequest(ctx context.Context, req *backend.QueryDataRequest,
s *Service, dsInfo datasourceInfo, projectName string, params url.Values, body map[string]any, logger log.Logger) (*backend.DataResponse, cloudMonitoringResponse, string, error) {
s *Service, dsInfo datasourceInfo, projectName string, params url.Values, body map[string]any, logger log.Logger, timeRange backend.TimeRange) (*backend.DataResponse, cloudMonitoringResponse, string, error) {
dr := &backend.DataResponse{}
projectName, err := s.ensureProject(ctx, dsInfo, projectName)
if err != nil {
Expand All @@ -154,7 +154,7 @@ func runTimeSeriesRequest(ctx context.Context, req *backend.QueryDataRequest,
return dr, cloudMonitoringResponse{}, "", nil
}

span := traceReq(ctx, req, dsInfo, r, params.Encode())
span := traceReq(ctx, req, dsInfo, r, params.Encode(), timeRange)
defer span.End()

d, err := doRequestWithPagination(ctx, r, dsInfo, params, body, logger)
Expand Down

0 comments on commit 4e740d8

Please sign in to comment.