helm-charts icon indicating copy to clipboard operation
helm-charts copied to clipboard

feat: Add serviceMonitor resource

Open shubham-cmyk opened this issue 10 months ago • 4 comments

Description

It would add a prometheus serviceMonitor resource.

Check List

  • [x] Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

  • [x] Helm chart version bumped
  • [ ] Helm chart CHANGELOG.md updated to reflect change

PR sponsored by Obmondo.com

shubham-cmyk avatar Apr 24 '24 10:04 shubham-cmyk

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"

smbambling avatar May 08 '24 12:05 smbambling

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

shubham-cmyk avatar May 27 '24 18:05 shubham-cmyk

We would love to have this feature in the chart!

eyenx avatar Jul 01 '24 09:07 eyenx

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

gaiksaya avatar Jul 01 '24 18:07 gaiksaya

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

eyenx avatar Aug 14 '24 08:08 eyenx

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

Thanks.

peterzhuamazon avatar Aug 14 '24 14:08 peterzhuamazon

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.

peterzhuamazon avatar Aug 15 '24 20:08 peterzhuamazon

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!

VILJkid avatar Aug 16 '24 08:08 VILJkid

Thanks @VILJkid

eyenx avatar Aug 16 '24 08:08 eyenx

@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.

VILJkid avatar Aug 16 '24 09:08 VILJkid

@TheAlgo can we merge this?

eyenx avatar Aug 20 '24 08:08 eyenx

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

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

VILJkid avatar Aug 21 '24 10:08 VILJkid

Thanks @TheAlgo!

The lint failures, and changelog version comparison is fixed!

VILJkid avatar Aug 21 '24 17:08 VILJkid

Thanks @peterzhuamazon!

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

VILJkid avatar Aug 21 '24 18:08 VILJkid

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

prudhvigodithi avatar Aug 22 '24 00:08 prudhvigodithi

Thanks @shubham-cmyk @VILJkid ,

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

Thanks!

peterzhuamazon avatar Aug 22 '24 18:08 peterzhuamazon

Thanks @peterzhuamazon,

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

VILJkid avatar Aug 23 '24 07:08 VILJkid

Hi @peterzhuamazon,

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

VILJkid avatar Aug 23 '24 08:08 VILJkid