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

[prometheus-mongodb-exporter] Support multi-target exporter #4875

Merged
merged 5 commits into from
Nov 5, 2024

Conversation

BapRx
Copy link
Contributor

@BapRx BapRx commented Sep 26, 2024

What this PR does / why we need it

Allow configuring multiple targets for the exporter v0.40.0 and later.

Which issue this PR fixes

Special notes for your reviewer

Until the exporter supports reading passwords from a file, we are limited to choose out of two solutions:

  • generate MONGODB_URI automatically from the target list BUT share the same username/password
  • create a secret manually with the content of MONGODB_URI but also manage the serviceMonitor list separately

I don't really know what's the best solution, open for suggestions! I went for the first solution which matches my use case but if you think the other option is better for most people, we can rethink the logic.

Checklist

  • DCO signed
  • Chart Version bumped
  • Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

Copy link
Member

@zeritti zeritti left a comment

Choose a reason for hiding this comment

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

Thank you, @BapRx, for your PR. Please, see my comments below.

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>
Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>
@BapRx
Copy link
Contributor Author

BapRx commented Oct 30, 2024

Hello @zeritti, thank you for your review, I pushed some changes to fix the issues you mentioned.

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>
@BapRx BapRx requested a review from zeritti October 30, 2024 09:41
@zeritti
Copy link
Member

zeritti commented Oct 30, 2024

Thank you, @BapRx, for your changes. I will still validate the changes over the coming days. As to the chosen use case of a single set of credentials, I am confident that we can start with that.
I would also suggest that we consider creating a CI test case - Prometheus operator CRDs are being included in a CI run and hence, we can make use of it.

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>
@zeritti
Copy link
Member

zeritti commented Oct 30, 2024

A CI values file for ct must have name in *-values.yaml, e.g. servicemonitor-multi-target-values.yaml, otherwise it gets ignored.

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>
@BapRx
Copy link
Contributor Author

BapRx commented Oct 30, 2024

A CI values file for ct must have name in *-values.yaml, e.g. servicemonitor-multi-target-values.yaml, otherwise it gets ignored.

I see, thank you 🙂

@zeritti zeritti merged commit f828f54 into prometheus-community:main Nov 5, 2024
4 checks passed
kranthikirang pushed a commit to kranthikirang/helm-charts that referenced this pull request Nov 5, 2024
…us-community#4875)

* feat(mongodb): support multi-target exporter

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>

* feat(prometheus-mongodb-exporter): apply suggestions

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>

* chore(prometheus-mongodb-exporter): bump chart version

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>

* chore(prometheus-mongodb-exporter): add CI multi-target test

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>

* fix(prometheus-mongodb-exporter): rename CI multi-target test file

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>

---------

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>
kranthikirang pushed a commit to kranthikirang/helm-charts that referenced this pull request Nov 5, 2024
…us-community#4875)

* feat(mongodb): support multi-target exporter

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>

* feat(prometheus-mongodb-exporter): apply suggestions

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>

* chore(prometheus-mongodb-exporter): bump chart version

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>

* chore(prometheus-mongodb-exporter): add CI multi-target test

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>

* fix(prometheus-mongodb-exporter): rename CI multi-target test file

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>

---------

Signed-off-by: Baptiste Roux <baptiste.roux@skale-5.com>
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.

[prometheus-mongodb-exporter] Multi Target Support
2 participants