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

(DynamoDB): Enabling ContributorInsights (boolean flag) does not reflect in child/related resources (like GSIs and replication regions) #15080

Open
2 tasks
arnab opened this issue Jun 11, 2021 · 9 comments
Labels
@aws-cdk/aws-dynamodb Related to Amazon DynamoDB effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2

Comments

@arnab
Copy link

arnab commented Jun 11, 2021

CDK's support for DynamoDB's ContributorInsights feature is enabled via a simple boolean flag: Table#contributorInsightsEnabled.

However, turning on this single boolean field does not turn on ContributorInsights in child/related resources, such as:

Use Case

If ContributorInsights is enabled via a single boolean field (like it is right now), it yields a nice simple user-experience. But I expect it to also enable it on related resources.

Otherwise, I expect to have other ways of enabling it on such child/related resources directly, but there does not appear to be any way to do this (aside from a CustomResource).

Proposed Solution

Either:

  • Turn on ContributorInsights on all child resources (part of the Table)
  • or offer options to toggle them on per child resource (like GSI and per replicatedRegion).

Other

Original discussion and Pull-Request:


This is a 🚀 Feature Request

@arnab arnab added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jun 11, 2021
@github-actions github-actions bot added the @aws-cdk/aws-dynamodb Related to Amazon DynamoDB label Jun 11, 2021
@skinny85
Copy link
Contributor

Hey @arnab,

thanks for opening the issue.

  1. GSIs: Yes, you're probably correct. We could do it automatically if contributorInsightsEnabled, or allow passing it in GlobalSecondaryIndexProps. Which do you think is better?
  2. Replica tables: I'm afraid I don't see a way to enable contributor insights in the API call for creating the replica, so I'm not sure we can do much there.

Thanks,
Adam

@skinny85 skinny85 added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jun 11, 2021
@arnab
Copy link
Author

arnab commented Jun 15, 2021

Thanks for the response, @skinny85 !

GSIs: We could do it automatically if contributorInsightsEnabled, or allow passing it in GlobalSecondaryIndexProps. Which do you think is better?

I think you will need to make this a property of GlobalSecondaryIndexProps so it's configurable per index (like in the Console). At the same time, a sane default would be to assume that all GSIs want it turned on, if it's specified at the table-level and the GSI itself does not specify it.

@arnab
Copy link
Author

arnab commented Jun 15, 2021

Replica tables: I'm afraid I don't see a way to enable contributor insights in the API call for creating the replica, so I'm not sure we can do much there.

If this was directly modelled as a wrapper around a CloudFormation resource, it would have made sense to request it as a feature from CloudFormation. From what I get, globalTables is a CDK specific feature (i.e. not backed by a direct CF resource and instead it's a Table resource + API calls (one per region) to create replicas), right? (Re: #5821)

Since it's a different API-calls, I would expect CDK to also follow the contributorInsightsEnabled specification, after the replica-creation, per region (i.e. another API call, per region)?

Or do we need additional support from DynamoDB for it?

@skinny85
Copy link
Contributor

GSIs: We could do it automatically if contributorInsightsEnabled, or allow passing it in GlobalSecondaryIndexProps. Which do you think is better?

I think you will need to make this a property of GlobalSecondaryIndexProps so it's configurable per index (like in the Console). At the same time, a sane default would be to assume that all GSIs want it turned on, if it's specified at the table-level and the GSI itself does not specify it.

Love it! Any chance you could submit us a PR with this change @arnab? Our "Contributing" guide is here: https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md.

@skinny85
Copy link
Contributor

Replica tables: I'm afraid I don't see a way to enable contributor insights in the API call for creating the replica, so I'm not sure we can do much there.

If this was directly modelled as a wrapper around a CloudFormation resource, it would have made sense to request it as a feature from CloudFormation. From what I get, globalTables is a CDK specific feature (i.e. not backed by a direct CF resource and instead it's a Table resource + API calls (one per region) to create replicas), right? (Re: #5821)

It's implemented as a Custom Resource, yes, because CloudFormation support was not added until very, very recently.

We will need to switch to the new CloudFormation resources at some point, and deprecate the current Custom Resource-based implementation.

However, I still don't see the new resource having the option to enable ContributorInsights.

Since it's a different API-calls, I would expect CDK to also follow the contributorInsightsEnabled specification, after the replica-creation, per region (i.e. another API call, per region)?

Or do we need additional support from DynamoDB for it?

The problem is the API call that we use, which is UpdateTables, does not allow setting ContributorInsights either.

@arnab
Copy link
Author

arnab commented Jun 16, 2021

Hmmm unfortunately, I won't be able to contribute to the solution at this time :( Happy to provide any discussion/feedback if you need though.


The problem is the API call that we use, which is UpdateTables, does not allow setting ContributorInsights either.

I see. It appears that there is a different API for enabling ContributorInsights per table/index (i.e. per region), as described here:

Since CDK is making an API call to create the replica region, if the contributorInsights option is enabled, it can (should?) also make a call per table (and per table/index as applicable) to enable it at the same time?

@skinny85
Copy link
Contributor

The problem is the API call that we use, which is UpdateTables, does not allow setting ContributorInsights either.

I see. It appears that there is a different API for enabling ContributorInsights per table/index (i.e. per region), as described here:

Since CDK is making an API call to create the replica region, if the contributorInsights option is enabled, it can (should?) also make a call per table (and per table/index as applicable) to enable it at the same time?

Hard to argue with that logic 🙂.

@skinny85 skinny85 added effort/large Large work item – several weeks of effort and removed effort/small Small work item – less than a day of effort labels Jun 16, 2021
@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jun 17, 2022
@arnab
Copy link
Author

arnab commented Jun 17, 2022

It’s not resolved, so please keep it open.

@github-actions github-actions bot removed the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jun 17, 2022
@pahud pahud added p2 and removed p1 labels Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-dynamodb Related to Amazon DynamoDB effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

3 participants