neptune: refactor DatabaseCluster to use DatabaseInstance instead of CfnDBInstance

[humanzz]

Describe the feature

DatabaseCluster L2 construct uses the L1 CfnDBInstance as can be seen here despite there being an L2 DatabaseInstance construct.

DatabaseCluster should use DatabaseInstance and DatabaseInstance might in fact be missing some props that should also be added

Use Case

N/A

Proposed Solution

N/A

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

Environment details (OS name and version, etc.)

macOS

[kaizencc]

Hi @humanzz! Looks like two issues at play here:

• DatabaseCluster L2 uses CfnDBInstance: I don't see anything inherently wrong with this model. Generally the resources implemented by L2s are always L1 constructs.

• DatabaseInstance L2 is missing props that should be added: Anything in particular that you think is essential? As always, a missing prop is not the end of the world and users can use escape hatches to add those props in.

[humanzz]

There's indeed nothing wrong with it. I just found it a bit odd that the DatabaseCluster, instead of using an available L2 for instance - DatabaseInstance - it instead uses the L1 CfnDBInstance.

In #20248, I was suggesting additional monitoring utilities e.g. metric() methods.
Neptune metrics have different dimensions - u can get metrics for cluster, u can get metrics for instance, etc. One can think that the DatabaseCluster would expose a metric() defaulting to cluster-level metric dimensions, and potentially exposing the underlying instances with their metric() methods. The usage of L1 instance would unnecessarily complicate that.

This indeed would be a breaking change - I gave it a try - and it led to instance logical ids changing. However, I'm wondering if now would be the best time to do that given neptune's constructs are still alpha

[kaizencc]

Hi @humanzz! I notice you have a few related issues around the metrics request for the neptune L2s. One of them was closed; I'm going to open that one up again (since your PR is related to that one) and close this one.

While I don't have bandwidth to give your PR a review just yet, hopefully the correct priority labels being added on is a consolation prize :).

[github-actions]

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