Resource xray sampling rules

[dedoussis]

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

Fixes #7028

Release note for CHANGELOG:

New Resource: aws_xray_sampling_rule (#7028)

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSXraySamplingRule'

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSXraySamplingRule -timeout 120m
=== RUN   TestAccAWSXraySamplingRule_basic
=== PAUSE TestAccAWSXraySamplingRule_basic
=== RUN   TestAccAWSXraySamplingRule_update
=== PAUSE TestAccAWSXraySamplingRule_update
=== RUN   TestAccAWSXraySamplingRule_import
=== PAUSE TestAccAWSXraySamplingRule_import
=== CONT  TestAccAWSXraySamplingRule_basic
=== CONT  TestAccAWSXraySamplingRule_update
=== CONT  TestAccAWSXraySamplingRule_import
--- PASS: TestAccAWSXraySamplingRule_basic (18.06s)
--- PASS: TestAccAWSXraySamplingRule_import (19.33s)
--- PASS: TestAccAWSXraySamplingRule_update (37.13s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	37.193s

References

https://docs.aws.amazon.com/xray/latest/devguide/xray-api-sampling.html

[bflad]

Hi @dedoussis 👋 Thanks for submitting this, despite the comment count its in pretty good shape! Initial feedback below -- please let us know if you have any questions or do not have time to implement these items.

One important note about adding new services, the vendoring should be included in a separate PR as noted in the contributing guide. Rebasing this pull request on master and running go mod vendor shows the xray SDK client is out of date already.

[dedoussis]

Thanks for reviewing this @bflad ! 😃
I do have time to implement any needed fixes. Have already picked up some of your comments.

Regarding the vendoring, I do get that SDKs can get outdated but do not understand how a separate PR solves this.

[dedoussis]

@bflad Do you think this would require a custom validator as the validation helper does not support anything along the lines of FloatBetween?

[bflad]

It appears there are two upstream PRs for adding FloatBetween to the helper/validation package upstream:

• Added FloatBetween validation function. terraform#18614
• Add FloatBetween terraform#20877

I cannot imagine there is a specific reason why one of these is not merged, so I will followup with those maintainers to make sure the new validation function gets in. We are likely pulling in a newer version of the upstream dependency later this week anyways as part of some Terraform 0.12 work, so this problem might resolve itself shortly. I would suggest omitting any changes in this regard for now.

[bflad]

Regarding the vendoring, I do get that SDKs can get outdated but do not understand how a separate PR solves this.

The maintainers generally prioritize merging vendoring changes before other code reviews as they are a fast moving target with how often the AWS Go SDK is released. If the AWS Go SDK for one service is out of sync version-wise with the rest of the services, it will show as unexpected changes the next time vendoring occurs. Since the AWS Go SDK is generally additive usually this is not a problem, but there have been rare cases in the past where that did not hold. Separating the SDK client addition, e.g. as #8512 did for managedblockchain, allows us to immediately get the service SDK into the codebase.

Once Go Module caching becomes more publicly stable and we feel comfortable removing the vendoring, this will no longer be a problem. 👍

[dedoussis]

@bflad Have addressed all of your comments. 😺
Once you're happy with the state of this PR, I will open a separate one for the vendoring.

[bflad]

This looks great, @dedoussis, thanks so much for your contribution! 🚀 I will add the go mod vendor commit when merging this pull request so you don't need to worry about that little detail. 😄

Output from acceptance testing:

--- PASS: TestAccAWSXraySamplingRule_basic (10.06s)
--- PASS: TestAccAWSXraySamplingRule_update (15.27s)

[bflad]

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

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!