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: add monitoring props and utility methods #20248

Closed
1 of 2 tasks
humanzz opened this issue May 7, 2022 · 9 comments · Fixed by #21995
Closed
1 of 2 tasks

neptune: add monitoring props and utility methods #20248

humanzz opened this issue May 7, 2022 · 9 comments · Fixed by #21995
Assignees
Labels
@aws-cdk/aws-neptune Related Amazon Neptune feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature/service-integration Add functionality to an L2 construct to enable easier integration with another service feature-request A feature should be added or improved. p1

Comments

@humanzz
Copy link
Contributor

humanzz commented May 7, 2022

Describe the feature

Update the Neptune DatabaseCluster construct to improve the monitoring experience

  1. Update props to enable configuring CloudWatch Logs
  2. Expose metricsXxx utility methods

Use Case

Improving the monitoring experience for Neptune Clusters

Proposed Solution

No response

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.21.1

Environment details (OS name and version, etc.)

macOS

@humanzz humanzz added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels May 7, 2022
@github-actions github-actions bot added the @aws-cdk/aws-neptune Related Amazon Neptune label May 7, 2022
@indrora indrora added p2 feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature/service-integration Add functionality to an L2 construct to enable easier integration with another service and removed needs-triage This issue or PR still needs to be triaged. labels Jul 19, 2022
@indrora
Copy link
Contributor

indrora commented Jul 19, 2022

Setting up logs appears to require AWS SDK access. Unless something Terribly Weird is going on with their Parameter configuration, setting up CloudWatch logs involves a custom integration added into the AWS CLI.

@humanzz
Copy link
Contributor Author

humanzz commented Sep 4, 2022

@indrora Setting up logs does not require AWS SDK access. There are 2 steps involved

  1. A parameter needs to be setup in the parameter group to enable audit logs https://docs.aws.amazon.com/neptune/latest/userguide/auditing.html#auditing-enable
  2. A config needs to be set on the cluster to enable exporting the logs to cloudwatch logs

As such

  1. a prop can be exposed to allow configuring https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-neptune-dbcluster.html#cfn-neptune-dbcluster-enablecloudwatchlogsexports on the cluster
  2. This is where things get interesting as I wonder if in the presence of the prop above, should there be any checks on the input parameter groups, or even creating a default one or should that simply be left for the construct user to do the right thing

@indrora
Copy link
Contributor

indrora commented Sep 6, 2022

Ah, I see now. Thanks.

yeah this is a gap in the CloudFormation coverage.

@indrora indrora added p1 and removed p2 labels Sep 6, 2022
@humanzz
Copy link
Contributor Author

humanzz commented Sep 8, 2022

I came across #15888 which seems to be asking for the same thing as 1 above

@Naumel
Copy link
Contributor

Naumel commented Sep 9, 2022

@humanzz Good eye!

I will close this issue in favor of the older one.

An extra 👍 on the feature request will make it easier for the team to gauge interest.

Additionally, thank you for the detailed inputs you have provided, it always appreciated!

@github-actions
Copy link

github-actions bot commented Sep 9, 2022

⚠️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.

humanzz added a commit to humanzz/aws-cdk that referenced this issue Sep 11, 2022
humanzz added a commit to humanzz/aws-cdk that referenced this issue Sep 12, 2022
- introduce LogType and CloudwatchLogsExports for use in DatabaseClusterProps
- introduce cloudwatchLogsExports prop to configure which log types should be exported to CloudWatch Logs and optionally set log retention
- update tests and integ tests
- update README

related to aws#20248
closes aws#15888
@kaizencc kaizencc reopened this Sep 13, 2022
@kaizencc
Copy link
Contributor

kaizencc commented Sep 13, 2022

Reopening this one to track the request for metricsXxx utility methods.

@humanzz
Copy link
Contributor Author

humanzz commented Sep 19, 2022

Thanks @kaizencc
I just hope someone is able to start reviewing it soon :)

@mergify mergify bot closed this as completed in #21995 Sep 22, 2022
mergify bot pushed a commit that referenced this issue Sep 22, 2022
closes #20248


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

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

mergify bot pushed a commit that referenced this issue Sep 30, 2022
- introduce LogType and CloudwatchLogsExports for use in DatabaseClusterProps
- introduce cloudwatchLogsExports prop to configure which log types should be exported to CloudWatch Logs and optionally set log retention
- update tests and integ tests
- update README

related to #20248
closes #15888


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
arewa pushed a commit to arewa/aws-cdk that referenced this issue Oct 8, 2022
- introduce LogType and CloudwatchLogsExports for use in DatabaseClusterProps
- introduce cloudwatchLogsExports prop to configure which log types should be exported to CloudWatch Logs and optionally set log retention
- update tests and integ tests
- update README

related to aws#20248
closes aws#15888


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk pushed a commit to homakk/aws-cdk that referenced this issue Dec 1, 2022
…1995)

closes aws#20248


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk pushed a commit to homakk/aws-cdk that referenced this issue Dec 1, 2022
- introduce LogType and CloudwatchLogsExports for use in DatabaseClusterProps
- introduce cloudwatchLogsExports prop to configure which log types should be exported to CloudWatch Logs and optionally set log retention
- update tests and integ tests
- update README

related to aws#20248
closes aws#15888


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*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 feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature/service-integration Add functionality to an L2 construct to enable easier integration with another service feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants