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.

Sign up for GitHub

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

Jump to bottom

[Backport 1.x] Add Service Monitor Resource #578

Merged
merged 1 commit into from
Sep 6, 2024
Merged

[Backport 1.x] Add Service Monitor Resource #578

merged 1 commit into from
Sep 6, 2024

Conversation

VILJkid
Copy link
Contributor

@VILJkid VILJkid commented Aug 23, 2024 •
edited
Loading

Description

It would add a prometheus serviceMonitor resource.

Check List

  • Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

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

PR sponsored by Obmondo

@VILJkid VILJkid mentioned this pull request Aug 23, 2024
Merged
3 tasks
@prudhvigodithi
Copy link
Collaborator

Thanks @VILJkid should we wait until this fix is pushed #579 (comment) ?
@getsaurabh02 @peterzhuamazon

@VILJkid
Copy link
Contributor Author

VILJkid commented Aug 23, 2024

Thanks for the update, @prudhvigodithi!
I'll keep an eye on that, and update this PR accordingly.

peterzhuamazon
peterzhuamazon requested changes Aug 23, 2024
View reviewed changes
Copy link
Member

@peterzhuamazon peterzhuamazon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Block until the bug is resolved on original main branch in case it get merged now.

Thanks.

@peterzhuamazon
Copy link
Member

Pending fix:

@peterzhuamazon
Copy link
Member

Pending fix:

* [[Fix] OpenSearch and OpenSearch Dashboards Service Monitor Bug #581](https://github.com/opensearch-project/helm-charts/pull/581)

Since this is merged now would you mind update this PR to include above changes?
Thanks @VILJkid !

@VILJkid
Copy link
Contributor Author

VILJkid commented Aug 29, 2024

Thanks @peterzhuamazon, I'll update this PR with the changes shortly.

@VILJkid VILJkid force-pushed the serviceMonitor-backport branch 2 times, most recently from 7253041 to e1dabb9 Compare August 29, 2024 05:10
@VILJkid
Copy link
Contributor Author

VILJkid commented Aug 29, 2024

@peterzhuamazon, kindly review the changes and let me know if it needs any improvement.
Thanks!

@peterzhuamazon
Copy link
Member

Thanks @VILJkid , adding @prudhvigodithi to take a second pass.

@peterzhuamazon peterzhuamazon mentioned this pull request Aug 29, 2024
Merged
3 tasks
@peterzhuamazon peterzhuamazon linked an issue Aug 29, 2024 that may be closed by this pull request
[Enhancement][opensearch/opensearch-dashboards] ServiceMonitor is missing the ability to add a specific label to it #585
Open
@peterzhuamazon
Copy link
Member

Waiting for @prudhvigodithi to give a second approval.

Thanks.

@prudhvigodithi
Copy link
Collaborator

There is a new bug created #588, @VILJkid can you please take a look before we merge this?
Thank you

peterzhuamazon
peterzhuamazon requested changes Sep 3, 2024
View reviewed changes
Copy link
Member

@peterzhuamazon peterzhuamazon left a comment

Choose a reason for hiding this comment

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

Block again due to new bug seen in here:

@VILJkid
Copy link
Contributor Author

VILJkid commented Sep 4, 2024

Thanks @prudhvigodithi, @peterzhuamazon for highlighting the issue.
Here's the fix PR: #589
Let me know if it needs any improvements.

@peterzhuamazon peterzhuamazon mentioned this pull request Sep 5, 2024
Merged
3 tasks
@peterzhuamazon
Copy link
Member

Thanks @prudhvigodithi, @peterzhuamazon for highlighting the issue. Here's the fix PR: #589 Let me know if it needs any improvements.

Thanks @VILJkid I have merged that PR now.
Please go ahead and backport the changes here as well.

Thanks!

@VILJkid
(feat) : service monitor resource support
a093e19 
Signed-off-by: VILJkid <sidvjmostwanted@gmail.com>

(clean) : redundant code

Signed-off-by: VILJkid <sidvjmostwanted@gmail.com>

(fix) : incorporate changes from pr#581

Signed-off-by: VILJkid <sidvjmostwanted@gmail.com>

(add) : protocol setting for service metrics

Signed-off-by: VILJkid <sidvjmostwanted@gmail.com>
@VILJkid
Copy link
Contributor Author

VILJkid commented Sep 6, 2024

Thanks @peterzhuamazon @prudhvigodithi!
I've updated this PR accordingly. Kindly review and let me know your thoughts.

peterzhuamazon
peterzhuamazon reviewed Sep 6, 2024
View reviewed changes
Copy link
Member

@peterzhuamazon peterzhuamazon left a comment

Choose a reason for hiding this comment

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

Thanks @VILJkid !

@peterzhuamazon peterzhuamazon merged commit bbd8158 into opensearch-project:1.x Sep 6, 2024
8 checks passed
@peterzhuamazon peterzhuamazon removed a link to an issue Sep 6, 2024
[Enhancement][opensearch/opensearch-dashboards] ServiceMonitor is missing the ability to add a specific label to it #585
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prudhvigodithi prudhvigodithi prudhvigodithi approved these changes

@peterzhuamazon peterzhuamazon peterzhuamazon approved these changes

@TheAlgo TheAlgo Awaiting requested review from TheAlgo TheAlgo is a code owner

@DandyDeveloper DandyDeveloper Awaiting requested review from DandyDeveloper DandyDeveloper is a code owner

@gaiksaya gaiksaya Awaiting requested review from gaiksaya gaiksaya is a code owner

Assignees
No one assigned
Labels
None yet
Projects
Engineering Effectiveness Board
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@VILJkid @prudhvigodithi @peterzhuamazon