opensearch-project / helm-charts

:wheel_of_dharma: A community repository for Helm Charts of OpenSearch Project.
https://opensearch.org/docs/latest/opensearch/install/helm/
Apache License 2.0
170 stars 228 forks source link

feat: Add serviceMonitor resource #537

Closed shubham-cmyk closed 1 month ago

shubham-cmyk commented 5 months ago

Description

It would add a prometheus serviceMonitor resource.

Check List

For any changes to files within Helm chart directories:

PR sponsored by Obmondo.com

smbambling commented 4 months ago

Is this inteded to be used in conjunction with https://github.com/Aiven-Open/prometheus-exporter-plugin-for-opensearch/tree/main ?

If so might I suggest setting the path to "/_prometheus/metrics"

shubham-cmyk commented 4 months ago

@TheAlgo Bumped the chart version can you run the check again

eyenx commented 3 months ago

We would love to have this feature in the chart!

gaiksaya commented 3 months ago

Hi @shubham-cmyk Can you please rebase? Looks like branch has a conflict.

eyenx commented 1 month ago

@shubham-cmyk do you need any help with this? we are looking forward to have this feature in the chart.

peterzhuamazon commented 1 month ago

I will take a look tomorrow and see if I can help rebase.

Thanks.

peterzhuamazon commented 1 month ago

I updated the version and changelog and tried to push but not able to do so, seems like the repo belongs to an org:

$ git push origin serviceMonitor
ERROR: Permission to Obmondo/helm-charts-1.git denied to peterzhuamazon.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Hi @shubham-cmyk could you please update the PR?

Thanks.

VILJkid commented 1 month ago

Hi team,

I'll be picking up where @shubham-cmyk left off to address the recent changes. I'll update the PR shortly with the necessary adjustments. Thanks for your patience, and I’ll keep you posted on the progress!

eyenx commented 1 month ago

Thanks @VILJkid

VILJkid commented 1 month ago

@eyenx, I’ve taken care of the rebase and chart version bump. @shubham-cmyk is on vacation, so please review the changes and inform me if any additional updates are needed.

eyenx commented 1 month ago

@TheAlgo can we merge this?

VILJkid commented 1 month ago

The requested changes have been pushed. Kindly review and merge this feature.

Thanks for your time, @prudhvigodithi @eyenx @bbarani @DandyDeveloper @peterzhuamazon @gaiksaya @TheAlgo

VILJkid commented 1 month ago

Thanks @TheAlgo!

The lint failures, and changelog version comparison is fixed!

VILJkid commented 1 month ago

Thanks @peterzhuamazon!

The fix has been pushed and the minor versions have been bumped up, as per the change request.

prudhvigodithi commented 1 month ago

Thanks @shubham-cmyk @VILJkid @peterzhuamazon @TheAlgo , LGTM we can merge once the CI checks pass.

peterzhuamazon commented 1 month ago

Thanks @shubham-cmyk @VILJkid ,

Would you be able to backport this change to 1.x branch?

Thanks!

VILJkid commented 1 month ago

Thanks @peterzhuamazon,

Yes, I'll raise another PR with this change for branch 1.x shortly.

VILJkid commented 1 month ago

Hi @peterzhuamazon,

Here's the PR for backporting this change to 1.x : https://github.com/opensearch-project/helm-charts/pull/578