Support customized metric specification in autoscaling predictive scaling policy #22451

gypdtc · 2022-01-06T23:07:36Z

Proposed change to support custom metrics in predictive scaling policy.

I am new to Terraform/Golang so I basically tried to following existing pattern implementing the change and adding tests. Please let me know if it complies with your requirement.

Solved by using TypeSet rather than TypeList for dimensions
~~One question is, I used one single dimension in my test,~~

					dimensions {
						name = "foo"
						value = "bar"
					}

if I tried to use two dimensions in the test,

					dimensions {
						name = "foo"
						value = "bar"
					}
					dimensions {
						name = "bar"
						value = "foo"
					}

it seems the test would fail because at some step, the order of those two dimensions changed. I wonder is this just a limitation from testing or it indicates some issues in the implementation ?

  policy_test.go:153: Step 1/2 error: After applying this test step, the plan was not empty.
        stdout:


        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place

        Terraform will perform the following actions:

          # aws_autoscaling_policy.test will be updated in-place
          ~ resource "aws_autoscaling_policy" "test" {
                id                        = "terraform-testacc-asp1-7870250811206468064-policy_predictive"
                name                      = "terraform-testacc-asp1-7870250811206468064-policy_predictive"
                # (6 unchanged attributes hidden)

              ~ predictive_scaling_configuration {
                    # (4 unchanged attributes hidden)

                  ~ metric_specification {
                        # (1 unchanged attribute hidden)

                      ~ customized_capacity_metric_specification {
                          ~ metric_data_queries {
                                id          = "capacity_sum"
                                # (1 unchanged attribute hidden)

                              ~ metric_stat {
                                    # (2 unchanged attributes hidden)

                                  ~ metric {
                                        # (2 unchanged attributes hidden)

                                      ~ dimensions {
                                          ~ name  = "bar" -> "foo"
                                          ~ value = "foo" -> "bar"
                                        }
                                      ~ dimensions {
                                          ~ name  = "foo" -> "bar"
                                          ~ value = "bar" -> "foo"
                                        }
                                    }
                                }
                            }
                            # (2 unchanged blocks hidden)
                        }


                        # (2 unchanged blocks hidden)
                    }
                }
            }

        Plan: 0 to add, 1 to change, 0 to destroy.

Another thing I noticed is the resource label field is required in Terraform all the time,

"resource_label": {
	Type:     schema.TypeString,
	Required: true,
},

where it is only required when PredefinedMetricType is ALBTargetGroupRequestCount in AWS. Just want to check if this discrepancy is by design.

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #22512.

Output from acceptance testing:

$ make testacc TESTS=TestAccAutoScalingPolicy_predictiveScaling PKG=autoscaling

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/autoscaling/... -v -count 1 -parallel 20 -run='TestAccAutoScalingPolicy_predictiveScaling' -timeout 180m
=== RUN   TestAccAutoScalingPolicy_predictiveScaling_Predefined
=== PAUSE TestAccAutoScalingPolicy_predictiveScaling_Predefined
=== RUN   TestAccAutoScalingPolicy_predictiveScaling_Custom
=== PAUSE TestAccAutoScalingPolicy_predictiveScaling_Custom
=== RUN   TestAccAutoScalingPolicy_predictiveScalingRemoved
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingRemoved
=== RUN   TestAccAutoScalingPolicy_predictiveScalingUpdated
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingUpdated
=== CONT  TestAccAutoScalingPolicy_predictiveScaling_Predefined
=== CONT  TestAccAutoScalingPolicy_predictiveScalingRemoved
=== CONT  TestAccAutoScalingPolicy_predictiveScaling_Custom
=== CONT  TestAccAutoScalingPolicy_predictiveScalingUpdated
--- PASS: TestAccAutoScalingPolicy_predictiveScaling_Predefined (52.07s)
--- PASS: TestAccAutoScalingPolicy_predictiveScaling_Custom (52.68s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingUpdated (76.47s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingRemoved (82.27s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/autoscaling	85.262s

…ing policy.

github-actions

Welcome @gypdtc 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

GlennChia · 2022-01-12T14:18:57Z

In addition to the resource file and the test file, other files need to be edited as well

The documentation file at https://github.com/hashicorp/terraform-provider-aws/blob/main/website/docs/r/autoscaling_policy.html.markdown needs to changed with the new arguments supported, their descriptions, and since this is quite a large change, adding an example to that document will also be good.
A changelog should be made to describe those changes. This change log should be named <pr number>.txt. An example is https://github.com/hashicorp/terraform-provider-aws/blob/main/.changelog/19693.txt where a new argument was added to an existing resource, similar to what is done in this PR

gypdtc · 2022-01-19T23:30:17Z

In addition to the resource file and the test file, other files need to be edited as well

The documentation file at https://github.com/hashicorp/terraform-provider-aws/blob/main/website/docs/r/autoscaling_policy.html.markdown needs to changed with the new arguments supported, their descriptions, and since this is quite a large change, adding an example to that document will also be good.

A changelog should be made to describe those changes. This change log should be named <pr number>.txt. An example is https://github.com/hashicorp/terraform-provider-aws/blob/main/.changelog/19693.txt where a new argument was added to an existing resource, similar to what is done in this PR

Hi Glenn, I have added another commit to address those questions, however, I could not find your later comment anywhere. If you could share a link that will be super helpful, thanks!

internal/service/autoscaling/policy_test.go

internal/service/autoscaling/policy.go

internal/service/autoscaling/policy_test.go

website/docs/r/autoscaling_policy.html.markdown

internal/service/autoscaling/policy.go

internal/service/autoscaling/policy_test.go

website/docs/r/autoscaling_policy.html.markdown

gypdtc · 2022-02-03T23:48:06Z

Latest ACC test output:

make testacc TESTS=TestAccAutoScalingPolicy PKG=autoscaling

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/autoscaling/... -v -count 1 -parallel 20 -run='TestAccAutoScalingPolicy' -timeout 180m
=== RUN   TestAccAutoScalingPolicy_basic
=== PAUSE TestAccAutoScalingPolicy_basic
=== RUN   TestAccAutoScalingPolicy_predictiveScalingPredefined
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingPredefined
=== RUN   TestAccAutoScalingPolicy_predictiveScalingCustom
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingCustom
=== RUN   TestAccAutoScalingPolicy_predictiveScalingRemoved
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingRemoved
=== RUN   TestAccAutoScalingPolicy_predictiveScalingUpdated
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingUpdated
=== RUN   TestAccAutoScalingPolicy_disappears
=== PAUSE TestAccAutoScalingPolicy_disappears
=== RUN   TestAccAutoScalingPolicy_simpleScalingStepAdjustment
=== PAUSE TestAccAutoScalingPolicy_simpleScalingStepAdjustment
=== RUN   TestAccAutoScalingPolicy_TargetTrack_predefined
=== PAUSE TestAccAutoScalingPolicy_TargetTrack_predefined
=== RUN   TestAccAutoScalingPolicy_TargetTrack_custom
=== PAUSE TestAccAutoScalingPolicy_TargetTrack_custom
=== RUN   TestAccAutoScalingPolicy_zeroValue
=== PAUSE TestAccAutoScalingPolicy_zeroValue
=== CONT  TestAccAutoScalingPolicy_basic
=== CONT  TestAccAutoScalingPolicy_disappears
=== CONT  TestAccAutoScalingPolicy_TargetTrack_custom
=== CONT  TestAccAutoScalingPolicy_simpleScalingStepAdjustment
=== CONT  TestAccAutoScalingPolicy_TargetTrack_predefined
=== CONT  TestAccAutoScalingPolicy_predictiveScalingCustom
=== CONT  TestAccAutoScalingPolicy_predictiveScalingUpdated
=== CONT  TestAccAutoScalingPolicy_zeroValue
=== CONT  TestAccAutoScalingPolicy_predictiveScalingPredefined
=== CONT  TestAccAutoScalingPolicy_predictiveScalingRemoved
--- PASS: TestAccAutoScalingPolicy_predictiveScalingPredefined (52.93s)
--- PASS: TestAccAutoScalingPolicy_disappears (61.53s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingCustom (61.89s)
--- PASS: TestAccAutoScalingPolicy_simpleScalingStepAdjustment (63.18s)
--- PASS: TestAccAutoScalingPolicy_TargetTrack_custom (65.42s)
--- PASS: TestAccAutoScalingPolicy_TargetTrack_predefined (67.97s)
--- PASS: TestAccAutoScalingPolicy_zeroValue (71.84s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingUpdated (82.08s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingRemoved (85.29s)
--- PASS: TestAccAutoScalingPolicy_basic (100.89s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/autoscaling	104.390s

gypdtc · 2022-02-04T04:33:45Z

ACC output:

make testacc TESTS=TestAccAutoScalingPolicy PKG=autoscaling

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/autoscaling/... -v -count 1 -parallel 20 -run='TestAccAutoScalingPolicy' -timeout 180m
=== RUN   TestAccAutoScalingPolicy_basic
=== PAUSE TestAccAutoScalingPolicy_basic
=== RUN   TestAccAutoScalingPolicy_predictiveScalingPredefined
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingPredefined
=== RUN   TestAccAutoScalingPolicy_predictiveScalingCustom
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingCustom
=== RUN   TestAccAutoScalingPolicy_predictiveScalingRemoved
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingRemoved
=== RUN   TestAccAutoScalingPolicy_predictiveScalingUpdated
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingUpdated
=== RUN   TestAccAutoScalingPolicy_disappears
=== PAUSE TestAccAutoScalingPolicy_disappears
=== RUN   TestAccAutoScalingPolicy_simpleScalingStepAdjustment
=== PAUSE TestAccAutoScalingPolicy_simpleScalingStepAdjustment
=== RUN   TestAccAutoScalingPolicy_TargetTrack_predefined
=== PAUSE TestAccAutoScalingPolicy_TargetTrack_predefined
=== RUN   TestAccAutoScalingPolicy_TargetTrack_custom
=== PAUSE TestAccAutoScalingPolicy_TargetTrack_custom
=== RUN   TestAccAutoScalingPolicy_zeroValue
=== PAUSE TestAccAutoScalingPolicy_zeroValue
=== CONT  TestAccAutoScalingPolicy_basic
=== CONT  TestAccAutoScalingPolicy_disappears
=== CONT  TestAccAutoScalingPolicy_predictiveScalingCustom
=== CONT  TestAccAutoScalingPolicy_simpleScalingStepAdjustment
=== CONT  TestAccAutoScalingPolicy_predictiveScalingRemoved
=== CONT  TestAccAutoScalingPolicy_predictiveScalingUpdated
=== CONT  TestAccAutoScalingPolicy_zeroValue
=== CONT  TestAccAutoScalingPolicy_TargetTrack_custom
=== CONT  TestAccAutoScalingPolicy_TargetTrack_predefined
=== CONT  TestAccAutoScalingPolicy_predictiveScalingPredefined
--- PASS: TestAccAutoScalingPolicy_TargetTrack_custom (51.66s)
--- PASS: TestAccAutoScalingPolicy_simpleScalingStepAdjustment (52.26s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingPredefined (52.41s)
--- PASS: TestAccAutoScalingPolicy_TargetTrack_predefined (54.40s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingCustom (55.23s)
--- PASS: TestAccAutoScalingPolicy_zeroValue (68.69s)
--- PASS: TestAccAutoScalingPolicy_disappears (70.61s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingUpdated (82.13s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingRemoved (84.10s)
--- PASS: TestAccAutoScalingPolicy_basic (97.19s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/autoscaling	100.174s

GlennChia · 2022-02-04T06:38:37Z

Ready for your review @AdamTylerLynch. There are some design decisions made to align with the current iteration of the public documentation. When the public doc is updated, the docs here can be updated as well. The author is from the service team and is aware of this.

AdamTylerLynch

This is a really strong first pull request! Wow! Thank you so much for your contribution.

I made some suggestion relating to readability and consistency.

I would also like to see another (or more predictive examples) that could inspire customers.

internal/service/autoscaling/policy.go

internal/service/autoscaling/policy_test.go

website/docs/r/autoscaling_policy.html.markdown

internal/service/autoscaling/policy.go

gypdtc · 2022-02-05T00:28:19Z

Acc test output:

make testacc TESTS=TestAccAutoScalingPolicy PKG=autoscaling

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/autoscaling/... -v -count 1 -parallel 20 -run='TestAccAutoScalingPolicy' -timeout 180m
=== RUN   TestAccAutoScalingPolicy_basic
=== PAUSE TestAccAutoScalingPolicy_basic
=== RUN   TestAccAutoScalingPolicy_predictiveScalingPredefined
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingPredefined
=== RUN   TestAccAutoScalingPolicy_predictiveScalingCustom
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingCustom
=== RUN   TestAccAutoScalingPolicy_predictiveScalingRemoved
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingRemoved
=== RUN   TestAccAutoScalingPolicy_predictiveScalingUpdated
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingUpdated
=== RUN   TestAccAutoScalingPolicy_disappears
=== PAUSE TestAccAutoScalingPolicy_disappears
=== RUN   TestAccAutoScalingPolicy_simpleScalingStepAdjustment
=== PAUSE TestAccAutoScalingPolicy_simpleScalingStepAdjustment
=== RUN   TestAccAutoScalingPolicy_TargetTrack_predefined
=== PAUSE TestAccAutoScalingPolicy_TargetTrack_predefined
=== RUN   TestAccAutoScalingPolicy_TargetTrack_custom
=== PAUSE TestAccAutoScalingPolicy_TargetTrack_custom
=== RUN   TestAccAutoScalingPolicy_zeroValue
=== PAUSE TestAccAutoScalingPolicy_zeroValue
=== CONT  TestAccAutoScalingPolicy_basic
=== CONT  TestAccAutoScalingPolicy_disappears
=== CONT  TestAccAutoScalingPolicy_predictiveScalingUpdated
=== CONT  TestAccAutoScalingPolicy_zeroValue
=== CONT  TestAccAutoScalingPolicy_predictiveScalingPredefined
=== CONT  TestAccAutoScalingPolicy_predictiveScalingRemoved
=== CONT  TestAccAutoScalingPolicy_TargetTrack_custom
=== CONT  TestAccAutoScalingPolicy_simpleScalingStepAdjustment
=== CONT  TestAccAutoScalingPolicy_predictiveScalingCustom
=== CONT  TestAccAutoScalingPolicy_TargetTrack_predefined
--- PASS: TestAccAutoScalingPolicy_simpleScalingStepAdjustment (47.69s)
--- PASS: TestAccAutoScalingPolicy_TargetTrack_predefined (48.24s)
--- PASS: TestAccAutoScalingPolicy_TargetTrack_custom (48.44s)
--- PASS: TestAccAutoScalingPolicy_zeroValue (48.69s)
--- PASS: TestAccAutoScalingPolicy_disappears (52.22s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingPredefined (64.59s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingCustom (64.80s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingUpdated (72.79s)
--- PASS: TestAccAutoScalingPolicy_basic (80.20s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingRemoved (84.92s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/autoscaling	87.969s

AdamTylerLynch

LGTM 🚀

Requesting Hashi review.

ewbankkit

LGTM 🚀.

% make testacc TESTS=TestAccAutoScalingPolicy_predictiveScaling PKG=autoscaling
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/autoscaling/... -v -count 1 -parallel 20 -run='TestAccAutoScalingPolicy_predictiveScaling' -timeout 180m
=== RUN   TestAccAutoScalingPolicy_predictiveScalingPredefined
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingPredefined
=== RUN   TestAccAutoScalingPolicy_predictiveScalingCustom
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingCustom
=== RUN   TestAccAutoScalingPolicy_predictiveScalingRemoved
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingRemoved
=== RUN   TestAccAutoScalingPolicy_predictiveScalingUpdated
=== PAUSE TestAccAutoScalingPolicy_predictiveScalingUpdated
=== CONT  TestAccAutoScalingPolicy_predictiveScalingPredefined
=== CONT  TestAccAutoScalingPolicy_predictiveScalingRemoved
=== CONT  TestAccAutoScalingPolicy_predictiveScalingUpdated
=== CONT  TestAccAutoScalingPolicy_predictiveScalingCustom
--- PASS: TestAccAutoScalingPolicy_predictiveScalingCustom (66.73s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingPredefined (68.31s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingUpdated (106.13s)
--- PASS: TestAccAutoScalingPolicy_predictiveScalingRemoved (110.55s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/autoscaling	116.006s

ewbankkit · 2022-03-03T13:58:29Z

@gypdtc Thanks for the contribution 🎉 👏.

github-actions · 2022-03-04T22:33:42Z

This functionality has been released in v4.4.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2022-05-12T02:32:02Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

Support cusomized metric specification in autoscaling predictive scal…

5a6c084

…ing policy.

github-actions bot reviewed Jan 6, 2022

View reviewed changes

gypdtc mentioned this pull request Jan 10, 2022

Support Custom metric specification in predictive scaling policy #22512

Closed

Add change log and website doc

41bb67c

github-actions bot added the documentation Introduces or discusses updates to documentation. label Jan 19, 2022

GlennChia suggested changes Jan 20, 2022

View reviewed changes

Add attribute validation, refactor namings

1a300d5

GlennChia suggested changes Jan 29, 2022

View reviewed changes

Address comments to fix styling issues

9d92857

make attribute assignment in alphabetical order

11e47e5

AdamTylerLynch suggested changes Feb 4, 2022

View reviewed changes

Use method to generate metric data query schema, add examples

a4e4089

AdamTylerLynch approved these changes Feb 9, 2022

View reviewed changes

gypdtc changed the title ~~[WIP]Support cusomized metric specification in autoscaling predictive scal…~~ Support cusomized metric specification in autoscaling predictive scal… Feb 9, 2022

gypdtc marked this pull request as ready for review February 9, 2022 20:40

ewbankkit changed the title ~~Support cusomized metric specification in autoscaling predictive scal…~~ Support cusomized metric specification in autoscaling predictive scaling policy Mar 3, 2022

ewbankkit added 3 commits March 3, 2022 08:41

Tweak CHANGELOG entry.

4e9bdc7

Fix markdown-lint errors.

7e7d3ab

Fix terrafmt errors in documentation.

44820ce

ewbankkit approved these changes Mar 3, 2022

View reviewed changes

ewbankkit changed the title ~~Support cusomized metric specification in autoscaling predictive scaling policy~~ Support customized metric specification in autoscaling predictive scaling policy Mar 3, 2022

ewbankkit merged commit 9e07041 into hashicorp:main Mar 3, 2022

github-actions bot added this to the v4.4.0 milestone Mar 3, 2022

github-actions bot locked as resolved and limited conversation to collaborators May 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support customized metric specification in autoscaling predictive scaling policy #22451

Support customized metric specification in autoscaling predictive scaling policy #22451

gypdtc commented Jan 6, 2022 •

edited by ewbankkit

Loading

github-actions bot left a comment

GlennChia commented Jan 12, 2022 •

edited

Loading

gypdtc commented Jan 19, 2022

gypdtc commented Feb 3, 2022

gypdtc commented Feb 4, 2022

GlennChia commented Feb 4, 2022

AdamTylerLynch left a comment

gypdtc commented Feb 5, 2022

AdamTylerLynch left a comment

ewbankkit left a comment

ewbankkit commented Mar 3, 2022

github-actions bot commented Mar 4, 2022

github-actions bot commented May 12, 2022

Support customized metric specification in autoscaling predictive scaling policy #22451

Support customized metric specification in autoscaling predictive scaling policy #22451

Conversation

gypdtc commented Jan 6, 2022 • edited by ewbankkit Loading

Community Note

github-actions bot left a comment

Choose a reason for hiding this comment

GlennChia commented Jan 12, 2022 • edited Loading

gypdtc commented Jan 19, 2022

gypdtc commented Feb 3, 2022

gypdtc commented Feb 4, 2022

GlennChia commented Feb 4, 2022

AdamTylerLynch left a comment

Choose a reason for hiding this comment

gypdtc commented Feb 5, 2022

AdamTylerLynch left a comment

Choose a reason for hiding this comment

ewbankkit left a comment

Choose a reason for hiding this comment

ewbankkit commented Mar 3, 2022

github-actions bot commented Mar 4, 2022

github-actions bot commented May 12, 2022

gypdtc commented Jan 6, 2022 •

edited by ewbankkit

Loading

GlennChia commented Jan 12, 2022 •

edited

Loading