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.

Sign up for GitHub

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

Jump to bottom

feat(rds): support Database Insights for Aurora databases #32851

Merged

mergify merged 12 commits into aws:main from sakurai-ryo:rds-database-insights

Feb 7, 2025

Contributor

sakurai-ryo commented Jan 11, 2025 •

edited

Loading

Issue # (if applicable)

Closes #32680

Reason for this change

AWS announces Amazon CloudWatch Database Insights.
https://aws.amazon.com/about-aws/whats-new/2024/12/amazon-cloudwatch-database-insights

Description of changes

Database Insights has two modes: Standard and Advanced.
For Aurora databases, standard mode is enabled by default.
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbcluster.html#aws-resource-rds-dbcluster-return-values

When using advanced mode, you must enable Performance Insights and set its data retention period to 465 days.
https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Database-Insights.html

Describe any new or updated permissions being added

Nothing.

Description of how you validated changes

Unit tests and integ-tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

sakurai-ryo added 3 commits

January 11, 2025 20:07


          feat(rds): support Database Insights for Aurora databases

5de141b


          Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into rd…

629a8b1

…s-database-insights


          rm unused import

95dea11

github-actions bot added the star-contributor label

aws-cdk-automation requested a review from a team

January 11, 2025 11:11

github-actions bot added effort/medium feature-request p2 labels

codecov bot commented Jan 11, 2025 •

edited

Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.83%. Comparing base (3490d2e) to head (994fa23).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32851   +/-   ##
=======================================
  Coverage   80.83%   80.83%           
=======================================
  Files         236      236           
  Lines       14251    14251           
  Branches     2490     2490           
=======================================
  Hits        11520    11520           
  Misses       2446     2446           
  Partials      285      285

Flag	Coverage Δ
suite.unit	`80.83% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`79.56% <ø> (ø)`
packages/aws-cdk-lib/core	`82.20% <ø> (ø)`

sakurai-ryo commented

View reviewed changes

packages/aws-cdk-lib/aws-rds/lib/cluster.ts Outdated

+                  }
+                  // Database Insights is not supported for non-Aurora engines
+                  if (props.databaseInsightsMode && !props.engine.engineType.startsWith('aurora')) {

Contributor Author

sakurai-ryo Jan 11, 2025

Currently, DatabaseClusterEngine only supports Aurora engines, but AWS::RDS::DBCluster supports standard MySQL and PostgreSQL engines.
Database Insights only supports Aurora engines, so I added validation here, just in case.


          update readme

9ed3e5e

aws-cdk-automation added the pr/needs-community-review label

lpizzinidev suggested changes

View reviewed changes

Contributor

lpizzinidev left a comment

Thanks 👍 Left comments for minor adjustments

packages/aws-cdk-lib/aws-rds/lib/cluster.ts Outdated

                   if (enablePerformanceInsights && props.enablePerformanceInsights === false) {
-                    throw new Error('`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set');
+                    throw new Error('`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set, or `databaseInsightsMode` was set to `DatabaseInsightsMode.ADVANCED`');

Contributor

lpizzinidev Jan 11, 2025

Suggested change

      
                  throw new Error('`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set, or `databaseInsightsMode` was set to `DatabaseInsightsMode.ADVANCED`');
          
                  throw new Error('`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set, or `databaseInsightsMode` was set to \'${DatabaseInsightsMode.ADVANCED}\'`');

Let's use the variable value for clarity.

Contributor Author

sakurai-ryo Jan 13, 2025

packages/aws-cdk-lib/aws-rds/lib/cluster.ts Outdated

+                    && props.performanceInsightRetention
+                    && props.performanceInsightRetention !== PerformanceInsightRetention.MONTHS_15
+                  ) {
+                    throw new Error('`performanceInsightRetention` must be set to PerformanceInsightRetention.MONTHS_15 when `databaseInsightsMode` is set to DatabaseInsightsMode.ADVANCED');

Contributor

lpizzinidev Jan 11, 2025

Suggested change

      
                  throw new Error('`performanceInsightRetention` must be set to PerformanceInsightRetention.MONTHS_15 when `databaseInsightsMode` is set to DatabaseInsightsMode.ADVANCED');
          
                  throw new Error('`performanceInsightRetention` must be set to \'${PerformanceInsightRetention.MONTHS_15}\' when `databaseInsightsMode` is set to \'${DatabaseInsightsMode.ADVANCED}\'');

Same as above.

Contributor Author

sakurai-ryo Jan 13, 2025

packages/aws-cdk-lib/aws-rds/lib/cluster.ts Outdated

Comment on lines 559 to 560

		*
		* In advanced mode, Performance Insights must be enabled.

Contributor

lpizzinidev Jan 11, 2025

Suggested change

      
               *
          
               * In advanced mode, Performance Insights must be enabled.

The current implementation is enabling insights automatically when DatabaseInsightsMode.ADVANCED

Contributor Author

sakurai-ryo Jan 13, 2025

packages/aws-cdk-lib/aws-rds/lib/cluster.ts Outdated

Comment on lines 431 to 432

		* @default - 7(PerformanceInsightRetention.DEFAULT) if `enablePerformanceInsights` is set,
		* 465(PerformanceInsightRetention.MONTHS_15) if DatabaseInsightsMode.ADVANCED is set for `databaseInsightsMode`.

Contributor

lpizzinidev Jan 11, 2025

Suggested change

      
               * @default - 7(PerformanceInsightRetention.DEFAULT) if `enablePerformanceInsights` is set,
          
               * 465(PerformanceInsightRetention.MONTHS_15) if DatabaseInsightsMode.ADVANCED is set for `databaseInsightsMode`.
          
               * @default - 7 days if `enablePerformanceInsights` is set. 465 days (15 months) if `databaseInsightsMode` is set to DatabaseInsightsMode.ADVANCED.

Contributor Author

sakurai-ryo Jan 13, 2025

packages/aws-cdk-lib/aws-rds/lib/cluster.ts Outdated

+                    throw new Error('`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set, or `databaseInsightsMode` was set to `DatabaseInsightsMode.ADVANCED`');
+                  }
+                  if (props.databaseInsightsMode === DatabaseInsightsMode.ADVANCED
+                    && props.performanceInsightRetention

Contributor

lpizzinidev Jan 11, 2025

Not sure about this. Could automatically setting retention to 15 months cause unexpected billing to customers unaware of the longer period? If so, it may be better to enforce performanceInsightRetention being set manually to PerformanceInsightRetention.MONTHS_15.

Contributor Author

sakurai-ryo Jan 13, 2025

Certainly, we should avoid situations where there are unintended costs to the user.
28bab16

packages/aws-cdk-lib/aws-rds/lib/cluster.ts Outdated

+                /**
+                 * The database insights mode.
+                 *
+                 * @default - DatabaseInsightsMode.STANDARD if Amazon Aurora engine is used, otherwise not set.

Contributor

lpizzinidev Jan 11, 2025

Suggested change

      
               * @default - DatabaseInsightsMode.STANDARD if Amazon Aurora engine is used, otherwise not set.
          
               * @default - DatabaseInsightsMode.STANDARD when performance insights are enabled and Amazon Aurora engine is used, otherwise not set.

Contributor Author

sakurai-ryo Jan 13, 2025

packages/aws-cdk-lib/aws-rds/README.md Outdated

Comment on lines 1382 to 1408

+              The standard mode of Database Insights is enabled by default for Aurora databases.
+              You can enhance the monitoring of your Aurora databases by enabling the advanced mode of Database Insights.
+              See [AWS docs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Database-Insights.html) for more information about Database Insights.
+              The following example shows enabling the advanced mode of Database Insights for a clustered database.
+              ```ts
+              declare const vpc: ec2.Vpc;
+              new rds.DatabaseCluster(this, 'Database', {
+                engine: rds.DatabaseClusterEngine.AURORA,
+                vpc: vpc,
+                // If you enable the advanced mode of Database Insights,
+                // Performance Insights is enabled and the retention period of Performance Insights data is set to 465(15 months).
+                databaseInsightsMode: rds.DatabaseInsightsMode.ADVANCED,
+                writer: rds.ClusterInstance.provisioned('Writer', {
+                  instanceType: ec2.InstanceType.of(ec2.InstanceClass.R7G, ec2.InstanceSize.LARGE),
+                }),
+              });
+              ```
+              Note that settings regarding Database Insights are only possible at the cluster level.
+              ### Supported Engines
+              Database Insights supports Amazon Aurora MySQL and Amazon Aurora PostgreSQL.

Contributor

lpizzinidev Jan 11, 2025

Suggested change

      
            The standard mode of Database Insights is enabled by default for Aurora databases.
          
            You can enhance the monitoring of your Aurora databases by enabling the advanced mode of Database Insights.
          
            See [AWS docs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Database-Insights.html) for more information about Database Insights.
          
            The following example shows enabling the advanced mode of Database Insights for a clustered database.
          
            ```ts
          
            declare const vpc: ec2.Vpc;
          
            new rds.DatabaseCluster(this, 'Database', {
          
              engine: rds.DatabaseClusterEngine.AURORA,
          
              vpc: vpc,
          
              // If you enable the advanced mode of Database Insights,
          
              // Performance Insights is enabled and the retention period of Performance Insights data is set to 465(15 months).
          
              databaseInsightsMode: rds.DatabaseInsightsMode.ADVANCED,
          
              writer: rds.ClusterInstance.provisioned('Writer', {
          
                instanceType: ec2.InstanceType.of(ec2.InstanceClass.R7G, ec2.InstanceSize.LARGE),
          
              }),
          
            });
          
            ```
          
            Note that settings regarding Database Insights are only possible at the cluster level.
          
            ### Supported Engines
          
            Database Insights supports Amazon Aurora MySQL and Amazon Aurora PostgreSQL.
          
            The standard mode of Database Insights is enabled by default for Aurora databases.
          
            You can enhance the monitoring of your Aurora databases by enabling the advanced mode of Database Insights.
          
            To control Database Insights mode, use the `databaseInsightsMode` property:
          
            ```ts
          
            declare const vpc: ec2.Vpc;
          
            new rds.DatabaseCluster(this, 'Database', {
          
              engine: rds.DatabaseClusterEngine.AURORA,
          
              vpc: vpc,
          
              // If you enable the advanced mode of Database Insights,
          
              // Performance Insights is enabled and the retention period of Performance Insights data is set to 465(15 months).
          
              databaseInsightsMode: rds.DatabaseInsightsMode.ADVANCED,
          
              writer: rds.ClusterInstance.provisioned('Writer', {
          
                instanceType: ec2.InstanceType.of(ec2.InstanceClass.R7G, ec2.InstanceSize.LARGE),
          
              }),
          
            });

Note: Database Insights are only supported for Amazon Aurora MySQL and Amazon Aurora PostgreSQL clusters.

Visit CloudWatch Database Insights for more details.

Feel free to improve further 👍

Contributor Author

sakurai-ryo Jan 13, 2025

aws-cdk-automation removed the pr/needs-community-review label

sakurai-ryo added 3 commits

January 13, 2025 18:32


          chore: improve error messages and documentation for Database Insights…

056320b

… configuration


          enforce performanceInsightRetention being set manually

28bab16


          Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into rd…

138fafe

…s-database-insights

Contributor Author

sakurai-ryo commented Jan 13, 2025

@lpizzinidev
Thank you for the review.
Could you please review this PR again?

lpizzinidev approved these changes

View reviewed changes

Contributor

lpizzinidev left a comment

Thanks 👍

aws-cdk-automation added the pr/needs-maintainer-review label

github-actions bot mentioned this pull request

Monthly PR metrics report #33266

Closed

GavinZZ previously approved these changes

View reviewed changes

Contributor

GavinZZ left a comment

LGTM, thank you!

Contributor

mergify bot commented Feb 3, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Contributor

GavinZZ commented Feb 5, 2025

@sakurai-ryo can you fix the merge conflicts when available? I've approved this PR.

sakurai-ryo added 2 commits

February 6, 2025 18:57


          Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into rd…

86834bc

…s-database-insights


          move validation to new file

28074f0

mergify bot dismissed GavinZZ’s stale review

February 6, 2025 09:58

Pull request has been modified.


          fix lint

19f7d1b

Contributor Author

sakurai-ryo commented Feb 6, 2025

@mergify update

Contributor

mergify bot commented Feb 6, 2025

update

☑️ Nothing to do

#commits-behind > 0 [📌 update requirement]
-closed [📌 update requirement]
-conflict [📌 update requirement]
queue-position = -1 [📌 update requirement]

aaythapa assigned GavinZZ

GavinZZ previously approved these changes

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review label

Contributor

mergify bot commented Feb 6, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Contributor

mergify bot commented Feb 7, 2025

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

Contributor

GavinZZ commented Feb 7, 2025

@Mergifyio requeue

Contributor

mergify bot commented Feb 7, 2025

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

Contributor

mergify bot commented Feb 7, 2025

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue


          Merge branch 'main' into rds-database-insights

d0ec73c

mergify bot dismissed GavinZZ’s stale review

February 7, 2025 19:38

Pull request has been modified.

GavinZZ approved these changes

View reviewed changes

Contributor

mergify bot commented Feb 7, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).


          Merge branch 'main' into rds-database-insights

994fa23

mergify bot merged commit f6ed4be into aws:main

20 checks passed

Contributor

mergify bot commented Feb 7, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Collaborator

aws-cdk-automation commented Feb 7, 2025

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 994fa23
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

github-actions bot commented Feb 7, 2025

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

github-actions bot locked as resolved and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

effort/medium feature-request p2 star-contributor