From 6e89421544d2bb5e4b9d3159f80a94655d4055cf Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Mon, 2 Jan 2023 09:08:31 +0100 Subject: [PATCH] CloudWatch: Add macro for resolving period in SEARCH expressions (#60435) * interpolate macro in the backend * refactor logger in cloudwatch query * revert added metrics * add docs * apply pr feedback * fix broken test * fix spelling --- .../aws-cloudwatch/query-editor/index.md | 4 + .../cloudwatch/metric_data_query_builder.go | 2 +- .../cloudwatch/models/cloudwatch_query.go | 42 +++-- .../models/cloudwatch_query_test.go | 154 +++++++++++++----- pkg/tsdb/cloudwatch/time_series_query.go | 2 +- .../completion/CompletionItemProvider.test.ts | 4 +- .../completion/CompletionItemProvider.ts | 5 + .../cloudwatch/metric-math/language.ts | 2 + 8 files changed, 159 insertions(+), 56 deletions(-) diff --git a/docs/sources/datasources/aws-cloudwatch/query-editor/index.md b/docs/sources/datasources/aws-cloudwatch/query-editor/index.md index 7a1d5e564b8..fe91bc93cbc 100644 --- a/docs/sources/datasources/aws-cloudwatch/query-editor/index.md +++ b/docs/sources/datasources/aws-cloudwatch/query-editor/index.md @@ -92,6 +92,10 @@ For example, to apply arithmetic operations to a metric, apply a unique string i > **Note:** If you use the expression field to reference another query, like `queryA * 2`, you can't create an alert rule based on that query. +##### Period macro + +If you're using a CloudWatch [`SEARCH`](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/search-expression-syntax.html) expression, you may want to use the `$__period_auto` macro rather than specifying a period explicitly. The `$__period_auto` macro will resolve to a [CloudWatch period](https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_GetMetricData.html) that is suitable for the chosen time range. + #### Deep-link Grafana panels to the CloudWatch console {{< figure src="/static/img/docs/v65/cloudwatch-deep-linking.png" max-width="500px" class="docs-image--right" caption="CloudWatch deep linking" >}} diff --git a/pkg/tsdb/cloudwatch/metric_data_query_builder.go b/pkg/tsdb/cloudwatch/metric_data_query_builder.go index b921a88739a..5552d7f3032 100644 --- a/pkg/tsdb/cloudwatch/metric_data_query_builder.go +++ b/pkg/tsdb/cloudwatch/metric_data_query_builder.go @@ -24,7 +24,7 @@ func (e *cloudWatchExecutor) buildMetricDataQuery(logger log.Logger, query *mode mdq.Label = &query.Label } - switch query.GetGMDAPIMode(logger) { + switch query.GetGetMetricDataAPIMode() { case models.GMDApiModeMathExpression: mdq.Period = aws.Int64(int64(query.Period)) mdq.Expression = aws.String(query.Expression) diff --git a/pkg/tsdb/cloudwatch/models/cloudwatch_query.go b/pkg/tsdb/cloudwatch/models/cloudwatch_query.go index 22de49c5fc4..29a67b3d80a 100644 --- a/pkg/tsdb/cloudwatch/models/cloudwatch_query.go +++ b/pkg/tsdb/cloudwatch/models/cloudwatch_query.go @@ -44,6 +44,7 @@ const ( const defaultRegion = "default" type CloudWatchQuery struct { + logger log.Logger RefId string Region string Id string @@ -65,7 +66,7 @@ type CloudWatchQuery struct { AccountId *string } -func (q *CloudWatchQuery) GetGMDAPIMode(logger log.Logger) GMDApiMode { +func (q *CloudWatchQuery) GetGetMetricDataAPIMode() GMDApiMode { if q.MetricQueryType == MetricQueryTypeSearch && q.MetricEditorMode == MetricEditorModeBuilder { if q.IsInferredSearchExpression() { return GMDApiModeInferredSearchExpression @@ -77,7 +78,7 @@ func (q *CloudWatchQuery) GetGMDAPIMode(logger log.Logger) GMDApiMode { return GMDApiModeSQLExpression } - logger.Warn("could not resolve CloudWatch metric query type. Falling back to metric stat.", "query", q) + q.logger.Warn("could not resolve CloudWatch metric query type. Falling back to metric stat.", "query", q) return GMDApiModeMetricStat } @@ -229,7 +230,7 @@ type metricsDataQuery struct { // ParseMetricDataQueries decodes the metric data queries json, validates, sets default values and returns an array of CloudWatchQueries. // The CloudWatchQuery has a 1 to 1 mapping to a query editor row -func ParseMetricDataQueries(dataQueries []backend.DataQuery, startTime time.Time, endTime time.Time, defaultRegion string, dynamicLabelsEnabled, +func ParseMetricDataQueries(dataQueries []backend.DataQuery, startTime time.Time, endTime time.Time, defaultRegion string, logger log.Logger, dynamicLabelsEnabled, crossAccountQueryingEnabled bool) ([]*CloudWatchQuery, error) { var metricDataQueries = make(map[string]metricsDataQuery) for _, query := range dataQueries { @@ -250,6 +251,7 @@ func ParseMetricDataQueries(dataQueries []backend.DataQuery, startTime time.Time var result []*CloudWatchQuery for refId, mdq := range metricDataQueries { cwQuery := &CloudWatchQuery{ + logger: logger, Alias: mdq.Alias, RefId: refId, Id: mdq.Id, @@ -266,6 +268,8 @@ func ParseMetricDataQueries(dataQueries []backend.DataQuery, startTime time.Time return nil, &QueryError{Err: err, RefID: refId} } + cwQuery.applyMacros(startTime, endTime) + cwQuery.migrateLegacyQuery(mdq, dynamicLabelsEnabled) result = append(result, cwQuery) @@ -274,6 +278,12 @@ func ParseMetricDataQueries(dataQueries []backend.DataQuery, startTime time.Time return result, nil } +func (q *CloudWatchQuery) applyMacros(startTime, endTime time.Time) { + if q.GetGetMetricDataAPIMode() == GMDApiModeMathExpression { + q.Expression = strings.ReplaceAll(q.Expression, "$__period_auto", strconv.Itoa(calculatePeriodBasedOnTimeRange(startTime, endTime))) + } +} + func (q *CloudWatchQuery) migrateLegacyQuery(query metricsDataQuery, dynamicLabelsEnabled bool) { q.Statistic = getStatistic(query) q.Label = getLabel(query, dynamicLabelsEnabled) @@ -396,21 +406,27 @@ func getLabel(query metricsDataQuery, dynamicLabelsEnabled bool) string { return result } +func calculatePeriodBasedOnTimeRange(startTime, endTime time.Time) int { + deltaInSeconds := endTime.Sub(startTime).Seconds() + periods := getRetainedPeriods(time.Since(startTime)) + datapoints := int(math.Ceil(deltaInSeconds / 2000)) + period := periods[len(periods)-1] + for _, value := range periods { + if datapoints <= value { + period = value + break + } + } + + return period +} + func getPeriod(query metricsDataQuery, startTime, endTime time.Time) (int, error) { periodString := query.Period var period int var err error if strings.ToLower(periodString) == "auto" || periodString == "" { - deltaInSeconds := endTime.Sub(startTime).Seconds() - periods := getRetainedPeriods(time.Since(startTime)) - datapoints := int(math.Ceil(deltaInSeconds / 2000)) - period = periods[len(periods)-1] - for _, value := range periods { - if datapoints <= value { - period = value - break - } - } + period = calculatePeriodBasedOnTimeRange(startTime, endTime) } else { period, err = strconv.Atoi(periodString) if err != nil { diff --git a/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go b/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go index 1e86de34f14..f82db0b50ea 100644 --- a/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go +++ b/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go @@ -15,6 +15,8 @@ import ( "github.com/grafana/grafana/pkg/tsdb/cloudwatch/utils" ) +var logger = &logtest.Fake{} + func TestCloudWatchQuery(t *testing.T) { t.Run("Deeplink", func(t *testing.T) { t.Run("is not generated for MetricQueryTypeQuery", func(t *testing.T) { @@ -317,7 +319,7 @@ func TestRequestParser(t *testing.T) { }, } - migratedQueries, err := ParseMetricDataQueries(oldQuery, time.Now(), time.Now(), "us-east-2", false, false) + migratedQueries, err := ParseMetricDataQueries(oldQuery, time.Now(), time.Now(), "us-east-2", logger, false, false) assert.NoError(t, err) require.Len(t, migratedQueries, 1) require.NotNil(t, migratedQueries[0]) @@ -348,7 +350,7 @@ func TestRequestParser(t *testing.T) { }, } - results, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + results, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, results, 1) res := results[0] @@ -391,7 +393,7 @@ func TestRequestParser(t *testing.T) { }, } - results, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + results, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) assert.NoError(t, err) require.Len(t, results, 1) res := results[0] @@ -424,7 +426,7 @@ func TestRequestParser(t *testing.T) { }, } - _, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + _, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.Error(t, err) assert.Equal(t, `error parsing query "", failed to parse dimensions: unknown type as dimension value`, err.Error()) @@ -453,7 +455,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { }, } - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) assert.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -485,7 +487,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { to := time.Now() from := to.Local().Add(time.Minute * time.Duration(5)) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 60, res[0].Period) @@ -495,7 +497,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { to := time.Now() from := to.AddDate(0, 0, -1) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 60, res[0].Period) @@ -504,7 +506,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { t.Run("Time range is 2 days", func(t *testing.T) { to := time.Now() from := to.AddDate(0, 0, -2) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 300, res[0].Period) @@ -514,7 +516,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { to := time.Now() from := to.AddDate(0, 0, -7) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 900, res[0].Period) @@ -524,7 +526,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { to := time.Now() from := to.AddDate(0, 0, -30) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 3600, res[0].Period) @@ -534,7 +536,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { to := time.Now() from := to.AddDate(0, 0, -90) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 21600, res[0].Period) @@ -544,7 +546,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { to := time.Now() from := to.AddDate(-1, 0, 0) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.Nil(t, err) require.Len(t, res, 1) assert.Equal(t, 21600, res[0].Period) @@ -554,7 +556,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { to := time.Now() from := to.AddDate(-2, 0, 0) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 86400, res[0].Period) @@ -563,7 +565,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { t.Run("Time range is 2 days, but 16 days ago", func(t *testing.T) { to := time.Now().AddDate(0, 0, -14) from := to.AddDate(0, 0, -2) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 300, res[0].Period) @@ -572,7 +574,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { t.Run("Time range is 2 days, but 90 days ago", func(t *testing.T) { to := time.Now().AddDate(0, 0, -88) from := to.AddDate(0, 0, -2) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 3600, res[0].Period) @@ -581,7 +583,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { t.Run("Time range is 2 days, but 456 days ago", func(t *testing.T) { to := time.Now().AddDate(0, 0, -454) from := to.AddDate(0, 0, -2) - res, err := ParseMetricDataQueries(query, from, to, "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 21600, res[0].Period) @@ -596,7 +598,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { }`), }, } - _, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + _, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.Error(t, err) assert.Equal(t, `error parsing query "", failed to parse period as duration: time: invalid duration "invalid"`, err.Error()) }) @@ -611,7 +613,7 @@ func Test_ParseMetricDataQueries_periods(t *testing.T) { }, } - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) assert.NoError(t, err) require.Len(t, res, 1) @@ -698,13 +700,13 @@ func Test_ParseMetricDataQueries_query_type_and_metric_editor_mode_and_GMD_query ), }, } - res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) assert.Equal(t, tc.expectedMetricQueryType, res[0].MetricQueryType) assert.Equal(t, tc.expectedMetricEditorMode, res[0].MetricEditorMode) - assert.Equal(t, tc.expectedGMDApiMode, res[0].GetGMDAPIMode(&logtest.Fake{})) + assert.Equal(t, tc.expectedGMDApiMode, res[0].GetGetMetricDataAPIMode()) }) } } @@ -724,7 +726,7 @@ func Test_ParseMetricDataQueries_hide_and_ReturnData(t *testing.T) { }`), }, } - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -745,7 +747,7 @@ func Test_ParseMetricDataQueries_hide_and_ReturnData(t *testing.T) { }`), }, } - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -766,7 +768,7 @@ func Test_ParseMetricDataQueries_hide_and_ReturnData(t *testing.T) { }`), }, } - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -785,7 +787,7 @@ func Test_ParseMetricDataQueries_hide_and_ReturnData(t *testing.T) { }`), }, } - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -806,7 +808,7 @@ func Test_ParseMetricDataQueries_hide_and_ReturnData(t *testing.T) { }`), }, } - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -827,7 +829,7 @@ func Test_ParseMetricDataQueries_hide_and_ReturnData(t *testing.T) { }`), }, } - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -850,7 +852,7 @@ func Test_ParseMetricDataQueries_ID(t *testing.T) { }`), }, } - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -871,7 +873,7 @@ func Test_ParseMetricDataQueries_ID(t *testing.T) { }`), }, } - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -898,7 +900,7 @@ func Test_ParseMetricDataQueries_sets_label_when_label_is_present_in_json_query( }, } - res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", true, false) + res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, true, false) assert.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -962,7 +964,7 @@ func Test_ParseMetricDataQueries_migrate_alias_to_label(t *testing.T) { }, } - res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", true, false) + res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, true, false) assert.NoError(t, err) require.Len(t, res, 1) @@ -1009,7 +1011,7 @@ func Test_ParseMetricDataQueries_migrate_alias_to_label(t *testing.T) { }, } - res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", true, false) + res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, true, false) assert.NoError(t, err) require.Len(t, res, 2) @@ -1078,7 +1080,7 @@ func Test_ParseMetricDataQueries_migrate_alias_to_label(t *testing.T) { }`, tc.labelJson)), }, } - res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", tc.dynamicLabelsFeatureToggleEnabled, false) + res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, tc.dynamicLabelsFeatureToggleEnabled, false) assert.NoError(t, err) require.Len(t, res, 1) @@ -1105,7 +1107,7 @@ func Test_ParseMetricDataQueries_statistics_and_query_type_validation_and_MatchE { JSON: []byte("{}"), }, - }, time.Now(), time.Now(), "us-east-2", false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false, false) assert.Error(t, err) assert.Equal(t, `error parsing query "", query must have either statistic or statistics field`, err.Error()) @@ -1118,7 +1120,7 @@ func Test_ParseMetricDataQueries_statistics_and_query_type_validation_and_MatchE { JSON: []byte(`{"type":"some other type", "statistic":"Average", "matchExact":false}`), }, - }, time.Now(), time.Now(), "us-east-2", false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false, false) assert.NoError(t, err) assert.Empty(t, actual) @@ -1130,7 +1132,7 @@ func Test_ParseMetricDataQueries_statistics_and_query_type_validation_and_MatchE { JSON: []byte(`{"statistic":"Average"}`), }, - }, time.Now(), time.Now(), "us-east-2", false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false, false) assert.NoError(t, err) assert.NotEmpty(t, actual) @@ -1142,7 +1144,7 @@ func Test_ParseMetricDataQueries_statistics_and_query_type_validation_and_MatchE { JSON: []byte(`{"statistic":"Average"}`), }, - }, time.Now(), time.Now(), "us-east-2", false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false, false) assert.NoError(t, err) assert.Len(t, actual, 1) @@ -1156,7 +1158,7 @@ func Test_ParseMetricDataQueries_statistics_and_query_type_validation_and_MatchE { JSON: []byte(`{"statistic":"Average","matchExact":false}`), }, - }, time.Now(), time.Now(), "us-east-2", false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false, false) assert.NoError(t, err) assert.Len(t, actual, 1) @@ -1172,7 +1174,7 @@ func Test_ParseMetricDataQueries_account_Id(t *testing.T) { { JSON: []byte(`{"accountId":"some account id", "statistic":"Average"}`), }, - }, time.Now(), time.Now(), "us-east-2", false, true) + }, time.Now(), time.Now(), "us-east-2", logger, false, true) assert.NoError(t, err) require.Len(t, actual, 1) @@ -1187,7 +1189,7 @@ func Test_ParseMetricDataQueries_account_Id(t *testing.T) { { JSON: []byte(`{"accountId":"some account id", "statistic":"Average"}`), }, - }, time.Now(), time.Now(), "us-east-2", false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false, false) assert.NoError(t, err) require.Len(t, actual, 1) @@ -1219,10 +1221,82 @@ func Test_ParseMetricDataQueries_default_region(t *testing.T) { } region := "us-east-2" - res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), region, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), region, logger, false, false) assert.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) assert.Equal(t, region, res[0].Region) }) } + +func Test_ParseMetricDataQueries_ApplyMacros(t *testing.T) { + t.Run("should expand $__period_auto macro when a metric search code query is used", func(t *testing.T) { + timeNow := time.Now() + testCases := []struct { + startTime time.Time + expectedPeriod string + }{ + { + startTime: timeNow.Add(-2 * time.Hour), + expectedPeriod: "60", + }, + { + startTime: timeNow.Add(-100 * time.Hour), + expectedPeriod: "300", + }, + { + startTime: timeNow.Add(-1000 * time.Hour), + expectedPeriod: "3600", + }, + } + for _, tc := range testCases { + t.Run(fmt.Sprintf("should expand $__period_auto macro to %s when a metric search code query is used", tc.expectedPeriod), func(t *testing.T) { + actual, err := ParseMetricDataQueries( + []backend.DataQuery{ + { + JSON: []byte(`{ + "refId":"A", + "region":"us-east-1", + "namespace":"ec2", + "metricName":"CPUUtilization", + "alias":"{{period}} {{any_other_word}}", + "dimensions":{"InstanceId":["test"]}, + "statistic":"Average", + "period":"600", + "hide":false, + "expression": "SEARCH('{AWS/EC2,InstanceId}', 'Average', $__period_auto)", + "metricQueryType": 0, + "metricEditorMode": 1 + }`), + }, + }, tc.startTime, time.Now(), "us-east-1", logger, false, false) + assert.NoError(t, err) + assert.Equal(t, fmt.Sprintf("SEARCH('{AWS/EC2,InstanceId}', 'Average', %s)", tc.expectedPeriod), actual[0].Expression) + }) + } + }) + + t.Run("should not expand __period_auto macro if it's a metric query code query", func(t *testing.T) { + actual, err := ParseMetricDataQueries( + []backend.DataQuery{ + { + JSON: []byte(`{ + "refId":"A", + "region":"us-east-1", + "namespace":"ec2", + "metricName":"CPUUtilization", + "alias":"{{period}} {{any_other_word}}", + "dimensions":{"InstanceId":["test"]}, + "statistic":"Average", + "period":"600", + "hide":false, + "expression": "SEARCH('{AWS/EC2,InstanceId}', 'Average', $__period_auto)", + "metricQueryType": 1, + "metricEditorMode": 1 + }`), + }, + }, time.Now(), time.Now(), "us-east-1", logger, false, false) + assert.NoError(t, err) + assert.Equal(t, "SEARCH('{AWS/EC2,InstanceId}', 'Average', $__period_auto)", actual[0].Expression) + }) +} diff --git a/pkg/tsdb/cloudwatch/time_series_query.go b/pkg/tsdb/cloudwatch/time_series_query.go index 7b17a5071d6..32a839f0767 100644 --- a/pkg/tsdb/cloudwatch/time_series_query.go +++ b/pkg/tsdb/cloudwatch/time_series_query.go @@ -36,7 +36,7 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, logger return nil, err } - requestQueries, err := models.ParseMetricDataQueries(req.Queries, startTime, endTime, instance.Settings.Region, + requestQueries, err := models.ParseMetricDataQueries(req.Queries, startTime, endTime, instance.Settings.Region, logger, e.features.IsEnabled(featuremgmt.FlagCloudWatchDynamicLabels), e.features.IsEnabled(featuremgmt.FlagCloudWatchCrossAccountQuerying)) if err != nil { diff --git a/public/app/plugins/datasource/cloudwatch/metric-math/completion/CompletionItemProvider.test.ts b/public/app/plugins/datasource/cloudwatch/metric-math/completion/CompletionItemProvider.test.ts index 0f935d925e1..0761ed05aa5 100644 --- a/public/app/plugins/datasource/cloudwatch/metric-math/completion/CompletionItemProvider.test.ts +++ b/public/app/plugins/datasource/cloudwatch/metric-math/completion/CompletionItemProvider.test.ts @@ -66,7 +66,9 @@ describe('MetricMath: CompletionItemProvider', () => { it('returns a suggestion for every period if the third arg of a search function', async () => { const { query, position } = MetricMathTestData.thirdArgAfterSearchQuery; const suggestions = await getSuggestions(query, position); - expect(suggestions.length).toEqual(METRIC_MATH_PERIODS.length); + // +1 because one suggestion is also added for the $__period_auto macro + const expectedSuggestionsLength = METRIC_MATH_PERIODS.length + 1; + expect(suggestions.length).toEqual(expectedSuggestionsLength); }); }); }); diff --git a/public/app/plugins/datasource/cloudwatch/metric-math/completion/CompletionItemProvider.ts b/public/app/plugins/datasource/cloudwatch/metric-math/completion/CompletionItemProvider.ts index 90502584d70..9ebfa2729a5 100644 --- a/public/app/plugins/datasource/cloudwatch/metric-math/completion/CompletionItemProvider.ts +++ b/public/app/plugins/datasource/cloudwatch/metric-math/completion/CompletionItemProvider.ts @@ -99,6 +99,11 @@ export class MetricMathCompletionItemProvider extends CompletionItemProvider { break; case SuggestionKind.Period: + addSuggestion('$__period_auto', { + kind: monaco.languages.CompletionItemKind.Variable, + sortText: 'a', + detail: 'Sets period dynamically to adjust to selected time range.', + }); METRIC_MATH_PERIODS.map((s, idx) => addSuggestion(s.toString(), { kind: monaco.languages.CompletionItemKind.Value, diff --git a/public/app/plugins/datasource/cloudwatch/metric-math/language.ts b/public/app/plugins/datasource/cloudwatch/metric-math/language.ts index e3b2cc9bd61..0c2de35a879 100644 --- a/public/app/plugins/datasource/cloudwatch/metric-math/language.ts +++ b/public/app/plugins/datasource/cloudwatch/metric-math/language.ts @@ -77,6 +77,7 @@ export const language: monacoType.languages.IMonarchLanguage = { root: [{ include: '@nonNestableStates' }, { include: '@strings' }], nonNestableStates: [ { include: '@variables' }, + { include: '@macros' }, { include: '@whitespace' }, { include: '@numbers' }, { include: '@assignment' }, @@ -92,6 +93,7 @@ export const language: monacoType.languages.IMonarchLanguage = { variables: [ [/\$[a-zA-Z0-9-_]+/, 'variable'], // $ followed by any letter/number we assume could be grafana template variable ], + macros: [[/\$__[a-zA-Z0-9-_]+/, 'type']], // example: $__period_auto whitespace: [[/\s+/, 'white']], assignment: [[/=/, 'tag']], numbers: [