From 4e1e220962c40d3fabb76bb31d83b16996113db3 Mon Sep 17 00:00:00 2001 From: Alexander Berger Date: Mon, 5 Aug 2019 17:28:09 +0200 Subject: [PATCH] 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 --- docs/sources/features/datasources/cloudwatch.md | 1 + pkg/tsdb/cloudwatch/get_metric_data_test.go | 4 ---- pkg/tsdb/cloudwatch/get_metric_statistics.go | 9 ++++++++- public/app/plugins/datasource/cloudwatch/datasource.ts | 1 - .../datasource/cloudwatch/query_parameter_ctrl.ts | 1 - public/app/plugins/datasource/cloudwatch/types.ts | 1 - 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/sources/features/datasources/cloudwatch.md b/docs/sources/features/datasources/cloudwatch.md index 5fc80b1e503..01daabb79de 100644 --- a/docs/sources/features/datasources/cloudwatch.md +++ b/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" diff --git a/pkg/tsdb/cloudwatch/get_metric_data_test.go b/pkg/tsdb/cloudwatch/get_metric_data_test.go index e08c219eb63..ae8b4090e4a 100644 --- a/pkg/tsdb/cloudwatch/get_metric_data_test.go +++ b/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) } } }) diff --git a/pkg/tsdb/cloudwatch/get_metric_statistics.go b/pkg/tsdb/cloudwatch/get_metric_statistics.go index 117a64e2b16..ded80670e13 100644 --- a/pkg/tsdb/cloudwatch/get_metric_statistics.go +++ b/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{ diff --git a/public/app/plugins/datasource/cloudwatch/datasource.ts b/public/app/plugins/datasource/cloudwatch/datasource.ts index dae94fbf309..cc247f2f680 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.ts @@ -52,7 +52,6 @@ export default class CloudWatchDatasource extends DataSourceApi 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 => { diff --git a/public/app/plugins/datasource/cloudwatch/query_parameter_ctrl.ts b/public/app/plugins/datasource/cloudwatch/query_parameter_ctrl.ts index 5991305cf44..6463f3ae785 100644 --- a/public/app/plugins/datasource/cloudwatch/query_parameter_ctrl.ts +++ b/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'); diff --git a/public/app/plugins/datasource/cloudwatch/types.ts b/public/app/plugins/datasource/cloudwatch/types.ts index aaffcad0df3..7547d0d4215 100644 --- a/public/app/plugins/datasource/cloudwatch/types.ts +++ b/public/app/plugins/datasource/cloudwatch/types.ts @@ -9,5 +9,4 @@ export interface CloudWatchQuery extends DataQuery { statistics: string[]; period: string; expression: string; - returnData: boolean; }