CloudWatch: Deprecate dynamic labels feature toggle, remove support for Alias in backend #66494

fridgepoet · 2023-04-13T15:52:34Z

What is this feature?

In the backend of the CloudWatch plugin, this

Removes any check for the Dynamic Label feature toggle, meaning Dynamic Label can no longer be disabled
- setting the feature toggle off (cloudWatchDynamicLabels = false) will no longer have any effect
Removes support for the Alias field to be used directly
Still migrates Alias to Dynamic Label once: The migration will be handled on the fly, and will be propagated to the dashboard json model only when the user opens the query editor. Once the user has saved the dashboard model with the migrated label value, migration won't happen again. This also applies to Alert rules.

The change of the frontend code will be in a separate PR.
The removal of the feature toggle itself will be in a separate PR.

Why do we need this feature?

This was slated for removal in Grafana 10

Which issue(s) does this PR fix?:

Contributes to #66437

Special notes for your reviewer:
Alias has been replaced by Dynamic Label in #48434

Testing notes:

Normal queries:

Start on the main branch with the cloudWatchDynamicLabels toggle off

[feature_toggles]
cloudWatchDynamicLabels = false

(Still on main branch) Create a dashboard with a CloudWatch datasource panel
You should see the Alias field. Enter some former Alias pattern like {{period}}
Save that dashboard.
Now you can check out this PR and restart Grafana.
No matter if the feature toggle is false or true, you should see no longer see the Alias field. It should now be a Label field populated with a migrated pattern, e.g. ${PROP('Period')}
- This Label will not be saved unless the dashboard is saved.
- Alert rules with Alias will not have their Label saved unless the rule is edited (opened and viewed) and saved again.

In alerts:

Create a dashboard with a CloudWatch datasource panel.
In the dashboard JSON model, do not include a label target in the panel yet, just use an alias target like:

      "targets": [
        {
          "alias": "{{period}}",
          "datasource": {
            "type": "cloudwatch",
            "uid": "xpyRJd9Mz"
          },
          "dimensions": {},
          "expression": "",
          "id": "",
          "logGroups": [],
          "matchExact": true,
          "metricEditorMode": 0,
          "metricName": "CPUUtilization",
          "metricQueryType": 0,
          "namespace": "AWS/EC2",
          "period": "",
          "queryMode": "Metrics",
          "refId": "A",
          "region": "default",
          "sqlExpression": "",
          "statistic": "Average"
        }
      ],

This simulates a "legacy alias" query.

Create an Alert based on this query.
With this PR, regardless of cloudWatchDynamicLabels equal to true or false, we should see requests from the Alert query being made to the GetMetricData API with label. (In this example, alias would have been migrated to label equal to ${PROP('Period')})
- this assertion was made in unit tests and locally by spying on the value of label while running (e.g. added a log line, can also check it by debugging with a breakpoint right around here)

Please check that:

It works as expected from a user's perspective.
If this is a pre-GA feature, it is behind a feature toggle.
The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

github-actions · 2023-04-13T16:00:02Z

⚠️ Possible breaking changes

(Open the links below in a new tab to go to the correct steps)

grafana-data has possible breaking changes (more info)
grafana-runtime has possible breaking changes (more info)
grafana-schema has possible breaking changes (more info)

Console output
Read our guideline

github-actions · 2023-04-13T16:09:22Z

Backend code coverage report for PR #66494

Plugin	Main	PR	Difference
cloudwatch	82.7%	81.9%	-.8%

github-actions · 2023-04-13T16:09:23Z

Frontend code coverage report for PR #66494

Plugin	Main	PR	Difference
cloudwatch	82.99%	82.99%	0%

fridgepoet · 2023-04-24T13:24:54Z

pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go

-			deepLink, err := query.BuildDeepLink(startTime, endTime, false)
+			deepLink, err := query.BuildDeepLink(startTime, endTime)
 			require.NoError(t, err)
-			assert.NotContains(t, deepLink, "label")


those were written backwards from what the test name described

fridgepoet · 2023-04-24T13:25:17Z

pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go

-			deepLink, err := query.BuildDeepLink(startTime, endTime, false)
+			deepLink, err := query.BuildDeepLink(startTime, endTime)
 			require.NoError(t, err)
-			assert.NotContains(t, deepLink, "label")


those were also written backwards from what the test name described

fridgepoet · 2023-04-24T13:31:43Z

pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go

-func TestQueryJSON(t *testing.T) {
-	jsonString := []byte(`{
-		"type": "timeSeriesQuery"
-	}`)
-	var res metricsDataQuery
-	err := json.Unmarshal(jsonString, &res)
-	require.NoError(t, err)
-	assert.Equal(t, "timeSeriesQuery", res.Type)
-}
-


this should have been removed in https://github.com/grafana/grafana/pull/66420/files#diff-2a558b494e9be887c5af055ab92ddda783a45e96b5d5a8addc8d8d276bc85e96L297

fridgepoet · 2023-04-24T13:45:02Z

pkg/tsdb/cloudwatch/response_parser_test.go

 		assert.Equal(t, "lb2", frame2.Fields[1].Labels["LoadBalancer"])
 	})

-	t.Run("Expand dimension value using substring", func(t *testing.T) {


seems to be a near-duplicate of the test above: asserts and acts on the same alias string

fridgepoet · 2023-04-24T15:21:41Z

pkg/tsdb/cloudwatch/response_parser_test.go

@@ -499,23 +249,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) {


should be covered in "buildDataFrames should use response label as frame name" now

fridgepoet · 2023-04-25T14:37:00Z

pkg/tsdb/cloudwatch/response_parser_test.go


-	t.Run("Expand dimension value using exact match", func(t *testing.T) {
+func Test_buildDataFrames_uses_response_label_as_frame_name(t *testing.T) {


These tests used to document how the Alias was transformed:
#21541
#22695
I'm reluctant to remove them as they continue to document different input queries.

The thing they document now is pretty mundane -- now all of the resulting frame names come from the response "label" from the GetMetricData API, not from the input alias.

sarahzinger

Kind of basic question on my part haha sorry, if the feature is enabled by default: https://github.com/grafana/grafana/blob/main/pkg/services/featuremgmt/registry.go#L124 then how does this pr make dynamic labels enabled by default? I thought it was already done? Would it be fair to reword this as removing support for alias or something like that instead?

Otherwise makes sense to me!

fridgepoet · 2023-04-26T09:54:30Z

Good point, thanks @sarahzinger. I've removed the focus from making the dynamic labels default since you're right that was already the case.
What's happening here is that even if someone sets the feature toggle off (cloudWatchDynamicLabels = false), our plan is that this will no longer have any effect.

(Since the code will still do a one-time Alias->Label migration, I wasn't able to truly remove Alias from the code. I am working on getting the deprecated comment in.)

sarahzinger · 2023-04-26T12:32:27Z

sounds good maybe the title change can be something like Cloudwatch: Deprecate Alias Feature Toggle to make it easier for people to spot if they were relying on it? Totally agree we should keep the migration!

fridgepoet · 2023-04-26T17:31:50Z

@sarahzinger I've created an issue to deprecate Alias and its migration here #67315, let me know what you think. I guess it can be in Grafana 11?

fridgepoet force-pushed the remove-alias branch from a1467bb to 907861d Compare April 20, 2023 16:59

grafanabot added datasource/CloudWatch area/backend labels Apr 20, 2023

fridgepoet force-pushed the remove-alias branch from 907861d to d711d91 Compare April 20, 2023 17:02

grafanabot added the area/frontend label Apr 20, 2023

Remove alias and use of dynamic labels feature flag

6387dc4

fridgepoet force-pushed the remove-alias branch from d711d91 to 6387dc4 Compare April 20, 2023 17:05

grafanabot added the levitate breaking change A label indicating a breaking change and assigned by Levitate. label Apr 20, 2023

fridgepoet changed the title ~~CloudWatch: Remove alias and cloudWatchDynamicLabels feature toggle~~ CloudWatch: Remove alias and check for feature boolean Apr 21, 2023

fridgepoet changed the title ~~CloudWatch: Remove alias and check for feature boolean~~ CloudWatch: Remove alias, making dynamic labels enabled by default Apr 21, 2023

fridgepoet added 9 commits April 21, 2023 15:25

go fmt

cb24d1e

Remove unused

8d6a789

Remove tests

9048b5f

Redo a few label strings and remove more references to alias

96e56f9

wip

5cf9f80

Put back tests

f4494b1

Merge remote-tracking branch 'origin/main' into remove-alias

80f26d6

touch up tests

3167b2e

Rename test

d5f9d4f

fridgepoet commented Apr 25, 2023

View reviewed changes

fridgepoet added no-backport Skip backport of PR add to changelog labels Apr 25, 2023

fridgepoet mentioned this pull request Apr 25, 2023

CloudWatch: Remove cloudWatchDynamicLabels feature toggle, deprecate Alias in favor of Label (dynamic label) #66437

Closed

4 tasks

fridgepoet marked this pull request as ready for review April 25, 2023 14:39

fridgepoet requested a review from a team as a code owner April 25, 2023 14:39

fridgepoet requested review from sarahzinger and removed request for a team April 25, 2023 14:39

fridgepoet requested a review from idastambuk April 25, 2023 14:39

fridgepoet changed the title ~~CloudWatch: Remove alias, making dynamic labels enabled by default~~ CloudWatch: Remove alias, making dynamic labels enabled by default in the backend Apr 25, 2023

fridgepoet added this to the 10.0.0 milestone Apr 25, 2023

sarahzinger modified the milestones: 10.0.0, 10.0.x Apr 25, 2023

fridgepoet marked this pull request as draft April 25, 2023 16:27

Put back alias migration

92fc91f

fridgepoet marked this pull request as ready for review April 25, 2023 16:49

sarahzinger approved these changes Apr 25, 2023

View reviewed changes

fridgepoet changed the title ~~CloudWatch: Remove alias, making dynamic labels enabled by default in the backend~~ CloudWatch: Remove any check for dynamic labels feature toggle Apr 26, 2023

Fix linter

f60e63f

fridgepoet changed the title ~~CloudWatch: Remove any check for dynamic labels feature toggle~~ CloudWatch: Remove any check for dynamic labels feature toggle, remove support for Alias field to be used directly Apr 26, 2023

fridgepoet changed the title ~~CloudWatch: Remove any check for dynamic labels feature toggle, remove support for Alias field to be used directly~~ CloudWatch: Start deprecating dynamic labels feature toggle, remove support for Alias Apr 26, 2023

fridgepoet changed the title ~~CloudWatch: Start deprecating dynamic labels feature toggle, remove support for Alias~~ CloudWatch: Deprecate dynamic labels feature toggle, remove support for Alias in backend Apr 26, 2023

fridgepoet added 4 commits April 26, 2023 16:32

Add deprecation to cue

2f6a4d8

Remove Alias from internal CloudWatchQuery object

762d5f7

Fix lint

a856ce6

add nolint to deprecated Alias usage

48ff872

fridgepoet mentioned this pull request Apr 26, 2023

CloudWatch: Deprecate Alias and its migration to label #67315

Closed

fridgepoet mentioned this pull request Apr 26, 2023

CloudWatch: Deprecate dynamic labels feature toggle, remove support for Alias in frontend #67222

Merged

3 tasks

fridgepoet merged commit e17496d into main Apr 27, 2023

fridgepoet deleted the remove-alias branch April 27, 2023 09:19

fridgepoet mentioned this pull request Apr 27, 2023

CloudWatch: Remove dynamic labels feature toggle #67371

Merged

3 tasks

zerok modified the milestones: 10.0.x, 10.0.0 May 2, 2023

zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CloudWatch: Deprecate dynamic labels feature toggle, remove support for Alias in backend #66494

CloudWatch: Deprecate dynamic labels feature toggle, remove support for Alias in backend #66494

fridgepoet commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

fridgepoet Apr 24, 2023

fridgepoet Apr 24, 2023

fridgepoet Apr 24, 2023

fridgepoet Apr 24, 2023

fridgepoet Apr 24, 2023

fridgepoet Apr 25, 2023

sarahzinger left a comment

fridgepoet commented Apr 26, 2023 •

edited

Loading

sarahzinger commented Apr 26, 2023

fridgepoet commented Apr 26, 2023


		t.Run("Expand dimension value using exact match", func(t *testing.T) {
		func Test_buildDataFrames_uses_response_label_as_frame_name(t *testing.T) {

CloudWatch: Deprecate dynamic labels feature toggle, remove support for Alias in backend #66494

CloudWatch: Deprecate dynamic labels feature toggle, remove support for Alias in backend #66494

Conversation

fridgepoet commented Apr 13, 2023 • edited Loading

github-actions bot commented Apr 13, 2023 • edited Loading

github-actions bot commented Apr 13, 2023 • edited Loading

github-actions bot commented Apr 13, 2023 • edited Loading

fridgepoet Apr 24, 2023

Choose a reason for hiding this comment

fridgepoet Apr 24, 2023

Choose a reason for hiding this comment

fridgepoet Apr 24, 2023

Choose a reason for hiding this comment

fridgepoet Apr 24, 2023

Choose a reason for hiding this comment

fridgepoet Apr 24, 2023

Choose a reason for hiding this comment

fridgepoet Apr 25, 2023

Choose a reason for hiding this comment

sarahzinger left a comment

Choose a reason for hiding this comment

fridgepoet commented Apr 26, 2023 • edited Loading

sarahzinger commented Apr 26, 2023

fridgepoet commented Apr 26, 2023

fridgepoet commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

fridgepoet commented Apr 26, 2023 •

edited

Loading