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

Improvement: Use of BatchGetSecretValue to lower API calls #95

Open
to11mtm opened this issue Dec 18, 2024 · 6 comments
Open

Improvement: Use of BatchGetSecretValue to lower API calls #95

to11mtm opened this issue Dec 18, 2024 · 6 comments

Comments

@to11mtm
Copy link

to11mtm commented Dec 18, 2024

Basically, observation is that we use GetSecretValue in SecretManagerConfigurationProvider

Thought being, we could instead use BatchGetSecretValue API to retrieve up to 20 secrets at a time.

Biggest benefit would be to library consumers who have refresh set up and have many secrets, as it should lower the overall number of API calls and thus cost (also, hopefully would help with startup and refresh times.)

Note: Some consideration might be needed around how the API works alongside the request in #94, but I don't think it's big lift.

I'm happy to do this as a PR if it is ok with Author.

@Kralizek
Copy link
Owner

If you want to put some time for an explorative PR, I want to see how it would look like.

@to11mtm
Copy link
Author

to11mtm commented Dec 20, 2024

Will do. So far main pain point I'm seeing is that it would be a 'breaking' change,

Options.ConfigureSecretValueRequest operates on GetSecretValueRequest but change would involve BatchGetSecretValueRequest instead.

@Kralizek
Copy link
Owner

May I suggest adding a property to the configuration provider and use that property to influence the backend provider source?

This would allow us to keep the current behavior without breaking existing users.

@ransagy
Copy link

ransagy commented Dec 22, 2024

Exactly what i came looking for since POCing Secrets Manager and discovering this library (which i totally assumed their .NET SDK would already have..).
Can help review and test if needed. Appreciate the effort :)

@to11mtm
Copy link
Author

to11mtm commented Dec 23, 2024

@ransagy If you get a chance, I'd bet would help if you could look at my draft PR #96 and pull down the branch to try; I have limited time to test against AWS especially in the next couple of weeks.

@ransagy
Copy link

ransagy commented Dec 24, 2024

@to11mtm I did a quick run and aside from one small possible bug (i left a comment), It seems to work well. Tried with all my secrets (~150) and it chunked them and called the list and batchfetch APIs the appropriate amount of times, all secrets were available with the correct values, etc.
I also tested a few numbers of smaller filtered list of secrets and those generated the correct amount of chunks and fetched the right values, as far as i can tell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants