Browse Source

CloudWatch: Fix alerting for queries with Id (using GetMetricData) (#17899)

This commit addresses half of #13749 by making sure GetMetricData 
works for alerting. Math Expressions (compound metrics) will still not 
work for alerting, this would require a bigger refactoring of Grafana's 
alerting service. However, with this commit at least alerting for basic 
metrics with non empty query Id will work.

Fixes half of #13749
Alexander Berger 6 years ago
parent
commit
4e1e220962

+ 1 - 0
docs/sources/features/datasources/cloudwatch.md

@@ -60,6 +60,7 @@ Here is a minimal policy example:
             "Sid": "AllowReadingMetricsFromCloudWatch",
             "Effect": "Allow",
             "Action": [
+                "cloudwatch:DescribeAlarmsForMetric",
                 "cloudwatch:ListMetrics",
                 "cloudwatch:GetMetricStatistics",
                 "cloudwatch:GetMetricData"

+ 0 - 4
pkg/tsdb/cloudwatch/get_metric_data_test.go

@@ -31,7 +31,6 @@ func TestCloudWatchGetMetricData(t *testing.T) {
 					Period:     300,
 					Id:         "id1",
 					Expression: "",
-					ReturnData: true,
 				},
 				"id2": {
 					RefId:      "B",
@@ -39,7 +38,6 @@ func TestCloudWatchGetMetricData(t *testing.T) {
 					Statistics: []*string{aws.String("Average")},
 					Id:         "id2",
 					Expression: "id1 * 2",
-					ReturnData: true,
 				},
 			}
 
@@ -58,11 +56,9 @@ func TestCloudWatchGetMetricData(t *testing.T) {
 					So(*v.MetricStat.Period, ShouldEqual, 300)
 					So(*v.MetricStat.Stat, ShouldEqual, "Average")
 					So(*v.Id, ShouldEqual, "id1")
-					So(*v.ReturnData, ShouldEqual, true)
 				} else {
 					So(*v.Id, ShouldEqual, "id2")
 					So(*v.Expression, ShouldEqual, "id1 * 2")
-					So(*v.ReturnData, ShouldEqual, true)
 				}
 			}
 		})

+ 8 - 1
pkg/tsdb/cloudwatch/get_metric_statistics.go

@@ -146,7 +146,14 @@ func parseQuery(model *simplejson.Json) (*CloudWatchQuery, error) {
 
 	alias := model.Get("alias").MustString()
 
-	returnData := model.Get("returnData").MustBool(false)
+	returnData := !model.Get("hide").MustBool(false)
+	queryType := model.Get("type").MustString()
+	if queryType == "" {
+		// If no type is provided we assume we are called by alerting service, which requires to return data!
+		// Note, this is sort of a hack, but the official Grafana interfaces do not carry the information
+		// who (which service) called the TsdbQueryEndpoint.Query(...) function.
+		returnData = true
+	}
 	highResolution := model.Get("highResolution").MustBool(false)
 
 	return &CloudWatchQuery{

+ 0 - 1
public/app/plugins/datasource/cloudwatch/datasource.ts

@@ -52,7 +52,6 @@ export default class CloudWatchDatasource extends DataSourceApi<CloudWatchQuery>
       item.period = String(this.getPeriod(item, options)); // use string format for period in graph query, and alerting
       item.id = this.templateSrv.replace(item.id, options.scopedVars);
       item.expression = this.templateSrv.replace(item.expression, options.scopedVars);
-      item.returnData = typeof item.hide === 'undefined' ? true : !item.hide;
 
       // valid ExtendedStatistics is like p90.00, check the pattern
       const hasInvalidStatistics = item.statistics.some(s => {

+ 0 - 1
public/app/plugins/datasource/cloudwatch/query_parameter_ctrl.ts

@@ -17,7 +17,6 @@ export class CloudWatchQueryParameterCtrl {
       target.region = target.region || 'default';
       target.id = target.id || '';
       target.expression = target.expression || '';
-      target.returnData = target.returnData || false;
       target.highResolution = target.highResolution || false;
 
       $scope.regionSegment = uiSegmentSrv.getSegmentForValue($scope.target.region, 'select region');

+ 0 - 1
public/app/plugins/datasource/cloudwatch/types.ts

@@ -9,5 +9,4 @@ export interface CloudWatchQuery extends DataQuery {
   statistics: string[];
   period: string;
   expression: string;
-  returnData: boolean;
 }