Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make 'profiler' a valid CloudWatch log export type for DocumentDB. #11051

Merged
merged 2 commits into from
Jan 14, 2020
Merged

Make 'profiler' a valid CloudWatch log export type for DocumentDB. #11051

merged 2 commits into from
Jan 14, 2020

Conversation

sengi
Copy link
Contributor

@sengi sengi commented Nov 28, 2019

Enabling the profiler feature in a DocumentDB parameter group requires adding the profiler logtype to enabled_cloudwatch_logs_exports on the aws_docdb_cluster resource. Add profiler to the list of allowed values for enabled_cloudwatch_logs_exports so that this becomes possible.

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

Closes #10953

Release note for CHANGELOG:

Add support for the `profiler` CloudWatch log type in `enabled_cloudwatch_logs_exports` for DocumentDB clusters [GH-10953]

Output from acceptance testing:

$ export AWS_DEFAULT_REGION=us-west-2
$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSDocDBCluster_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSDocDBCluster_basic -timeout 120m
=== RUN   TestAccAWSDocDBCluster_basic
=== PAUSE TestAccAWSDocDBCluster_basic
=== CONT  TestAccAWSDocDBCluster_basic
--- PASS: TestAccAWSDocDBCluster_basic (147.21s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	147.264s
$ 

@ghost ghost added needs-triage Waiting for first response or review from a maintainer. size/XS Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/docdb Issues and PRs that pertain to the docdb service. labels Nov 28, 2019
@sengi
Copy link
Contributor Author

sengi commented Nov 28, 2019

Please let me know if this requires adding a test (or extending an existing one) — I'll gladly do so but could use some advice on how to go about it.

@sengi sengi marked this pull request as ready for review November 28, 2019 11:25
@sengi sengi requested a review from a team November 28, 2019 11:25
@gdavison
Copy link
Contributor

Thanks for the PR, @sengi. A test would be great for this.

You can start at https://github.com/terraform-providers/terraform-provider-aws/blob/2cff1c9143d4e3a93e203e0096a42a04bbfdf1fa/aws/resource_aws_docdb_cluster_test.go#L557, and add the "profiler" entry.

Next, add a check like https://github.com/terraform-providers/terraform-provider-aws/blob/2cff1c9143d4e3a93e203e0096a42a04bbfdf1fa/aws/resource_aws_docdb_cluster_test.go#L42 for the attribute "enabled_cloudwatch_logs_exports.1" matching "profiler".

@gdavison gdavison added enhancement Requests to existing resources that expand the functionality or scope. waiting-response Maintainers are waiting on response from community or contributor. and removed needs-triage Waiting for first response or review from a maintainer. labels Nov 28, 2019
@gdavison gdavison self-assigned this Nov 28, 2019
@ghost ghost added the tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. label Nov 29, 2019
@sengi
Copy link
Contributor Author

sengi commented Nov 29, 2019

Thanks for the detailed advice, @gdavison! I hadn't quite understood the basics of how to use the acceptance test suite before but I think I get it now.

I've updated the PR description with the test output. (I also verified that the new assertion failed before updating the fixture to match it, but for clarity I haven't included the output from that run.)

Ready for another look.

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Nov 29, 2019
@bflad bflad requested a review from gdavison January 14, 2020 15:20
Copy link
Contributor

@gdavison gdavison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

--- PASS: TestAccAWSDocDBCluster_missingUserNameCausesError (9.51s)
--- PASS: TestAccAWSDocDBClusterParameterGroup_disappears (12.97s)
--- PASS: TestAccAWSDocDBClusterParameterGroup_Description (13.32s)
--- PASS: TestAccAWSDocDBClusterParameterGroup_basic (13.39s)
--- PASS: TestAccAWSDocDBClusterParameterGroup_generatedName (15.33s)
--- PASS: TestAccAWSDocDBClusterParameterGroup_namePrefix (15.53s)
--- PASS: TestAccAWSDocDBClusterParameterGroup_Parameter (20.14s)
--- PASS: TestAccAWSDocDBClusterParameterGroup_Tags (25.53s)
--- PASS: TestAccAWSDocDBCluster_namePrefix (104.54s)
--- PASS: TestAccAWSDocDBCluster_generatedName (134.84s)
--- PASS: TestAccAWSDocDBCluster_encrypted (130.09s)
--- PASS: TestAccAWSDocDBCluster_updateCloudwatchLogsExports (135.24s)
--- PASS: TestAccAWSDocDBCluster_kmsKey (149.10s)
--- PASS: TestAccAWSDocDBCluster_basic (174.96s)
--- PASS: TestAccAWSDocDBClusterSnapshot_basic (185.31s)
--- PASS: TestAccAWSDocDBCluster_updateTags (188.06s)
--- PASS: TestAccAWSDocDBCluster_takeFinalSnapshot (195.58s)
--- PASS: TestAccAWSDocDBCluster_backupsUpdate (225.94s)
--- PASS: TestAccAWSDocDBCluster_Port (268.69s)
--- PASS: TestAccAWSDocDBClusterInstance_az (642.86s)
--- PASS: TestAccAWSDocDBClusterInstance_generatedName (664.72s)
--- PASS: TestAccAWSDocDBClusterInstance_disappears (706.67s)
--- PASS: TestAccAWSDocDBClusterInstance_namePrefix (724.40s)
--- PASS: TestAccAWSDocDBClusterInstance_kmsKey (741.75s)
--- PASS: TestAccAWSDocDBClusterInstance_basic (1324.98s)

@gdavison gdavison added this to the v2.45.0 milestone Jan 14, 2020
@gdavison gdavison merged commit d146f4e into hashicorp:master Jan 14, 2020
gdavison added a commit that referenced this pull request Jan 14, 2020
@ghost
Copy link

ghost commented Jan 17, 2020

This has been released in version 2.45.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 for triage. Thanks!

@ghost
Copy link

ghost commented Mar 27, 2020

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!

@ghost ghost locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/docdb Issues and PRs that pertain to the docdb service. size/XS Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_docdb_cluster doesn't support exporting profiler logs
2 participants