GCM: Improve error handling (grafana#93474)

* Improve error handling * More manual errorsource * Revert some changes from original PR * Update test
Nortech-ai · Sep 20, 2024 · e18d029 · e18d029
1 parent 19c7e1f
commit e18d029
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 26 deletions.
diff --git a/pkg/tsdb/cloud-monitoring/annotation_query.go b/pkg/tsdb/cloud-monitoring/annotation_query.go
@@ -23,7 +23,10 @@ type annotationEvent struct {
 func (s *Service) executeAnnotationQuery(ctx context.Context, req *backend.QueryDataRequest, dsInfo datasourceInfo, queries []cloudMonitoringQueryExecutor, logger log.Logger) (
 	*backend.QueryDataResponse, error) {
 	resp := backend.NewQueryDataResponse()
-	queryRes, dr, _, err := queries[0].run(ctx, req, s, dsInfo, logger)
+	dr, queryRes, _, err := queries[0].run(ctx, req, s, dsInfo, logger)
+	if dr.Error != nil {
+		errorsource.AddErrorToResponse(queries[0].getRefID(), resp, dr.Error)
+	}
 	if err != nil {
 		errorsource.AddErrorToResponse(queries[0].getRefID(), resp, err)
 		return resp, err
@@ -39,10 +42,14 @@ func (s *Service) executeAnnotationQuery(ctx context.Context, req *backend.Query
 	firstQuery := req.Queries[0]
 	err = json.Unmarshal(firstQuery.JSON, &tslq)
 	if err != nil {
+		logger.Error("error unmarshaling query", "error", err, "statusSource", backend.ErrorSourceDownstream)
+		errorsource.AddErrorToResponse(firstQuery.RefID, resp, err)
 		return resp, nil
 	}
-	err = parseToAnnotations(req.Queries[0].RefID, queryRes, dr.(cloudMonitoringResponse), tslq.TimeSeriesList.Title, tslq.TimeSeriesList.Text)
-	resp.Responses[firstQuery.RefID] = *queryRes
+
+	// parseToAnnotations never actually returns an error
+	err = parseToAnnotations(req.Queries[0].RefID, dr, queryRes.(cloudMonitoringResponse), tslq.TimeSeriesList.Title, tslq.TimeSeriesList.Text)
+	resp.Responses[firstQuery.RefID] = *dr
 
 	if err != nil {
 		errorsource.AddErrorToResponse(firstQuery.RefID, resp, err)

diff --git a/pkg/tsdb/cloud-monitoring/cloudmonitoring.go b/pkg/tsdb/cloud-monitoring/cloudmonitoring.go
@@ -361,20 +361,21 @@ func (s *Service) executeTimeSeriesQuery(ctx context.Context, req *backend.Query
 	*backend.QueryDataResponse, error) {
 	resp := backend.NewQueryDataResponse()
 	for _, queryExecutor := range queries {
-		queryRes, dr, executedQueryString, err := queryExecutor.run(ctx, req, s, dsInfo, logger)
+		dr, queryRes, executedQueryString, err := queryExecutor.run(ctx, req, s, dsInfo, logger)
 		if err != nil {
 			errorsource.AddErrorToResponse(queryExecutor.getRefID(), resp, err)
 			return resp, err
 		}
-		err = queryExecutor.parseResponse(queryRes, dr, executedQueryString, logger)
+		err = queryExecutor.parseResponse(dr, queryRes, executedQueryString, logger)
 		if err != nil {
-			// Default to a plugin error if there's no source
+			dr.Error = err
+			// // Default to a plugin error if there's no source
 			errWithSource := errorsource.SourceError(backend.ErrorSourcePlugin, err, false)
-			queryRes.Error = errWithSource.Unwrap()
-			queryRes.ErrorSource = errWithSource.ErrorSource()
+			dr.Error = errWithSource.Unwrap()
+			dr.ErrorSource = errWithSource.ErrorSource()
 		}
 
-		resp.Responses[queryExecutor.getRefID()] = *queryRes
+		resp.Responses[queryExecutor.getRefID()] = *dr
 	}
 
 	return resp, nil
@@ -608,21 +609,21 @@ func unmarshalResponse(res *http.Response, logger log.Logger) (cloudMonitoringRe
 	}()
 
 	if res.StatusCode/100 != 2 {
-		logger.Error("Request failed", "status", res.Status, "body", string(body))
+		logger.Error("Request failed", "status", res.Status, "body", string(body), "statusSource", backend.ErrorSourceDownstream)
 		return cloudMonitoringResponse{}, errorsource.SourceError(backend.ErrorSourceFromHTTPStatus(res.StatusCode), fmt.Errorf("query failed: %s", string(body)), false)
 	}
 
 	var data cloudMonitoringResponse
 	err = json.Unmarshal(body, &data)
 	if err != nil {
-		logger.Error("Failed to unmarshal CloudMonitoring response", "error", err, "status", res.Status, "body", string(body))
+		logger.Error("Failed to unmarshal CloudMonitoring response", "error", err, "status", res.Status, "body", string(body), "statusSource", backend.ErrorSourceDownstream)
 		return cloudMonitoringResponse{}, fmt.Errorf("failed to unmarshal query response: %w", err)
 	}
 
 	return data, nil
 }
 
-func addConfigData(frames data.Frames, dl string, unit string, period *string) data.Frames {
+func addConfigData(frames data.Frames, dl string, unit string, period *string, logger log.Logger) data.Frames {
 	for i := range frames {
 		if frames[i].Fields[1].Config == nil {
 			frames[i].Fields[1].Config = &data.FieldConfig{}
@@ -646,7 +647,7 @@ func addConfigData(frames data.Frames, dl string, unit string, period *string) d
 		if period != nil && *period != "" {
 			err := addInterval(*period, frames[i].Fields[0])
 			if err != nil {
-				backend.Logger.Error("Failed to add interval", "error", err)
+				logger.Error("Failed to add interval: %s", err, "statusSource", backend.ErrorSourceDownstream)
 			}
 		}
 	}

diff --git a/pkg/tsdb/cloud-monitoring/promql_query.go b/pkg/tsdb/cloud-monitoring/promql_query.go
@@ -23,7 +23,8 @@ func (promQLQ *cloudMonitoringProm) run(ctx context.Context, req *backend.QueryD
 	dr := &backend.DataResponse{}
 	projectName, err := s.ensureProject(ctx, dsInfo, promQLQ.parameters.ProjectName)
 	if err != nil {
-		return dr, backend.DataResponse{}, "", err
+		dr.Error = err
+		return dr, backend.DataResponse{}, "", nil
 	}
 	r, err := createRequest(ctx, &dsInfo, path.Join("/v1/projects", projectName, "location/global/prometheus/api/v1/query_range"), nil)
 	if err != nil {
@@ -43,12 +44,13 @@ func (promQLQ *cloudMonitoringProm) run(ctx context.Context, req *backend.QueryD
 
 	res, err := doRequestProm(r, dsInfo, requestBody)
 	if err != nil {
-		return dr, backend.DataResponse{}, "", err
+		dr.Error = err
+		return dr, backend.DataResponse{}, "", nil
 	}
 
 	defer func() {
 		if err := res.Body.Close(); err != nil {
-			s.logger.Error("Failed to close response body", "err", err)
+			s.logger.Error("Failed to close response body", "err", err, "statusSource", backend.ErrorSourceDownstream)
 		}
 	}()
 

diff --git a/pkg/tsdb/cloud-monitoring/promql_query_test.go b/pkg/tsdb/cloud-monitoring/promql_query_test.go
@@ -59,8 +59,10 @@ func TestPromqlQuery(t *testing.T) {
 		}
 
 		dr, parsedProm, _, err := query.run(context.Background(), &backend.QueryDataRequest{}, service, dsInfo, service.logger)
-		require.Error(t, err)
-		require.Equal(t, "not found!", err.Error())
+		require.NoError(t, err)
+		require.Error(t, dr.Error)
+		require.Equal(t, "not found!", dr.Error.Error())
+		require.True(t, backend.IsDownstreamError(dr.Error))
 
 		err = query.parseResponse(dr, parsedProm, "", service.logger)
 		require.NoError(t, err)

diff --git a/pkg/tsdb/cloud-monitoring/resource_handler.go b/pkg/tsdb/cloud-monitoring/resource_handler.go
@@ -386,7 +386,7 @@ func writeResponseBytes(rw http.ResponseWriter, code int, msg []byte) {
 	rw.WriteHeader(code)
 	_, err := rw.Write(msg)
 	if err != nil {
-		backend.Logger.Error("Unable to write HTTP response", "error", err)
+		backend.Logger.Error("Unable to write HTTP response", "error", err, "statusSource", backend.ErrorSourceDownstream)
 	}
 }
 

diff --git a/pkg/tsdb/cloud-monitoring/time_series_filter.go b/pkg/tsdb/cloud-monitoring/time_series_filter.go
@@ -21,7 +21,7 @@ func (timeSeriesFilter *cloudMonitoringTimeSeriesList) run(ctx context.Context,
 }
 
 func parseTimeSeriesResponse(queryRes *backend.DataResponse,
-	response cloudMonitoringResponse, executedQueryString string, query cloudMonitoringQueryExecutor, params url.Values, groupBys []string, _ log.Logger) error {
+	response cloudMonitoringResponse, executedQueryString string, query cloudMonitoringQueryExecutor, params url.Values, groupBys []string, logger log.Logger) error {
 	frames := data.Frames{}
 
 	for _, series := range response.TimeSeries {
@@ -47,7 +47,7 @@ func parseTimeSeriesResponse(queryRes *backend.DataResponse,
 	if len(response.TimeSeries) > 0 {
 		dl := query.buildDeepLink()
 		aggregationAlignmentString := params.Get("aggregation.alignmentPeriod")
-		frames = addConfigData(frames, dl, response.Unit, &aggregationAlignmentString)
+		frames = addConfigData(frames, dl, response.Unit, &aggregationAlignmentString, logger)
 	}
 
 	queryRes.Frames = frames
@@ -93,6 +93,7 @@ func (timeSeriesFilter *cloudMonitoringTimeSeriesList) buildDeepLink() string {
 			"Failed to generate deep link: unable to parse metrics explorer URL",
 			"ProjectName", timeSeriesFilter.parameters.ProjectName,
 			"error", err,
+			"statusSource", backend.ErrorSourcePlugin,
 		)
 	}
 

diff --git a/pkg/tsdb/cloud-monitoring/time_series_query.go b/pkg/tsdb/cloud-monitoring/time_series_query.go
@@ -78,7 +78,7 @@ func (timeSeriesQuery *cloudMonitoringTimeSeriesQuery) parseResponse(queryRes *b
 	}
 	if len(response.TimeSeriesData) > 0 {
 		dl := timeSeriesQuery.buildDeepLink()
-		frames = addConfigData(frames, dl, response.Unit, timeSeriesQuery.parameters.GraphPeriod)
+		frames = addConfigData(frames, dl, response.Unit, timeSeriesQuery.parameters.GraphPeriod, logger)
 	}
 
 	queryRes.Frames = frames
@@ -106,6 +106,7 @@ func (timeSeriesQuery *cloudMonitoringTimeSeriesQuery) buildDeepLink() string {
 			"Failed to generate deep link: unable to parse metrics explorer URL",
 			"ProjectName", timeSeriesQuery.parameters.Query,
 			"error", err,
+			"statusSource", backend.ErrorSourcePlugin,
 		)
 	}
 

diff --git a/pkg/tsdb/cloud-monitoring/utils.go b/pkg/tsdb/cloud-monitoring/utils.go
@@ -61,7 +61,7 @@ func createRequest(ctx context.Context, dsInfo *datasourceInfo, proxyPass string
 	}
 	req, err := http.NewRequestWithContext(ctx, method, dsInfo.services[cloudMonitor].url, body)
 	if err != nil {
-		backend.Logger.Error("Failed to create request", "error", err)
+		backend.Logger.Error("Failed to create request", "error", err, "statusSource", backend.ErrorSourceDownstream)
 		return nil, fmt.Errorf("failed to create request: %w", err)
 	}
 
@@ -142,23 +142,26 @@ func runTimeSeriesRequest(ctx context.Context, req *backend.QueryDataRequest,
 	dr := &backend.DataResponse{}
 	projectName, err := s.ensureProject(ctx, dsInfo, projectName)
 	if err != nil {
-		return dr, cloudMonitoringResponse{}, "", err
+		dr.Error = err
+		return dr, cloudMonitoringResponse{}, "", nil
 	}
 	timeSeriesMethod := "timeSeries"
 	if body != nil {
 		timeSeriesMethod += ":query"
 	}
 	r, err := createRequest(ctx, &dsInfo, path.Join("/v3/projects", projectName, timeSeriesMethod), nil)
 	if err != nil {
-		return dr, cloudMonitoringResponse{}, "", err
+		dr.Error = err
+		return dr, cloudMonitoringResponse{}, "", nil
 	}
 
 	span := traceReq(ctx, req, dsInfo, r, params.Encode())
 	defer span.End()
 
 	d, err := doRequestWithPagination(ctx, r, dsInfo, params, body, logger)
 	if err != nil {
-		return dr, cloudMonitoringResponse{}, "", err
+		dr.Error = err
+		return dr, cloudMonitoringResponse{}, "", nil
 	}
 
 	return dr, d, r.URL.RawQuery, nil