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

(neptune-alpha): Unable to set custom retention period for multiple CloudWatch log exports #26295

Closed
kscjosep opened this issue Jul 8, 2023 · 2 comments · Fixed by #28643
Closed
Labels
@aws-cdk/aws-neptune Related Amazon Neptune bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@kscjosep
Copy link

kscjosep commented Jul 8, 2023

Describe the bug

I have created a Neptune cluster with the DatabaseCluster construct via aws-neptune-alpha. I enabled audit and slowquery logging, and I would like to export these logs to CloudWatch. However, I am unable to set a custom retention period for both sets of logs due to a bug related to string interpolation. The build will fail with the error message "Error: There is already a Construct with name [object Object]LogRetention in DatabaseCluster ...". Since the interpolated LogType object always resolves to [object Object], CDK throws errors stating that such a construct already exists when more than one log type is exported.

Expected Behavior

Users should be able to export more than one log type to CloudWatch and set a custom retention period using cloudwatchLogsExports and cloudwatchLogsRetention like so:

this.databaseCluster = new DatabaseCluster(
      this,
      `${name}`,
      {
        ...
        ...
        cloudwatchLogsExports: [new LogType('audit'), new LogType('slowquery')],
        cloudwatchLogsRetention: RetentionDays.ONE_MONTH,
      }
    )

Current Behavior

When a custom retention period is set for more than one log type exported to CloudWatch, the error There is already a Construct with name '[object Object]LogRetention' in DatabaseCluster {databaseCluster name} is thrown.

Reproduction Steps

Instantiate a new Neptune DatabaseCluster instance using aws-neptune-alpha, and enable audit logging and slow query logging. Then enable exporting these logs to CloudWatch with a custom retention period like so:

this.databaseCluster = new DatabaseCluster(
      this,
      `${name}`,
      {
        ...
        ...
        cloudwatchLogsExports: [new LogType('audit'), new LogType('slowquery')],
        cloudwatchLogsRetention: RetentionDays.ONE_MONTH,
      }
    )

Possible Solution

Interpolating the string value of the object instead of the object itself should fix the issue (ie ${logType.value}LogRetention instead of ${logType}LogRetention).

Additional Information/Context

No response

CDK CLI Version

2.87.0

Framework Version

No response

Node.js Version

v19.7.0

OS

macOS/Linux

Language

Typescript

Language Version

3.9.4

Other information

No response

@kscjosep kscjosep added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 8, 2023
@github-actions github-actions bot added the @aws-cdk/aws-neptune Related Amazon Neptune label Jul 8, 2023
@pahud pahud self-assigned this Jul 10, 2023
@pahud
Copy link
Contributor

pahud commented Jul 10, 2023

Agree. Thank you for your PR.

new logs.LogRetention(this, `${logType}LogRetention`, {

@pahud pahud added p1 effort/small Small work item – less than a day of effort labels Jul 10, 2023
@pahud pahud removed their assignment Jul 10, 2023
@pahud pahud removed the needs-triage This issue or PR still needs to be triaged. label Jul 10, 2023
@mrgrain mrgrain assigned mrgrain and unassigned mrgrain Sep 4, 2023
@mergify mergify bot closed this as completed in #28643 Jan 30, 2024
mergify bot pushed a commit that referenced this issue Jan 30, 2024
…en configuring log retention (#28643)

This PR includes a breaking change to the aws-neptune-alpha module.

I have resolved an issue where it was not possible to set multiple `cloudwatchLogsExports` when configuring log retention for a `DatabaseCluster`.

The root cause of the problem was that the LogRetention creation included `[object object]` in the construct's ID. With the current fix, the given `logType` can now be correctly included in the `LogRetention`.
```diff
- new logs.LogRetention(this, `${logType}LogRetention`, {..} // "DatabaseobjectObjectLogRetentionA247AF0C"
+ new logs.LogRetention(this, `${logType.value}LogRetention`, {..} // "DatabaseauditLogRetention8A29F5CC"
```

BREAKING CHANGE: Corrected LogRetention IDs for DatabaseCluster. Previously, regardless of the log type, the string ‘objectObject’ was always included, but after the correction, the log type is now included.


Closes #26295.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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.

SankyRed pushed a commit that referenced this issue Feb 8, 2024
…en configuring log retention (#28643)

This PR includes a breaking change to the aws-neptune-alpha module.

I have resolved an issue where it was not possible to set multiple `cloudwatchLogsExports` when configuring log retention for a `DatabaseCluster`.

The root cause of the problem was that the LogRetention creation included `[object object]` in the construct's ID. With the current fix, the given `logType` can now be correctly included in the `LogRetention`.
```diff
- new logs.LogRetention(this, `${logType}LogRetention`, {..} // "DatabaseobjectObjectLogRetentionA247AF0C"
+ new logs.LogRetention(this, `${logType.value}LogRetention`, {..} // "DatabaseauditLogRetention8A29F5CC"
```

BREAKING CHANGE: Corrected LogRetention IDs for DatabaseCluster. Previously, regardless of the log type, the string ‘objectObject’ was always included, but after the correction, the log type is now included.


Closes #26295.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-neptune Related Amazon Neptune bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
3 participants