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

[KYUUBI #6006][HELM] Support additional labels for service monitor #6019

Closed
wants to merge 2 commits into from

Conversation

sudohainguyen
Copy link
Contributor

@sudohainguyen sudohainguyen commented Jan 25, 2024

🔍 Description

Issue References 🔗

This pull request fixes #6006

Describe Your Solution 🔧

Add new value additionalLabels in serviceMonitor to support templating the labels so that Prometheus can discover it

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Our Prometheus is based on kube-prometheus-stack chart, so it requires label release: kube-prometheus-stack
The current setup of kyuubi helm does not allow the ServiceMonitor can be discovered by Prome because it's not able to pass extra labels to it.

Behavior With This Pull Request 🎉

The MR enables templating extra labels

Related Unit Tests


Checklist 📝

Be nice. Be informative.

@codecov-commenter
Copy link

codecov-commenter commented Jan 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (47a1091) 61.19% compared to head (ed3fb0e) 61.08%.
Report is 2 commits behind head on master.

❗ Current head ed3fb0e differs from pull request most recent head 69a86c5. Consider uploading reports for the commit 69a86c5 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6019      +/-   ##
============================================
- Coverage     61.19%   61.08%   -0.12%     
  Complexity       23       23              
============================================
  Files           623      623              
  Lines         37060    37060              
  Branches       5024     5024              
============================================
- Hits          22680    22639      -41     
- Misses        11948    11980      +32     
- Partials       2432     2441       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

charts/kyuubi/values.yaml Outdated Show resolved Hide resolved
Copy link
Member

@pan3793 pan3793 left a comment

Choose a reason for hiding this comment

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

The use case makes sense to me, thanks for contributing

@pan3793 pan3793 added this to the v1.9.0 milestone Jan 29, 2024
@pan3793 pan3793 changed the title [KYUUBI #6006] Support additional labels for service monitor [KYUUBI #6006][HELM] Support additional labels for service monitor Jan 29, 2024
@pan3793 pan3793 closed this in 6ae2086 Jan 29, 2024
@pan3793
Copy link
Member

pan3793 commented Jan 29, 2024

Thanks, merged to master

@sudohainguyen sudohainguyen deleted the helmchart branch January 29, 2024 05:52
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this pull request Feb 5, 2024
# 🔍 Description
## Issue References 🔗

This pull request fixes apache#6006

## Describe Your Solution 🔧

Add new value `additionalLabels` in `serviceMonitor` to support templating the labels so that Prometheus can discover it

## Types of changes 🔖

- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️
Our Prometheus is based on `kube-prometheus-stack` chart, so it requires label `release: kube-prometheus-stack`
The current setup of kyuubi helm does not allow the ServiceMonitor can be discovered by Prome because it's not able to pass extra labels to it.

#### Behavior With This Pull Request 🎉
The MR enables templating extra labels

#### Related Unit Tests

---

# Checklist 📝

- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes apache#6019 from sudohainguyen/helmchart.

Closes apache#6006

69a86c5 [Hai Nguyen] chore: simplify labels values
ed3fb0e [Hai Nguyen] [KYUUBI apache#6006] Support additional labels for service monitor

Authored-by: Hai Nguyen <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this pull request Mar 21, 2024
# 🔍 Description
## Issue References 🔗

This pull request fixes apache#6006

## Describe Your Solution 🔧

Add new value `additionalLabels` in `serviceMonitor` to support templating the labels so that Prometheus can discover it

## Types of changes 🔖

- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️
Our Prometheus is based on `kube-prometheus-stack` chart, so it requires label `release: kube-prometheus-stack`
The current setup of kyuubi helm does not allow the ServiceMonitor can be discovered by Prome because it's not able to pass extra labels to it.

#### Behavior With This Pull Request 🎉
The MR enables templating extra labels

#### Related Unit Tests

---

# Checklist 📝

- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes apache#6019 from sudohainguyen/helmchart.

Closes apache#6006

69a86c5 [Hai Nguyen] chore: simplify labels values
ed3fb0e [Hai Nguyen] [KYUUBI apache#6006] Support additional labels for service monitor

Authored-by: Hai Nguyen <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
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.

[Improvement] Add template to provide additional labels for ServiceMonitor
3 participants