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

aws-cdk-lib/aws-rds/DatabaseInstance: Get log group of cloudwatchLogsExports #20358

Closed
2 tasks
Korbi10 opened this issue May 16, 2022 · 4 comments
Closed
2 tasks
Assignees
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@Korbi10
Copy link

Korbi10 commented May 16, 2022

Describe the feature

As software developer I would like to get the new created log group of an DatabaseInstance if the DatabaseInstance has the cloudwatchLogsExport property set, so that I can build metric filter based on that log group and I do not have to calculate the naming behavior like AWS does

Use Case

Currently in our database stack we get the log group of the DatabaseInstance by calculating the name by ourself which looks for the Metric filter Like this:

new MetricFilter(this, 'SomeID', {
logGroup: LogGroup.fromLogGroupName(this, 'SomeID', /aws/rds/instance/${this.databaseInstance.instanceIdentifier}/postgresql),
filterPattern: FilterPattern.anyTerm('ERROR', 'Caused by', 'error'),
metricName: logMetrics.metricName,
metricNamespace: logMetrics.namespace,
defaultValue: 0,
metricValue: '1',
});

this indicates, that we always knew the DatabaseInstance identifier and the used engine. Also we rely here on the naming convention of AWS. This seems dangerous to me.

Proposed Solution

The DatabaseInstance has a method or Property which contains the log group name or ARN so that we can get it in other services.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.23.0

Environment details (OS name and version, etc.)

Windows 10, Typescript

@Korbi10 Korbi10 added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels May 16, 2022
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label May 16, 2022
@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jul 28, 2022
@peterwoodworth
Copy link
Contributor

Currently you could access these log groups through escape hatches, by selecting the proper child from the DatabaseInstance.node.findChild(). I would suggest looking at the synthesized construct tree (cdk.out/tree.json) to know which log groups you want to access

We create the log groups here

protected setLogRetention() {
if (this.cloudwatchLogsExports && this.cloudwatchLogsRetention) {
for (const log of this.cloudwatchLogsExports) {
new logs.LogRetention(this, `LogRetention${log}`, {
logGroupName: `/aws/rds/instance/${this.instanceIdentifier}/${log}`,
retention: this.cloudwatchLogsRetention,
role: this.cloudwatchLogsRetentionRole,
});
}
}
}

I don't see why we couldn't expose these directly from the DatabaseInstance. Leaving this up as a good first issue for anyone to submit a PR for 🙂

@GavinZZ
Copy link
Contributor

GavinZZ commented Jan 11, 2024

I noticed that the previous PR was closed due to inactivity. Going to give it another attempt to address this issue.
Also noticed that the same issue is happening on RDS Cluster, adding log group access to cluster instance as well.

mergify bot pushed a commit that referenced this issue Jan 15, 2024
…8676)

There isn't an easy way to get the new created log group of an DatabaseInstance if the DatabaseInstance has the cloudwatchLogsExport property set. Right now users would have to manual figure out the log group name by constructing `/aws/rds/instance/${this.databaseInstance.instanceIdentifier}/postgresql`. Same applies to RDS cluster resource.

This is also dangerous as it relies on the naming convention unmodified. It would break the application if the naming convention was somehow changed.

Add a new property `cloudwatchLogGroups` to RDS instance and cluster.

Closes #<#20358>.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@GavinZZ
Copy link
Contributor

GavinZZ commented Jan 18, 2024

This is now supported (pending release), detail usage can be found in README

@GavinZZ GavinZZ closed this as completed Jan 18, 2024
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet
3 participants