diff --git a/pkg/tsdb/cloudwatch/cloudwatch.go b/pkg/tsdb/cloudwatch/cloudwatch.go index 771698f3563..f1e62c28372 100644 --- a/pkg/tsdb/cloudwatch/cloudwatch.go +++ b/pkg/tsdb/cloudwatch/cloudwatch.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "net/http" - "regexp" "sync" "github.com/aws/aws-sdk-go/aws" @@ -51,7 +50,6 @@ const ( ) var logger = log.New("tsdb.cloudwatch") -var aliasFormat = regexp.MustCompile(`\{\{\s*(.+?)\s*\}\}`) func ProvideService(cfg *setting.Cfg, httpClientProvider httpclient.Provider, features featuremgmt.FeatureToggles) *CloudWatchService { logger.Debug("Initializing") diff --git a/pkg/tsdb/cloudwatch/kinds/dataquery/types_dataquery_gen.go b/pkg/tsdb/cloudwatch/kinds/dataquery/types_dataquery_gen.go index f0b62e3612d..d442e211870 100644 --- a/pkg/tsdb/cloudwatch/kinds/dataquery/types_dataquery_gen.go +++ b/pkg/tsdb/cloudwatch/kinds/dataquery/types_dataquery_gen.go @@ -422,7 +422,8 @@ type CloudWatchMetricsQuery struct { // The ID of the AWS account to query for the metric, specifying `all` will query all accounts that the monitoring account is permitted to query. AccountId *string `json:"accountId,omitempty"` - // To be deprecated. Use label + // Deprecated: use label + // @deprecated use label Alias *string `json:"alias,omitempty"` // For mixed data sources the selected datasource is on the query level. diff --git a/pkg/tsdb/cloudwatch/metric_data_input_builder.go b/pkg/tsdb/cloudwatch/metric_data_input_builder.go index 962e628b5f4..ebb667c2610 100644 --- a/pkg/tsdb/cloudwatch/metric_data_input_builder.go +++ b/pkg/tsdb/cloudwatch/metric_data_input_builder.go @@ -7,7 +7,6 @@ import ( "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" ) @@ -19,7 +18,7 @@ func (e *cloudWatchExecutor) buildMetricDataInput(logger log.Logger, startTime t ScanBy: aws.String("TimestampAscending"), } - shouldSetLabelOptions := e.features.IsEnabled(featuremgmt.FlagCloudWatchDynamicLabels) && len(queries) > 0 && len(queries[0].TimezoneUTCOffset) > 0 + shouldSetLabelOptions := len(queries) > 0 && len(queries[0].TimezoneUTCOffset) > 0 if shouldSetLabelOptions { metricDataInput.LabelOptions = &cloudwatch.LabelOptions{ diff --git a/pkg/tsdb/cloudwatch/metric_data_input_builder_test.go b/pkg/tsdb/cloudwatch/metric_data_input_builder_test.go index 24b441eeba2..320cd8109fd 100644 --- a/pkg/tsdb/cloudwatch/metric_data_input_builder_test.go +++ b/pkg/tsdb/cloudwatch/metric_data_input_builder_test.go @@ -20,17 +20,14 @@ func TestMetricDataInputBuilder(t *testing.T) { name string timezoneUTCOffset string expectedLabelOptions *cloudwatch.LabelOptions - featureEnabled bool }{ - {name: "when timezoneUTCOffset is provided and feature is enabled", timezoneUTCOffset: "+1234", expectedLabelOptions: &cloudwatch.LabelOptions{Timezone: aws.String("+1234")}, featureEnabled: true}, - {name: "when timezoneUTCOffset is not provided and feature is enabled", timezoneUTCOffset: "", expectedLabelOptions: nil, featureEnabled: true}, - {name: "when timezoneUTCOffset is provided and feature is disabled", timezoneUTCOffset: "+1234", expectedLabelOptions: nil, featureEnabled: false}, - {name: "when timezoneUTCOffset is not provided and feature is disabled", timezoneUTCOffset: "", expectedLabelOptions: nil, featureEnabled: false}, + {name: "when timezoneUTCOffset is provided", timezoneUTCOffset: "+1234", expectedLabelOptions: &cloudwatch.LabelOptions{Timezone: aws.String("+1234")}}, + {name: "when timezoneUTCOffset is not provided", timezoneUTCOffset: "", expectedLabelOptions: nil}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures(featuremgmt.FlagCloudWatchDynamicLabels, tc.featureEnabled)) + executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) query := getBaseQuery() query.TimezoneUTCOffset = tc.timezoneUTCOffset diff --git a/pkg/tsdb/cloudwatch/metric_data_query_builder.go b/pkg/tsdb/cloudwatch/metric_data_query_builder.go index 5552d7f3032..0a6fa90316b 100644 --- a/pkg/tsdb/cloudwatch/metric_data_query_builder.go +++ b/pkg/tsdb/cloudwatch/metric_data_query_builder.go @@ -10,7 +10,6 @@ import ( "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" ) @@ -20,7 +19,7 @@ func (e *cloudWatchExecutor) buildMetricDataQuery(logger log.Logger, query *mode ReturnData: aws.Bool(query.ReturnData), } - if e.features.IsEnabled(featuremgmt.FlagCloudWatchDynamicLabels) && len(query.Label) > 0 { + if len(query.Label) > 0 { mdq.Label = &query.Label } diff --git a/pkg/tsdb/cloudwatch/metric_data_query_builder_test.go b/pkg/tsdb/cloudwatch/metric_data_query_builder_test.go index 84b827ec925..d075fdfd7fd 100644 --- a/pkg/tsdb/cloudwatch/metric_data_query_builder_test.go +++ b/pkg/tsdb/cloudwatch/metric_data_query_builder_test.go @@ -95,8 +95,8 @@ func TestMetricDataQueryBuilder(t *testing.T) { assert.Equal(t, `SUM([a,b])`, *mdq.Expression) }) - t.Run("should set label when dynamic labels feature toggle is enabled", func(t *testing.T) { - executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures(featuremgmt.FlagCloudWatchDynamicLabels)) + t.Run("should set label", func(t *testing.T) { + executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) query := getBaseQuery() query.Label = "some label" @@ -107,35 +107,19 @@ func TestMetricDataQueryBuilder(t *testing.T) { assert.Equal(t, "some label", *mdq.Label) }) - testCases := map[string]struct { - feature *featuremgmt.FeatureManager - label string - }{ - "should not set label when dynamic labels feature toggle is disabled": { - feature: featuremgmt.WithFeatures(), - label: "some label", - }, - "should not set label for empty string query label": { - feature: featuremgmt.WithFeatures(featuremgmt.FlagCloudWatchDynamicLabels), - label: "", - }, - } + t.Run("should not set label for empty string query label", func(t *testing.T) { + executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) + query := getBaseQuery() + query.Label = "" - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, tc.feature) - query := getBaseQuery() - query.Label = tc.label + mdq, err := executor.buildMetricDataQuery(logger, query) - mdq, err := executor.buildMetricDataQuery(logger, query) - - assert.NoError(t, err) - assert.Nil(t, mdq.Label) - }) - } + assert.NoError(t, err) + assert.Nil(t, mdq.Label) + }) t.Run(`should not specify accountId when it is "all"`, func(t *testing.T) { - executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures(featuremgmt.FlagCloudWatchDynamicLabels)) + executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) query := &models.CloudWatchQuery{ Namespace: "AWS/EC2", MetricName: "CPUUtilization", @@ -153,7 +137,7 @@ func TestMetricDataQueryBuilder(t *testing.T) { }) t.Run("should set accountId when it is specified", func(t *testing.T) { - executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures(featuremgmt.FlagCloudWatchDynamicLabels)) + executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) query := &models.CloudWatchQuery{ Namespace: "AWS/EC2", MetricName: "CPUUtilization", diff --git a/pkg/tsdb/cloudwatch/models/cloudwatch_query.go b/pkg/tsdb/cloudwatch/models/cloudwatch_query.go index e88723e5cb4..7666e709c24 100644 --- a/pkg/tsdb/cloudwatch/models/cloudwatch_query.go +++ b/pkg/tsdb/cloudwatch/models/cloudwatch_query.go @@ -63,7 +63,6 @@ type CloudWatchQuery struct { ReturnData bool Dimensions map[string][]string Period int - Alias string Label string MatchExact bool UsedExpression string @@ -150,7 +149,7 @@ func (q *CloudWatchQuery) IsMultiValuedDimensionExpression() bool { return false } -func (q *CloudWatchQuery) BuildDeepLink(startTime time.Time, endTime time.Time, dynamicLabelEnabled bool) (string, error) { +func (q *CloudWatchQuery) BuildDeepLink(startTime time.Time, endTime time.Time) (string, error) { if q.IsMathExpression() || q.MetricQueryType == MetricQueryTypeQuery { return "", nil } @@ -166,9 +165,7 @@ func (q *CloudWatchQuery) BuildDeepLink(startTime time.Time, endTime time.Time, if q.isSearchExpression() { metricExpressions := &metricExpression{Expression: q.UsedExpression} - if dynamicLabelEnabled { - metricExpressions.Label = q.Label - } + metricExpressions.Label = q.Label link.Metrics = []interface{}{metricExpressions} } else { metricStat := []interface{}{q.Namespace, q.MetricName} @@ -179,9 +176,7 @@ func (q *CloudWatchQuery) BuildDeepLink(startTime time.Time, endTime time.Time, Stat: q.Statistic, Period: q.Period, } - if dynamicLabelEnabled { - metricStatMeta.Label = q.Label - } + metricStatMeta.Label = q.Label if q.AccountId != nil { metricStatMeta.AccountId = *q.AccountId } @@ -221,7 +216,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, logger log.Logger, dynamicLabelsEnabled, +func ParseMetricDataQueries(dataQueries []backend.DataQuery, startTime time.Time, endTime time.Time, defaultRegion string, logger log.Logger, crossAccountQueryingEnabled bool) ([]*CloudWatchQuery, error) { var metricDataQueries = make(map[string]metricsDataQuery) for _, query := range dataQueries { @@ -251,10 +246,6 @@ func ParseMetricDataQueries(dataQueries []backend.DataQuery, startTime time.Time TimezoneUTCOffset: mdq.TimezoneUTCOffset, } - if mdq.Alias != nil { - cwQuery.Alias = *mdq.Alias - } - if mdq.MetricName != nil { cwQuery.MetricName = *mdq.MetricName } @@ -271,13 +262,17 @@ func ParseMetricDataQueries(dataQueries []backend.DataQuery, startTime time.Time cwQuery.Expression = *mdq.Expression } + if mdq.Label != nil { + cwQuery.Label = *mdq.Label + } + if err := cwQuery.validateAndSetDefaults(refId, mdq, startTime, endTime, defaultRegion, crossAccountQueryingEnabled); err != nil { return nil, &QueryError{Err: err, RefID: refId} } cwQuery.applyMacros(startTime, endTime) - cwQuery.migrateLegacyQuery(mdq, dynamicLabelsEnabled) + cwQuery.migrateLegacyQuery(mdq) result = append(result, cwQuery) } @@ -291,9 +286,9 @@ func (q *CloudWatchQuery) applyMacros(startTime, endTime time.Time) { } } -func (q *CloudWatchQuery) migrateLegacyQuery(query metricsDataQuery, dynamicLabelsEnabled bool) { +func (q *CloudWatchQuery) migrateLegacyQuery(query metricsDataQuery) { q.Statistic = getStatistic(query) - q.Label = getLabel(query, dynamicLabelsEnabled) + q.Label = getLabel(query) } func (q *CloudWatchQuery) validateAndSetDefaults(refId string, metricsDataQuery metricsDataQuery, startTime, endTime time.Time, @@ -386,33 +381,33 @@ var aliasPatterns = map[string]string{ var legacyAliasRegexp = regexp.MustCompile(`{{\s*(.+?)\s*}}`) -func getLabel(query metricsDataQuery, dynamicLabelsEnabled bool) string { +func getLabel(query metricsDataQuery) string { + deprecatedAlias := query.Alias //nolint:staticcheck + if query.Label != nil { return *query.Label } - if query.Alias != nil && *query.Alias == "" { + if deprecatedAlias != nil && *deprecatedAlias == "" { return "" } var result string - if dynamicLabelsEnabled { - fullAliasField := "" - if query.Alias != nil { - fullAliasField = *query.Alias - } - matches := legacyAliasRegexp.FindAllStringSubmatch(fullAliasField, -1) - - for _, groups := range matches { - fullMatch := groups[0] - subgroup := groups[1] - if dynamicLabel, ok := aliasPatterns[subgroup]; ok { - fullAliasField = strings.ReplaceAll(fullAliasField, fullMatch, dynamicLabel) - } else { - fullAliasField = strings.ReplaceAll(fullAliasField, fullMatch, fmt.Sprintf(`${PROP('Dim.%s')}`, subgroup)) - } - } - result = fullAliasField + fullAliasField := "" + if deprecatedAlias != nil { + fullAliasField = *deprecatedAlias } + matches := legacyAliasRegexp.FindAllStringSubmatch(fullAliasField, -1) + + for _, groups := range matches { + fullMatch := groups[0] + subgroup := groups[1] + if dynamicLabel, ok := aliasPatterns[subgroup]; ok { + fullAliasField = strings.ReplaceAll(fullAliasField, fullMatch, dynamicLabel) + } else { + fullAliasField = strings.ReplaceAll(fullAliasField, fullMatch, fmt.Sprintf(`${PROP('Dim.%s')}`, subgroup)) + } + } + result = fullAliasField return result } diff --git a/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go b/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go index 8901397f203..007a0af7015 100644 --- a/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go +++ b/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go @@ -8,12 +8,12 @@ import ( "time" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/kinds/dataquery" "github.com/grafana/kindsys" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/log/logtest" - "github.com/grafana/grafana/pkg/tsdb/cloudwatch/kinds/dataquery" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/utils" ) @@ -39,12 +39,12 @@ func TestCloudWatchQuery(t *testing.T) { MetricEditorMode: MetricEditorModeBuilder, } - deepLink, err := query.BuildDeepLink(startTime, endTime, false) + deepLink, err := query.BuildDeepLink(startTime, endTime) require.NoError(t, err) assert.Empty(t, deepLink) }) - t.Run("does not include label in case dynamic label is diabled", func(t *testing.T) { + t.Run("includes label and it's a metric stat query", func(t *testing.T) { startTime := time.Now() endTime := startTime.Add(2 * time.Hour) query := &CloudWatchQuery{ @@ -63,36 +63,12 @@ func TestCloudWatchQuery(t *testing.T) { MetricEditorMode: MetricEditorModeBuilder, } - deepLink, err := query.BuildDeepLink(startTime, endTime, false) + deepLink, err := query.BuildDeepLink(startTime, endTime) require.NoError(t, err) - assert.NotContains(t, deepLink, "label") + assert.Contains(t, deepLink, "label") }) - t.Run("includes label in case dynamic label is enabled and it's a metric stat query", func(t *testing.T) { - startTime := time.Now() - endTime := startTime.Add(2 * time.Hour) - query := &CloudWatchQuery{ - RefId: "A", - Region: "us-east-1", - Expression: "", - Statistic: "Average", - Period: 300, - Id: "id1", - MatchExact: true, - Label: "${PROP('Namespace')}", - Dimensions: map[string][]string{ - "InstanceId": {"i-12345678"}, - }, - MetricQueryType: MetricQueryTypeSearch, - MetricEditorMode: MetricEditorModeBuilder, - } - - deepLink, err := query.BuildDeepLink(startTime, endTime, false) - require.NoError(t, err) - assert.NotContains(t, deepLink, "label") - }) - - t.Run("includes label in case dynamic label is enabled and it's a math expression query", func(t *testing.T) { + t.Run("includes label and it's a math expression query", func(t *testing.T) { startTime := time.Now() endTime := startTime.Add(2 * time.Hour) query := &CloudWatchQuery{ @@ -108,9 +84,9 @@ func TestCloudWatchQuery(t *testing.T) { MetricEditorMode: MetricEditorModeRaw, } - deepLink, err := query.BuildDeepLink(startTime, endTime, false) + deepLink, err := query.BuildDeepLink(startTime, endTime) require.NoError(t, err) - assert.NotContains(t, deepLink, "label") + assert.Contains(t, deepLink, "label") }) t.Run("includes account id in case its a metric stat query and an account id is set", func(t *testing.T) { @@ -133,7 +109,7 @@ func TestCloudWatchQuery(t *testing.T) { MetricEditorMode: MetricEditorModeBuilder, } - deepLink, err := query.BuildDeepLink(startTime, endTime, false) + deepLink, err := query.BuildDeepLink(startTime, endTime) require.NoError(t, err) assert.Contains(t, deepLink, "accountId%22%3A%22123456789") }) @@ -155,7 +131,7 @@ func TestCloudWatchQuery(t *testing.T) { MetricEditorMode: MetricEditorModeRaw, } - deepLink, err := query.BuildDeepLink(startTime, endTime, false) + deepLink, err := query.BuildDeepLink(startTime, endTime) require.NoError(t, err) assert.NotContains(t, deepLink, "accountId%22%3A%22123456789") }) @@ -311,7 +287,7 @@ func TestRequestParser(t *testing.T) { }, } - migratedQueries, err := ParseMetricDataQueries(oldQuery, time.Now(), time.Now(), "us-east-2", logger, false, false) + migratedQueries, err := ParseMetricDataQueries(oldQuery, time.Now(), time.Now(), "us-east-2", logger, false) assert.NoError(t, err) require.Len(t, migratedQueries, 1) require.NotNil(t, migratedQueries[0]) @@ -342,7 +318,7 @@ func TestRequestParser(t *testing.T) { }, } - results, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) + results, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.NoError(t, err) require.Len(t, results, 1) res := results[0] @@ -385,7 +361,7 @@ func TestRequestParser(t *testing.T) { }, } - results, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) + results, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) assert.NoError(t, err) require.Len(t, results, 1) res := results[0] @@ -418,7 +394,7 @@ func TestRequestParser(t *testing.T) { }, } - _, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false, false) + _, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.Error(t, err) assert.Equal(t, `error parsing query "", failed to parse dimensions: unknown type as dimension value`, err.Error()) @@ -447,7 +423,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", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) assert.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -479,7 +455,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 60, res[0].Period) @@ -489,7 +465,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 60, res[0].Period) @@ -498,7 +474,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 300, res[0].Period) @@ -508,7 +484,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 900, res[0].Period) @@ -518,7 +494,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 3600, res[0].Period) @@ -528,7 +504,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 21600, res[0].Period) @@ -538,7 +514,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.Nil(t, err) require.Len(t, res, 1) assert.Equal(t, 21600, res[0].Period) @@ -548,7 +524,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 86400, res[0].Period) @@ -557,7 +533,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 300, res[0].Period) @@ -566,7 +542,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 3600, res[0].Period) @@ -575,7 +551,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", logger, false, false) + res, err := ParseMetricDataQueries(query, from, to, "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) assert.Equal(t, 21600, res[0].Period) @@ -590,7 +566,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", logger, false, false) + _, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.Error(t, err) assert.Equal(t, `error parsing query "", failed to parse period as duration: time: invalid duration "invalid"`, err.Error()) }) @@ -605,7 +581,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", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) assert.NoError(t, err) require.Len(t, res, 1) @@ -692,7 +668,7 @@ func Test_ParseMetricDataQueries_query_type_and_metric_editor_mode_and_GMD_query ), }, } - res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -718,7 +694,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", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -739,7 +715,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", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -760,7 +736,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", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -779,7 +755,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", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -800,7 +776,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", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -821,7 +797,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", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -844,7 +820,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", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -865,7 +841,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", logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), "us-east-2", logger, false) require.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -882,7 +858,6 @@ func Test_ParseMetricDataQueries_sets_label_when_label_is_present_in_json_query( "region":"us-east-1", "namespace":"ec2", "metricName":"CPUUtilization", - "alias":"some alias", "label":"some label", "dimensions":{"InstanceId":["test"]}, "statistic":"Average", @@ -892,11 +867,10 @@ func Test_ParseMetricDataQueries_sets_label_when_label_is_present_in_json_query( }, } - res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, true, false) + res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, false) assert.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) - assert.Equal(t, "some alias", res[0].Alias) // untouched assert.Equal(t, "some label", res[0].Label) } @@ -936,12 +910,12 @@ func Test_migrateAliasToDynamicLabel_single_query_preserves_old_alias_and_create }, } - assert.Equal(t, tc.expectedLabel, getLabel(queryToMigrate, true)) + assert.Equal(t, tc.expectedLabel, getLabel(queryToMigrate)) }) } } func Test_ParseMetricDataQueries_migrate_alias_to_label(t *testing.T) { - t.Run("migrates alias to label when label does not already exist and feature toggle enabled", func(t *testing.T) { + t.Run("migrates alias to label when label does not already exist", func(t *testing.T) { query := []backend.DataQuery{ { JSON: []byte(`{ @@ -958,13 +932,12 @@ func Test_ParseMetricDataQueries_migrate_alias_to_label(t *testing.T) { }, } - res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, true, false) + res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, false) assert.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) - assert.Equal(t, "{{period}} {{any_other_word}}", res[0].Alias) assert.Equal(t, "${PROP('Period')} ${PROP('Dim.any_other_word')}", res[0].Label) assert.Equal(t, map[string][]string{"InstanceId": {"test"}}, res[0].Dimensions) assert.Equal(t, true, res[0].ReturnData) @@ -1005,7 +978,7 @@ func Test_ParseMetricDataQueries_migrate_alias_to_label(t *testing.T) { }, } - res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, true, false) + res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, false) assert.NoError(t, err) require.Len(t, res, 2) @@ -1014,7 +987,6 @@ func Test_ParseMetricDataQueries_migrate_alias_to_label(t *testing.T) { }) require.NotNil(t, res[0]) - assert.Equal(t, "{{period}} {{any_other_word}}", res[0].Alias) assert.Equal(t, "${PROP('Period')} ${PROP('Dim.any_other_word')}", res[0].Label) assert.Equal(t, map[string][]string{"InstanceId": {"test"}}, res[0].Dimensions) assert.Equal(t, true, res[0].ReturnData) @@ -1025,7 +997,6 @@ func Test_ParseMetricDataQueries_migrate_alias_to_label(t *testing.T) { assert.Equal(t, "Average", res[0].Statistic) require.NotNil(t, res[1]) - assert.Equal(t, "{{ label }}", res[1].Alias) assert.Equal(t, "${LABEL}", res[1].Label) assert.Equal(t, map[string][]string{"InstanceId": {"test"}}, res[1].Dimensions) assert.Equal(t, true, res[1].ReturnData) @@ -1042,19 +1013,11 @@ func Test_ParseMetricDataQueries_migrate_alias_to_label(t *testing.T) { dynamicLabelsFeatureToggleEnabled bool expectedLabel string }{ - "when label already exists, feature toggle enabled": { + "when label already exists": { labelJson: `"label":"some label",`, dynamicLabelsFeatureToggleEnabled: true, - expectedLabel: "some label"}, - "when label does not exist, feature toggle is disabled": { - labelJson: "", - dynamicLabelsFeatureToggleEnabled: false, - expectedLabel: "", + expectedLabel: "some label", }, - "when label already exists, feature toggle is disabled": { - labelJson: `"label":"some label",`, - dynamicLabelsFeatureToggleEnabled: false, - expectedLabel: "some label"}, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -1074,13 +1037,12 @@ func Test_ParseMetricDataQueries_migrate_alias_to_label(t *testing.T) { }`, tc.labelJson)), }, } - res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, tc.dynamicLabelsFeatureToggleEnabled, false) + res, err := ParseMetricDataQueries(query, time.Now(), time.Now(), "us-east-2", logger, false) assert.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) - assert.Equal(t, "{{period}} {{any_other_word}}", res[0].Alias) assert.Equal(t, tc.expectedLabel, res[0].Label) assert.Equal(t, map[string][]string{"InstanceId": {"test"}}, res[0].Dimensions) assert.Equal(t, true, res[0].ReturnData) @@ -1101,7 +1063,7 @@ func Test_ParseMetricDataQueries_statistics_and_query_type_validation_and_MatchE { JSON: []byte("{}"), }, - }, time.Now(), time.Now(), "us-east-2", logger, false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false) assert.Error(t, err) assert.Equal(t, `error parsing query "", query must have either statistic or statistics field`, err.Error()) @@ -1114,7 +1076,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", logger, false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false) assert.NoError(t, err) assert.Empty(t, actual) @@ -1126,7 +1088,7 @@ func Test_ParseMetricDataQueries_statistics_and_query_type_validation_and_MatchE { JSON: []byte(`{"statistic":"Average"}`), }, - }, time.Now(), time.Now(), "us-east-2", logger, false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false) assert.NoError(t, err) assert.NotEmpty(t, actual) @@ -1138,7 +1100,7 @@ func Test_ParseMetricDataQueries_statistics_and_query_type_validation_and_MatchE { JSON: []byte(`{"statistic":"Average"}`), }, - }, time.Now(), time.Now(), "us-east-2", logger, false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false) assert.NoError(t, err) assert.Len(t, actual, 1) @@ -1152,7 +1114,7 @@ func Test_ParseMetricDataQueries_statistics_and_query_type_validation_and_MatchE { JSON: []byte(`{"statistic":"Average","matchExact":false}`), }, - }, time.Now(), time.Now(), "us-east-2", logger, false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false) assert.NoError(t, err) assert.Len(t, actual, 1) @@ -1168,7 +1130,7 @@ func Test_ParseMetricDataQueries_account_Id(t *testing.T) { { JSON: []byte(`{"accountId":"some account id", "statistic":"Average"}`), }, - }, time.Now(), time.Now(), "us-east-2", logger, false, true) + }, time.Now(), time.Now(), "us-east-2", logger, true) assert.NoError(t, err) require.Len(t, actual, 1) @@ -1183,7 +1145,7 @@ func Test_ParseMetricDataQueries_account_Id(t *testing.T) { { JSON: []byte(`{"accountId":"some account id", "statistic":"Average"}`), }, - }, time.Now(), time.Now(), "us-east-2", logger, false, false) + }, time.Now(), time.Now(), "us-east-2", logger, false) assert.NoError(t, err) require.Len(t, actual, 1) @@ -1215,7 +1177,7 @@ 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, logger, false, false) + res, err := ParseMetricDataQueries(query, time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour), region, logger, false) assert.NoError(t, err) require.Len(t, res, 1) require.NotNil(t, res[0]) @@ -1253,7 +1215,6 @@ func Test_ParseMetricDataQueries_ApplyMacros(t *testing.T) { "region":"us-east-1", "namespace":"ec2", "metricName":"CPUUtilization", - "alias":"{{period}} {{any_other_word}}", "dimensions":{"InstanceId":["test"]}, "statistic":"Average", "period":"600", @@ -1263,7 +1224,7 @@ func Test_ParseMetricDataQueries_ApplyMacros(t *testing.T) { "metricEditorMode": 1 }`), }, - }, tc.startTime, time.Now(), "us-east-1", logger, false, false) + }, tc.startTime, time.Now(), "us-east-1", logger, false) assert.NoError(t, err) assert.Equal(t, fmt.Sprintf("SEARCH('{AWS/EC2,InstanceId}', 'Average', %s)", tc.expectedPeriod), actual[0].Expression) }) @@ -1279,7 +1240,6 @@ func Test_ParseMetricDataQueries_ApplyMacros(t *testing.T) { "region":"us-east-1", "namespace":"ec2", "metricName":"CPUUtilization", - "alias":"{{period}} {{any_other_word}}", "dimensions":{"InstanceId":["test"]}, "statistic":"Average", "period":"600", @@ -1289,7 +1249,7 @@ func Test_ParseMetricDataQueries_ApplyMacros(t *testing.T) { "metricEditorMode": 1 }`), }, - }, time.Now(), time.Now(), "us-east-1", logger, false, false) + }, time.Now(), time.Now(), "us-east-1", logger, false) assert.NoError(t, err) assert.Equal(t, "SEARCH('{AWS/EC2,InstanceId}', 'Average', $__period_auto)", actual[0].Expression) }) diff --git a/pkg/tsdb/cloudwatch/response_parser.go b/pkg/tsdb/cloudwatch/response_parser.go index 34c66d5c1a2..78b7154afa7 100644 --- a/pkg/tsdb/cloudwatch/response_parser.go +++ b/pkg/tsdb/cloudwatch/response_parser.go @@ -3,14 +3,12 @@ package cloudwatch import ( "fmt" "sort" - "strconv" "strings" "time" "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/data" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" ) @@ -32,7 +30,7 @@ func (e *cloudWatchExecutor) parseResponse(startTime time.Time, endTime time.Tim } var err error - dataRes.Frames, err = buildDataFrames(startTime, endTime, response, queryRow, e.features.IsEnabled(featuremgmt.FlagCloudWatchDynamicLabels)) + dataRes.Frames, err = buildDataFrames(startTime, endTime, response, queryRow) if err != nil { return nil, err } @@ -110,12 +108,12 @@ func getLabels(cloudwatchLabel string, query *models.CloudWatchQuery) data.Label } func buildDataFrames(startTime time.Time, endTime time.Time, aggregatedResponse models.QueryRowResponse, - query *models.CloudWatchQuery, dynamicLabelEnabled bool) (data.Frames, error) { + query *models.CloudWatchQuery) (data.Frames, error) { frames := data.Frames{} for _, metric := range aggregatedResponse.Metrics { label := *metric.Label - deepLink, err := query.BuildDeepLink(startTime, endTime, dynamicLabelEnabled) + deepLink, err := query.BuildDeepLink(startTime, endTime) if err != nil { return nil, err } @@ -143,14 +141,10 @@ func buildDataFrames(startTime time.Time, endTime time.Time, aggregatedResponse timeField := data.NewField(data.TimeSeriesTimeFieldName, nil, []*time.Time{}) valueField := data.NewField(data.TimeSeriesValueFieldName, labels, []*float64{}) - frameName := label - if !dynamicLabelEnabled { - frameName = formatAlias(query, query.Statistic, labels, label) - } - valueField.SetConfig(&data.FieldConfig{DisplayNameFromDS: frameName, Links: createDataLinks(deepLink)}) + valueField.SetConfig(&data.FieldConfig{DisplayNameFromDS: label, Links: createDataLinks(deepLink)}) emptyFrame := data.Frame{ - Name: frameName, + Name: label, Fields: []*data.Field{ timeField, valueField, @@ -175,14 +169,10 @@ func buildDataFrames(startTime time.Time, endTime time.Time, aggregatedResponse timeField := data.NewField(data.TimeSeriesTimeFieldName, nil, timestamps) valueField := data.NewField(data.TimeSeriesValueFieldName, labels, points) - frameName := label - if !dynamicLabelEnabled { - frameName = formatAlias(query, query.Statistic, labels, label) - } - valueField.SetConfig(&data.FieldConfig{DisplayNameFromDS: frameName, Links: createDataLinks(deepLink)}) + valueField.SetConfig(&data.FieldConfig{DisplayNameFromDS: label, Links: createDataLinks(deepLink)}) frame := data.Frame{ - Name: frameName, + Name: label, Fields: []*data.Field{ timeField, valueField, @@ -213,66 +203,6 @@ func buildDataFrames(startTime time.Time, endTime time.Time, aggregatedResponse return frames, nil } -func formatAlias(query *models.CloudWatchQuery, stat string, dimensions map[string]string, label string) string { - region := query.Region - namespace := query.Namespace - metricName := query.MetricName - period := strconv.Itoa(query.Period) - - if query.IsUserDefinedSearchExpression() { - pIndex := strings.LastIndex(query.Expression, ",") - period = strings.Trim(query.Expression[pIndex+1:], " )") - sIndex := strings.LastIndex(query.Expression[:pIndex], ",") - stat = strings.Trim(query.Expression[sIndex+1:pIndex], " '") - } - - if len(query.Alias) == 0 && query.IsMathExpression() { - return query.Id - } - if len(query.Alias) == 0 && query.IsInferredSearchExpression() && !query.IsMultiValuedDimensionExpression() { - return label - } - if len(query.Alias) == 0 && query.MetricQueryType == models.MetricQueryTypeQuery { - return label - } - - // common fields - commonFields := map[string]string{ - "region": region, - "period": period, - } - if len(label) != 0 { - commonFields["label"] = label - } - - // since the SQL query string is not (yet) parsed, we don't know what namespace, metric, statistic and labels it's using at this point - if query.MetricQueryType != models.MetricQueryTypeQuery { - commonFields["namespace"] = namespace - commonFields["metric"] = metricName - commonFields["stat"] = stat - for k, v := range dimensions { - commonFields[k] = v - } - } - - result := aliasFormat.ReplaceAllFunc([]byte(query.Alias), func(in []byte) []byte { - labelName := strings.Replace(string(in), "{{", "", 1) - labelName = strings.Replace(labelName, "}}", "", 1) - labelName = strings.TrimSpace(labelName) - if val, exists := commonFields[labelName]; exists { - return []byte(val) - } - - return in - }) - - if string(result) == "" { - return metricName + "_" + stat - } - - return string(result) -} - func createDataLinks(link string) []data.DataLink { dataLinks := []data.DataLink{} if link != "" { diff --git a/pkg/tsdb/cloudwatch/response_parser_test.go b/pkg/tsdb/cloudwatch/response_parser_test.go index 5613268e5fd..1701bcce634 100644 --- a/pkg/tsdb/cloudwatch/response_parser_test.go +++ b/pkg/tsdb/cloudwatch/response_parser_test.go @@ -4,7 +4,6 @@ import ( "encoding/json" "os" "path/filepath" - "strings" "testing" "time" @@ -27,8 +26,6 @@ func loadGetMetricDataOutputsFromFile(filePath string) ([]*cloudwatch.GetMetricD } func TestCloudWatchResponseParser(t *testing.T) { - startTime := time.Now() - endTime := startTime.Add(2 * time.Hour) t.Run("when aggregating multi-outputs response", func(t *testing.T) { getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./testdata/multiple-outputs-query-a.json") require.NoError(t, err) @@ -137,8 +134,12 @@ func TestCloudWatchResponseParser(t *testing.T) { }) }) }) +} - t.Run("Expand dimension value using exact match", func(t *testing.T) { +func Test_buildDataFrames_uses_response_label_as_frame_name(t *testing.T) { + startTime := time.Now() + endTime := startTime.Add(2 * time.Hour) + t.Run("using exact match", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &models.QueryRowResponse{ Metrics: []*cloudwatch.MetricDataResult{ @@ -186,92 +187,28 @@ func TestCloudWatchResponseParser(t *testing.T) { }, Statistic: "Average", Period: 60, - Alias: "{{LoadBalancer}} Expanded", MetricQueryType: models.MetricQueryTypeSearch, MetricEditorMode: models.MetricEditorModeBuilder, } - frames, err := buildDataFrames(startTime, endTime, *response, query, false) + frames, err := buildDataFrames(startTime, endTime, *response, query) require.NoError(t, err) frame1 := frames[0] - assert.Equal(t, "lb1 Expanded", frame1.Name) + assert.Equal(t, "lb1", frame1.Name) assert.Equal(t, "lb1", frame1.Fields[1].Labels["LoadBalancer"]) frame2 := frames[1] - assert.Equal(t, "lb2 Expanded", frame2.Name) + assert.Equal(t, "lb2", frame2.Name) assert.Equal(t, "lb2", frame2.Fields[1].Labels["LoadBalancer"]) }) - t.Run("Expand dimension value using substring", func(t *testing.T) { - timestamp := time.Unix(0, 0) - response := &models.QueryRowResponse{ - Metrics: []*cloudwatch.MetricDataResult{ - { - Id: aws.String("id1"), - Label: aws.String("lb1 Sum"), - Timestamps: []*time.Time{ - aws.Time(timestamp), - aws.Time(timestamp.Add(time.Minute)), - aws.Time(timestamp.Add(3 * time.Minute)), - }, - Values: []*float64{ - aws.Float64(10), - aws.Float64(20), - aws.Float64(30), - }, - StatusCode: aws.String("Complete"), - }, - { - Id: aws.String("id2"), - Label: aws.String("lb2 Average"), - Timestamps: []*time.Time{ - aws.Time(timestamp), - aws.Time(timestamp.Add(time.Minute)), - aws.Time(timestamp.Add(3 * time.Minute)), - }, - Values: []*float64{ - aws.Float64(10), - aws.Float64(20), - aws.Float64(30), - }, - StatusCode: aws.String("Complete"), - }, - }} - - query := &models.CloudWatchQuery{ - RefId: "refId1", - Region: "us-east-1", - Namespace: "AWS/ApplicationELB", - MetricName: "TargetResponseTime", - Dimensions: map[string][]string{ - "LoadBalancer": {"lb1", "lb2"}, - "TargetGroup": {"tg"}, - }, - Statistic: "Average", - Period: 60, - Alias: "{{LoadBalancer}} Expanded", - MetricQueryType: models.MetricQueryTypeSearch, - MetricEditorMode: models.MetricEditorModeBuilder, - } - frames, err := buildDataFrames(startTime, endTime, *response, query, false) - require.NoError(t, err) - - frame1 := frames[0] - assert.Equal(t, "lb1 Expanded", frame1.Name) - assert.Equal(t, "lb1", frame1.Fields[1].Labels["LoadBalancer"]) - - frame2 := frames[1] - assert.Equal(t, "lb2 Expanded", frame2.Name) - assert.Equal(t, "lb2", frame2.Fields[1].Labels["LoadBalancer"]) - }) - - t.Run("Expand dimension value using wildcard", func(t *testing.T) { + t.Run("using wildcard", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &models.QueryRowResponse{ Metrics: []*cloudwatch.MetricDataResult{ { Id: aws.String("lb3"), - Label: aws.String("lb3"), + Label: aws.String("some label lb3"), Timestamps: []*time.Time{ aws.Time(timestamp), aws.Time(timestamp.Add(time.Minute)), @@ -286,7 +223,7 @@ func TestCloudWatchResponseParser(t *testing.T) { }, { Id: aws.String("lb4"), - Label: aws.String("lb4"), + Label: aws.String("some label lb4"), Timestamps: []*time.Time{ aws.Time(timestamp), aws.Time(timestamp.Add(time.Minute)), @@ -313,24 +250,23 @@ func TestCloudWatchResponseParser(t *testing.T) { }, Statistic: "Average", Period: 60, - Alias: "{{LoadBalancer}} Expanded", MetricQueryType: models.MetricQueryTypeSearch, MetricEditorMode: models.MetricEditorModeBuilder, } - frames, err := buildDataFrames(startTime, endTime, *response, query, false) + frames, err := buildDataFrames(startTime, endTime, *response, query) require.NoError(t, err) - assert.Equal(t, "lb3 Expanded", frames[0].Name) - assert.Equal(t, "lb4 Expanded", frames[1].Name) + assert.Equal(t, "some label lb3", frames[0].Name) + assert.Equal(t, "some label lb4", frames[1].Name) }) - t.Run("Expand dimension value when no values are returned and a multi-valued template variable is used", func(t *testing.T) { + t.Run("when no values are returned and a multi-valued template variable is used", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &models.QueryRowResponse{ Metrics: []*cloudwatch.MetricDataResult{ { Id: aws.String("lb3"), - Label: aws.String("lb3"), + Label: aws.String("some label"), Timestamps: []*time.Time{ aws.Time(timestamp), aws.Time(timestamp.Add(time.Minute)), @@ -351,25 +287,24 @@ func TestCloudWatchResponseParser(t *testing.T) { }, Statistic: "Average", Period: 60, - Alias: "{{LoadBalancer}} Expanded", MetricQueryType: models.MetricQueryTypeSearch, MetricEditorMode: models.MetricEditorModeBuilder, } - frames, err := buildDataFrames(startTime, endTime, *response, query, false) + frames, err := buildDataFrames(startTime, endTime, *response, query) require.NoError(t, err) assert.Len(t, frames, 2) - assert.Equal(t, "lb1 Expanded", frames[0].Name) - assert.Equal(t, "lb2 Expanded", frames[1].Name) + assert.Equal(t, "some label", frames[0].Name) + assert.Equal(t, "some label", frames[1].Name) }) - t.Run("Expand dimension value when no values are returned and a multi-valued template variable and two single-valued dimensions are used", func(t *testing.T) { + t.Run("when no values are returned and a multi-valued template variable and two single-valued dimensions are used", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &models.QueryRowResponse{ Metrics: []*cloudwatch.MetricDataResult{ { Id: aws.String("lb3"), - Label: aws.String("lb3"), + Label: aws.String("some label"), Timestamps: []*time.Time{ aws.Time(timestamp), aws.Time(timestamp.Add(time.Minute)), @@ -393,25 +328,24 @@ func TestCloudWatchResponseParser(t *testing.T) { }, Statistic: "Average", Period: 60, - Alias: "{{LoadBalancer}} Expanded {{InstanceType}} - {{Resource}}", MetricQueryType: models.MetricQueryTypeSearch, MetricEditorMode: models.MetricEditorModeBuilder, } - frames, err := buildDataFrames(startTime, endTime, *response, query, false) + frames, err := buildDataFrames(startTime, endTime, *response, query) require.NoError(t, err) assert.Len(t, frames, 2) - assert.Equal(t, "lb1 Expanded micro - res", frames[0].Name) - assert.Equal(t, "lb2 Expanded micro - res", frames[1].Name) + assert.Equal(t, "some label", frames[0].Name) + assert.Equal(t, "some label", frames[1].Name) }) - t.Run("Should only expand certain fields when using SQL queries", func(t *testing.T) { + t.Run("when using SQL queries", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &models.QueryRowResponse{ Metrics: []*cloudwatch.MetricDataResult{ { Id: aws.String("lb3"), - Label: aws.String("lb3"), + Label: aws.String("some label"), Timestamps: []*time.Time{ aws.Time(timestamp), }, @@ -433,20 +367,13 @@ func TestCloudWatchResponseParser(t *testing.T) { }, Statistic: "Average", Period: 60, - Alias: "{{LoadBalancer}} {{InstanceType}} {{metric}} {{namespace}} {{stat}} {{region}} {{period}}", MetricQueryType: models.MetricQueryTypeQuery, MetricEditorMode: models.MetricEditorModeRaw, } - frames, err := buildDataFrames(startTime, endTime, *response, query, false) + frames, err := buildDataFrames(startTime, endTime, *response, query) require.NoError(t, err) - assert.False(t, strings.Contains(frames[0].Name, "AWS/ApplicationELB")) - assert.False(t, strings.Contains(frames[0].Name, "lb1")) - assert.False(t, strings.Contains(frames[0].Name, "micro")) - assert.False(t, strings.Contains(frames[0].Name, "AWS/ApplicationELB")) - - assert.True(t, strings.Contains(frames[0].Name, "us-east-1")) - assert.True(t, strings.Contains(frames[0].Name, "60")) + assert.Equal(t, "some label", frames[0].Name) }) t.Run("Parse cloudwatch response", func(t *testing.T) { @@ -455,7 +382,7 @@ func TestCloudWatchResponseParser(t *testing.T) { Metrics: []*cloudwatch.MetricDataResult{ { Id: aws.String("id1"), - Label: aws.String("lb"), + Label: aws.String("some label"), Timestamps: []*time.Time{ aws.Time(timestamp), aws.Time(timestamp.Add(time.Minute)), @@ -482,15 +409,14 @@ func TestCloudWatchResponseParser(t *testing.T) { }, Statistic: "Average", Period: 60, - Alias: "{{namespace}}_{{metric}}_{{stat}}", MetricQueryType: models.MetricQueryTypeSearch, MetricEditorMode: models.MetricEditorModeBuilder, } - frames, err := buildDataFrames(startTime, endTime, *response, query, false) + frames, err := buildDataFrames(startTime, endTime, *response, query) require.NoError(t, err) frame := frames[0] - assert.Equal(t, "AWS/ApplicationELB_TargetResponseTime_Average", frame.Name) + assert.Equal(t, "some label", frame.Name) assert.Equal(t, "Time", frame.Fields[0].Name) assert.Equal(t, "lb", frame.Fields[1].Labels["LoadBalancer"]) assert.Equal(t, 10.0, *frame.Fields[1].At(0).(*float64)) @@ -499,23 +425,4 @@ func TestCloudWatchResponseParser(t *testing.T) { assert.Equal(t, "Value", frame.Fields[1].Name) assert.Equal(t, "", frame.Fields[1].Config.DisplayName) }) - - t.Run("buildDataFrames should use response label as frame name when dynamic label is enabled", func(t *testing.T) { - response := &models.QueryRowResponse{ - Metrics: []*cloudwatch.MetricDataResult{ - { - Label: aws.String("some response label"), - Timestamps: []*time.Time{}, - Values: []*float64{aws.Float64(10)}, - StatusCode: aws.String("Complete"), - }, - }, - } - - frames, err := buildDataFrames(startTime, endTime, *response, &models.CloudWatchQuery{}, true) - - assert.NoError(t, err) - require.Len(t, frames, 1) - assert.Equal(t, "some response label", frames[0].Name) - }) } diff --git a/pkg/tsdb/cloudwatch/time_series_query.go b/pkg/tsdb/cloudwatch/time_series_query.go index 32a839f0767..e53e7a056f9 100644 --- a/pkg/tsdb/cloudwatch/time_series_query.go +++ b/pkg/tsdb/cloudwatch/time_series_query.go @@ -37,7 +37,6 @@ func (e *cloudWatchExecutor) executeTimeSeriesQuery(ctx context.Context, logger } 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 { return nil, err diff --git a/pkg/tsdb/cloudwatch/time_series_query_test.go b/pkg/tsdb/cloudwatch/time_series_query_test.go index 5ee82a2ae99..d51a3ca5e13 100644 --- a/pkg/tsdb/cloudwatch/time_series_query_test.go +++ b/pkg/tsdb/cloudwatch/time_series_query_test.go @@ -77,7 +77,6 @@ func TestTimeSeriesQuery(t *testing.T) { }, "region": "us-east-2", "id": "a", - "alias": "NetworkOut", "statistics": [ "Maximum" ], @@ -104,7 +103,6 @@ func TestTimeSeriesQuery(t *testing.T) { }, "region": "us-east-2", "id": "b", - "alias": "NetworkIn", "statistics": [ "Maximum" ], @@ -275,7 +273,6 @@ type queryParameters struct { MetricEditorMode dataquery.CloudWatchMetricsQueryMetricEditorMode `json:"metricEditorMode"` Dimensions queryDimensions `json:"dimensions"` Expression string `json:"expression"` - Alias string `json:"alias"` Label *string `json:"label"` Statistic string `json:"statistic"` Period string `json:"period"` @@ -300,7 +297,6 @@ func newTestQuery(t testing.TB, p queryParameters) json.RawMessage { Expression string `json:"expression"` Region string `json:"region"` ID string `json:"id"` - Alias string `json:"alias"` Label *string `json:"label"` Statistic string `json:"statistic"` Period string `json:"period"` @@ -317,7 +313,6 @@ func newTestQuery(t testing.TB, p queryParameters) json.RawMessage { MetricEditorMode: p.MetricEditorMode, Dimensions: p.Dimensions, Expression: p.Expression, - Alias: p.Alias, Label: p.Label, Statistic: p.Statistic, Period: p.Period, @@ -346,10 +341,10 @@ func Test_QueryData_timeSeriesQuery_GetMetricDataWithContext(t *testing.T) { return DataSource{Settings: models.CloudWatchSettings{}}, nil }) - t.Run("passes query label as GetMetricData label when dynamic labels feature toggle is enabled", func(t *testing.T) { + t.Run("passes query label as GetMetricData label", func(t *testing.T) { api = mocks.MetricsAPI{} api.On("GetMetricDataWithContext", mock.Anything, mock.Anything, mock.Anything).Return(&cloudwatch.GetMetricDataOutput{}, nil) - executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures(featuremgmt.FlagCloudWatchDynamicLabels)) + executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) query := newTestQuery(t, queryParameters{ Label: aws.String("${PROP('Period')} some words ${PROP('Dim.InstanceId')}"), }) @@ -378,27 +373,17 @@ func Test_QueryData_timeSeriesQuery_GetMetricDataWithContext(t *testing.T) { }) testCases := map[string]struct { - feature *featuremgmt.FeatureManager parameters queryParameters }{ - "should not pass GetMetricData label when query label is empty, dynamic labels is enabled": { - feature: featuremgmt.WithFeatures(featuremgmt.FlagCloudWatchDynamicLabels), - }, - "should not pass GetMetricData label when query label is empty string, dynamic labels is enabled": { - feature: featuremgmt.WithFeatures(featuremgmt.FlagCloudWatchDynamicLabels), - parameters: queryParameters{Label: aws.String("")}, - }, - "should not pass GetMetricData label when dynamic labels is disabled": { - feature: featuremgmt.WithFeatures(), - parameters: queryParameters{Label: aws.String("${PROP('Period')} some words ${PROP('Dim.InstanceId')}")}, - }, + "should not pass GetMetricData label when query label is empty": {}, + "should not pass GetMetricData label when query label is empty string": {parameters: queryParameters{Label: aws.String("")}}, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { api = mocks.MetricsAPI{} api.On("GetMetricDataWithContext", mock.Anything, mock.Anything, mock.Anything).Return(&cloudwatch.GetMetricDataOutput{}, nil) - executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, tc.feature) + executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) _, err := executor.QueryData(context.Background(), &backend.QueryDataRequest{ PluginContext: backend.PluginContext{DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{}}, @@ -425,7 +410,7 @@ func Test_QueryData_timeSeriesQuery_GetMetricDataWithContext(t *testing.T) { } } -func Test_QueryData_response_data_frame_names(t *testing.T) { +func Test_QueryData_response_data_frame_name_is_always_response_label(t *testing.T) { origNewCWClient := NewCWClient t.Cleanup(func() { NewCWClient = origNewCWClient @@ -449,11 +434,10 @@ func Test_QueryData_response_data_frame_names(t *testing.T) { }) executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) - t.Run("where user defines search expression and alias is defined, then frame name prioritizes period and stat from expression over input", func(t *testing.T) { + t.Run("where user defines search expression", func(t *testing.T) { query := newTestQuery(t, queryParameters{ - MetricQueryType: models.MetricQueryTypeSearch, // contributes to isUserDefinedSearchExpression = true - MetricEditorMode: models.MetricEditorModeRaw, // contributes to isUserDefinedSearchExpression = true - Alias: "{{period}} {{stat}}", + MetricQueryType: models.MetricQueryTypeSearch, // contributes to isUserDefinedSearchExpression = true + MetricEditorMode: models.MetricEditorModeRaw, // contributes to isUserDefinedSearchExpression = true Expression: `SEARCH('{AWS/EC2,InstanceId} MetricName="CPUUtilization"', 'Average', 300)`, // period 300 and stat 'Average' parsed from this expression Statistic: "Maximum", // stat parsed from expression takes precedence over 'Maximum' Period: "1200", // period parsed from expression takes precedence over 1200 @@ -471,10 +455,10 @@ func Test_QueryData_response_data_frame_names(t *testing.T) { }) assert.NoError(t, err) - assert.Equal(t, "300 Average", resp.Responses["A"].Frames[0].Name) + assert.Equal(t, labelFromGetMetricData, resp.Responses["A"].Frames[0].Name) }) - t.Run("where no alias is provided and query is math expression, then frame name is queryId", func(t *testing.T) { + t.Run("where query is math expression", func(t *testing.T) { query := newTestQuery(t, queryParameters{ MetricQueryType: models.MetricQueryTypeSearch, MetricEditorMode: models.MetricEditorModeRaw, @@ -492,10 +476,10 @@ func Test_QueryData_response_data_frame_names(t *testing.T) { }) assert.NoError(t, err) - assert.Equal(t, queryId, resp.Responses["A"].Frames[0].Name) + assert.Equal(t, labelFromGetMetricData, resp.Responses["A"].Frames[0].Name) }) - t.Run("where no alias provided and query type is MetricQueryTypeQuery, then frame name is label", func(t *testing.T) { + t.Run("where query type is MetricQueryTypeQuery", func(t *testing.T) { query := newTestQuery(t, queryParameters{ MetricQueryType: models.MetricQueryTypeQuery, }) @@ -515,7 +499,7 @@ func Test_QueryData_response_data_frame_names(t *testing.T) { assert.Equal(t, labelFromGetMetricData, resp.Responses["A"].Frames[0].Name) }) - // where query is inferred search expression and not multivalued dimension expression, then frame name is label + // where query is inferred search expression and not multivalued dimension expression testCasesReturningLabel := map[string]queryParameters{ "with specific dimensions, matchExact false": {Dimensions: queryDimensions{[]string{"some-instance"}}, MatchExact: false}, "with wildcard dimensions, matchExact false": {Dimensions: queryDimensions{[]string{"*"}}, MatchExact: false}, @@ -542,7 +526,7 @@ func Test_QueryData_response_data_frame_names(t *testing.T) { }) } - // complementary test cases to above return default of "metricName_stat" + // complementary test cases to above testCasesReturningMetricStat := map[string]queryParameters{ "with specific dimensions, matchExact true": { Dimensions: queryDimensions{[]string{"some-instance"}}, @@ -585,7 +569,7 @@ func Test_QueryData_response_data_frame_names(t *testing.T) { }) assert.NoError(t, err) - assert.Equal(t, "CPUUtilization_Maximum", resp.Responses["A"].Frames[0].Name) + assert.Equal(t, labelFromGetMetricData, resp.Responses["A"].Frames[0].Name) }) } } @@ -627,7 +611,6 @@ func TestTimeSeriesQuery_CrossAccountQuerying(t *testing.T) { }, "region": "us-east-2", "id": "a", - "alias": "NetworkOut", "statistic": "Maximum", "period": "300", "hide": false, @@ -668,7 +651,6 @@ func TestTimeSeriesQuery_CrossAccountQuerying(t *testing.T) { }, "region": "us-east-2", "id": "a", - "alias": "NetworkOut", "statistic": "Maximum", "period": "300", "hide": false, @@ -710,7 +692,6 @@ func TestTimeSeriesQuery_CrossAccountQuerying(t *testing.T) { }, "region": "us-east-2", "id": "a", - "alias": "NetworkOut", "statistic": "Maximum", "period": "300", "hide": false, @@ -752,7 +733,6 @@ func TestTimeSeriesQuery_CrossAccountQuerying(t *testing.T) { }, "region": "us-east-2", "id": "a", - "alias": "NetworkOut", "statistic": "Maximum", "period": "300", "hide": false, diff --git a/public/app/plugins/datasource/cloudwatch/dataquery.cue b/public/app/plugins/datasource/cloudwatch/dataquery.cue index e4248e20b6b..5ca33d0bbff 100644 --- a/public/app/plugins/datasource/cloudwatch/dataquery.cue +++ b/public/app/plugins/datasource/cloudwatch/dataquery.cue @@ -67,7 +67,8 @@ composableKinds: DataQuery: { metricEditorMode?: #MetricEditorMode // ID can be used to reference other queries in math expressions. The ID can include numbers, letters, and underscore, and must start with a lowercase letter. id: string - // To be deprecated. Use label + // Deprecated: use label + // @deprecated use label alias?: string // Change the time series legend names using dynamic labels. See https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/graph-dynamic-labels.html for more details. label?: string diff --git a/public/app/plugins/datasource/cloudwatch/dataquery.gen.ts b/public/app/plugins/datasource/cloudwatch/dataquery.gen.ts index 32d3bc67012..c1a8751c66c 100644 --- a/public/app/plugins/datasource/cloudwatch/dataquery.gen.ts +++ b/public/app/plugins/datasource/cloudwatch/dataquery.gen.ts @@ -65,7 +65,8 @@ export type Dimensions = Record)>; */ export interface CloudWatchMetricsQuery extends common.DataQuery, MetricStat { /** - * To be deprecated. Use label + * Deprecated: use label + * @deprecated use label */ alias?: string; /**