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

feat(serviceMonitors): add option of additional Labels in serviceMonitor #586

Merged
merged 4 commits into from
Sep 9, 2024
Merged

feat(serviceMonitors): add option of additional Labels in serviceMonitor #586

merged 4 commits into from
Sep 9, 2024

Conversation

eyenx
Copy link
Contributor

@eyenx eyenx commented Aug 29, 2024 •
edited
Loading

Description

Adds ability to add additional Labels to ServiceMonitor in case Prometheus is scraping for specific labels

Issues Resolved

#585

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

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@prudhvigodithi prudhvigodithi mentioned this pull request Aug 29, 2024
Open
@peterzhuamazon
Copy link
Member

Adding @VILJkid to add some thoughts on this as the original serviceMonitor feature contributor.

Thanks.

@peterzhuamazon
Copy link
Member

We will most likely merge the original backport 1.x PR before this one:

@VILJkid
Copy link
Contributor

VILJkid commented Sep 4, 2024

Thanks @eyenx for this feature addition!
Custom labels will help organize it more effectively.
This looks good. Adding a sample custom label in comment would be helpful!

@eyenx
Copy link
Contributor Author

eyenx commented Sep 4, 2024

Will do after lunch

@eyenx
Copy link
Contributor Author

eyenx commented Sep 4, 2024

@VILJkid Done!

@eyenx eyenx force-pushed the feat/add-additional-labels-to-serviceMonitor branch 2 times, most recently from e04e80b to 1647306 Compare September 5, 2024 09:17
@peterzhuamazon
Copy link
Member

Hi @eyenx please rebase your changes as @VILJkid 's fix just merged.

We will go ahead and review this starting next week.

Thanks being patient!

@eyenx
feat(serviceMonitors): add option of additional Labels in serviceMonitor
2ed2ef8 
Signed-off-by: Toni Tauro <toni.tauro@adfinis.com>
@eyenx
fix(CHANGELOG): add new release
2293d83 
Signed-off-by: Toni Tauro <toni.tauro@adfinis.com>
@eyenx
doc(values): add examples
2d8e082 
Signed-off-by: Toni Tauro <toni.tauro@adfinis.com>
@eyenx eyenx force-pushed the feat/add-additional-labels-to-serviceMonitor branch from 1647306 to 2d8e082 Compare September 9, 2024 06:32
@eyenx
Copy link
Contributor Author

eyenx commented Sep 9, 2024

@peterzhuamazon rebased

@eyenx
Copy link
Contributor Author

eyenx commented Sep 9, 2024

aaah my editor reformatted the whole CHANGELOG.. one sec.

@eyenx eyenx force-pushed the feat/add-additional-labels-to-serviceMonitor branch 2 times, most recently from 3eff7e2 to 16dcd2a Compare September 9, 2024 06:38
@eyenx
fix(CHANGELOG): correct formatting
f468c04 
Signed-off-by: Toni Tauro <toni.tauro@adfinis.com>
@eyenx eyenx force-pushed the feat/add-additional-labels-to-serviceMonitor branch from 16dcd2a to f468c04 Compare September 9, 2024 06:39
@eyenx
Copy link
Contributor Author

eyenx commented Sep 9, 2024

Okay @peterzhuamazon @VILJkid fixed it.

peterzhuamazon
peterzhuamazon reviewed Sep 9, 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.

I feel like this change is pretty straight forward.
What do you think about it @VILJkid @prudhvigodithi ?

Thanks.

@prudhvigodithi
Copy link
Collaborator

LGTM, thanks @eyenx.

@prudhvigodithi prudhvigodithi added the backport 1.x Backport to 1.x branch after merging to main label Sep 9, 2024
@peterzhuamazon peterzhuamazon merged commit 10646b1 into opensearch-project:main Sep 9, 2024
8 checks passed
@peterzhuamazon
Copy link
Member

Thanks @eyenx for the contribution and would you mind backport to 1.x as well?

Thanks!

@peterzhuamazon peterzhuamazon linked an issue Sep 9, 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 peterzhuamazon mentioned this pull request Sep 9, 2024
Merged
3 tasks
@eyenx
Copy link
Contributor Author

eyenx commented Sep 10, 2024

Thanks @eyenx for the contribution and would you mind backport to 1.x as well?

Thanks!

I'll try to see if I can fit this into my schedule of this week.

@eyenx
Copy link
Contributor Author

eyenx commented Sep 10, 2024

@peterzhuamazon #592

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

@VILJkid VILJkid VILJkid 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
backport 1.x Backport to 1.x branch after merging to main first-time-contributor
Projects
Engineering Effectiveness Board
Status: ✅ Done
Milestone
No milestone
4 participants
@eyenx @peterzhuamazon @VILJkid @prudhvigodithi