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

support multiple aws accounts with credentials #140

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bryanfang
Copy link

No description provided.

Signed-off-by: Bryan Fang <bryan.fang@aktana.com>
@bryanfang bryanfang force-pushed the feature/support-multiple-accounts branch from 39862e0 to 2b01ff7 Compare March 16, 2024 00:34
Copy link
Contributor

@vmercierfr vmercierfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work to add the support of multi AWS accounts!

Some logics are duplicated (could be simplified to faciliate maintenance), and we may improve exporter scrape duration with GO routines.

The main blocker could be to continue to support multiple AWS authentication methods (including AWS assumed role), I would like to propose an another alternative in upcoming weeks.

// getMetrics collects and return all RDS metrics
func (c *rdsCollector) fetchMetrics() error {
c.logger.Debug("received query")

if len(c.accountRegionClients) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The business logic is duplicated, while we may build a single loop with all AWS accounts to proceed.

In this implementation, AWS accounts will be retrieved one by one, whereas we could rely on Goroutine to collect them in parallel execution.

@vmercierfr vmercierfr requested a review from dcupif March 22, 2024 17:12
@felipegfalcao
Copy link

It seems very interesting to me since I use the cloudwatch exporter with multiple roles to access multi-AWS accounts. I will be following this change

At YACE we have something very interesting:
https://github.com/nerdswords/yet-another-cloudwatch-exporter?tab=readme-ov-file#options

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

Successfully merging this pull request may close these issues.

None yet

3 participants