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

[kube-prometheus-stack] add serviceMonitor.additionalEndpoints #3515

Merged
merged 10 commits into from Oct 16, 2023
Merged

Conversation

TheRealNoob
Copy link
Contributor

@TheRealNoob TheRealNoob commented Jun 21, 2023

What this PR does / why we need it

Adds support for additional Endpoints in built-in serviceMonitors. When adding an oauth-sidecar with --metrics-address, in order to add monitoring for it would require creating a 2nd ServiceMonitor. This adds support for adding it as an endpoint on the existing ServiceMonitor.

# desired spec
prometheus:
  serviceMonitor:
    additionalEndpoints:
      - path: /metrics
        port: oauth-metrics

# current spec
prometheus:
  additionalServiceMonitors:
    - name: prometheus-prometheus-oauth
      selector:
        matchLabels:
          app: kube-prometheus-stack-prometheus
          release: prometheus
      endpoints:
        - path: /metrics
          port: oauth-metrics

Which issue this PR fixes

Special notes for your reviewer

I also included several very small (related) changes in values.yaml. Mostly comments.

Checklist

  • DCO signed
  • Chart Version bumped
  • Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

@stale
Copy link

stale bot commented Aug 7, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale stale bot added the lifecycle/stale label Aug 7, 2023
@QuentinBisson
Copy link
Member

@TheRealNoob can you sign your commits?

@stale stale bot removed the lifecycle/stale label Aug 22, 2023
@TheRealNoob TheRealNoob reopened this Oct 14, 2023
@TheRealNoob TheRealNoob marked this pull request as draft October 14, 2023 18:05
@TheRealNoob TheRealNoob marked this pull request as ready for review October 15, 2023 04:11
@TheRealNoob
Copy link
Contributor Author

@QuentinBisson Thank you for the msg. Unfortunately I missed it in my email and it managed to slip my mind. I've now updated it and am ready for review. Couple talking points

  • In the process of merging from master I saw that someone added a new endpoint to the same ServiceMonitor that this PR looks to change. Previously I did a simpler approach of templating the passed list, but this time I changed it to look more similar to the other two endpoints.
  • I didn't add any unittests. Let me know if you need them. But from the looks of it very little is tested using that approach.
  • I've not used Thanos from this helm chart so my adding additionalEndpoints to it was a bit of a shot in the dark. Let me know if it doesn't make sense of it I missed adding it to something more.

@TheRealNoob
Copy link
Contributor Author

I think following semver this chart version should be bumped to 51.9.0 not 51.8.1 -- let me know.

{{ toYaml .Values.alertmanager.serviceMonitor.relabelings | indent 6 }}
{{- end }}
{{- if .Values.alertmanager.serviceMonitor.metricRelabelings }}
metricRelabelings: {{- tpl (toYaml .Values.alertmanager.serviceMonitor.metricRelabelings | nindent 6) . }}
Copy link
Member

Choose a reason for hiding this comment

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

Why would this even require nindent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My purpose in changing this was just to make the code more readable. The outcome should be identical. I've just run a helm template on my branch as well the main branch (with a values file that defines both relabelings and metricsRelabelings) and and the outcomes are the identical.

The documentation confirms both are expected to be a []RelabelConfig list, so nindent sounds right to me.

Copy link
Member

Choose a reason for hiding this comment

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

Alright :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you feel it doesn't make it more readable I'd be happy to change it back. Looking at the code on Github's website I'd say it doesn't feel any different but when looking on an IDE, each Endpoint is now equally indented so you can now tell where they start and stop.

metricRelabelings: {{- tpl (toYaml .Values.alertmanager.serviceMonitor.metricRelabelings | nindent 6) . }}
{{- end }}
{{- if .Values.alertmanager.serviceMonitor.relabelings }}
relabelings: {{- toYaml .Values.alertmanager.serviceMonitor.relabelings | nindent 6 }}
Copy link
Member

Choose a reason for hiding this comment

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

Same remark

@QuentinBisson
Copy link
Member

@TheRealNoob Could you fix the linting errors, check my comments and bump the version to 51.9.0? Then we should be good to go :)

TheRealNoob and others added 8 commits October 16, 2023 09:41
Signed-off-by: TheRealNoob <mike1118@live.com>
…pport (#3865)

* Fixed defaults for annotations from list to dict

Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>

* Added service labels and annotations

Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>

* Added ingress labels

Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>

* Bumped version to 0.7.2

Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>

---------

Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>
Signed-off-by: TheRealNoob <mike1118@live.com>
Signed-off-by: TheRealNoob <mike1118@live.com>
Signed-off-by: TheRealNoob <mike1118@live.com>
Signed-off-by: TheRealNoob <mike1118@live.com>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: TheRealNoob <mike1118@live.com>
* fix range function to include relabelings and metricRelabelings

Signed-off-by: Christian Niehoff <christian.niehoff@dwpbank.de>

* bump chart version

Signed-off-by: Christian Niehoff <christian.niehoff@dwpbank.de>

---------

Signed-off-by: Christian Niehoff <christian.niehoff@dwpbank.de>
Signed-off-by: TheRealNoob <mike1118@live.com>
* [prometheus-smartctl-exporter] Add relabelings to servicemonitor

Signed-off-by: nepomucen <szymon@aus.krakow.pl>

* bump chart version to 0.6.1

Signed-off-by: nepomucen <szymon@aus.krakow.pl>

* Revert "bump chart version to 0.6.1"

This reverts commit 124b85c.

Signed-off-by: nepomucen <szymon@aus.krakow.pl>

* Bump chart's minor version instead of patch version

Signed-off-by: nepomucen <szymon@aus.krakow.pl>

* Use with to control variable scoping

Signed-off-by: nepomucen <szymon@aus.krakow.pl>

---------

Signed-off-by: nepomucen <szymon@aus.krakow.pl>
Signed-off-by: TheRealNoob <mike1118@live.com>
Signed-off-by: TheRealNoob <mike1118@live.com>
Signed-off-by: TheRealNoob <TheRealNoob@users.noreply.github.com>
@TheRealNoob
Copy link
Contributor Author

This DCO thing is very annoying....

@QuentinBisson QuentinBisson merged commit b6567c0 into prometheus-community:main Oct 16, 2023
4 checks passed
@QuentinBisson
Copy link
Member

Thank you @TheRealNoob 🎉

spiceratops added a commit to spiceratops/k8s-gitops that referenced this pull request Oct 16, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
|
[kube-prometheus-stack](https://togithub.com/prometheus-community/helm-charts)
| minor | `51.8.0` -> `51.9.0` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>prometheus-community/helm-charts
(kube-prometheus-stack)</summary>

###
[`v51.9.0`](https://togithub.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-51.9.0)

kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards,
and Prometheus rules combined with documentation and scripts to provide
easy to operate end-to-end Kubernetes cluster monitoring with Prometheus
using the Prometheus Operator.

#### What's Changed

- \[kube-prometheus-stack] add serviceMonitor.additionalEndpoints by
[@&#8203;TheRealNoob](https://togithub.com/TheRealNoob) in
[prometheus-community/helm-charts#3515

**Full Changelog**:
prometheus-community/helm-charts@prometheus-smartctl-exporter-0.7.0...kube-prometheus-stack-51.9.0

###
[`v51.8.1`](https://togithub.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-51.8.1)

kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards,
and Prometheus rules combined with documentation and scripts to provide
easy to operate end-to-end Kubernetes cluster monitoring with Prometheus
using the Prometheus Operator.

#### What's Changed

- \[kube-prometheus-stack] fix range function in servicemonitors by
[@&#8203;r3kzi](https://togithub.com/r3kzi) in
[prometheus-community/helm-charts#3888

**Full Changelog**:
prometheus-community/helm-charts@kube-prometheus-stack-51.8.0...kube-prometheus-stack-51.8.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/spiceratops/k8s-gitops).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xOS4yIiwidXBkYXRlZEluVmVyIjoiMzcuMTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->
nrdufour added a commit to nrdufour/home-ops that referenced this pull request Oct 17, 2023
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [kube-prometheus-stack](https://github.com/prometheus-operator/kube-prometheus) ([source](https://github.com/prometheus-community/helm-charts)) | minor | `51.8.0` -> `51.9.0` |

---

### Release Notes

<details>
<summary>prometheus-community/helm-charts (kube-prometheus-stack)</summary>

### [`v51.9.0`](https://github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-51.9.0)

[Compare Source](prometheus-community/helm-charts@kube-prometheus-stack-51.8.1...kube-prometheus-stack-51.9.0)

kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator.

#### What's Changed

-   \[kube-prometheus-stack] add serviceMonitor.additionalEndpoints by [@&#8203;TheRealNoob](https://github.com/TheRealNoob) in prometheus-community/helm-charts#3515

**Full Changelog**: prometheus-community/helm-charts@prometheus-smartctl-exporter-0.7.0...kube-prometheus-stack-51.9.0

### [`v51.8.1`](https://github.com/prometheus-community/helm-charts/releases/tag/kube-prometheus-stack-51.8.1)

[Compare Source](prometheus-community/helm-charts@kube-prometheus-stack-51.8.0...kube-prometheus-stack-51.8.1)

kube-prometheus-stack collects Kubernetes manifests, Grafana dashboards, and Prometheus rules combined with documentation and scripts to provide easy to operate end-to-end Kubernetes cluster monitoring with Prometheus using the Prometheus Operator.

#### What's Changed

-   \[kube-prometheus-stack] fix range function in servicemonitors by [@&#8203;r3kzi](https://github.com/r3kzi) in prometheus-community/helm-charts#3888

**Full Changelog**: prometheus-community/helm-charts@kube-prometheus-stack-51.8.0...kube-prometheus-stack-51.8.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMC4yIiwidXBkYXRlZEluVmVyIjoiMzcuMjAuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Reviewed-on: https://git.home/nrdufour/home-ops/pulls/155
Co-authored-by: Renovate <renovate@ptinem.io>
Co-committed-by: Renovate <renovate@ptinem.io>
Matiasmct pushed a commit to giffgaff/prometheus-charts that referenced this pull request Mar 20, 2024
…theus-community#3515)

* add serviceMonitor.additionalEndpoints

Signed-off-by: TheRealNoob <mike1118@live.com>

* [prometheus-json-exporter] defaults and service labels/annotations support (prometheus-community#3865)

* Fixed defaults for annotations from list to dict

Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>

* Added service labels and annotations

Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>

* Added ingress labels

Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>

* Bumped version to 0.7.2

Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>

---------

Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>
Signed-off-by: TheRealNoob <mike1118@live.com>

* fix closing if statements

Signed-off-by: TheRealNoob <mike1118@live.com>

* add newlines

Signed-off-by: TheRealNoob <mike1118@live.com>

* fix root reference

Signed-off-by: TheRealNoob <mike1118@live.com>

* [kube-prometheus-stack] Fix IO panels in Cluster Overview (prometheus-community#3902)

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: TheRealNoob <mike1118@live.com>

* [kube-prometheus-stack] fix range function in servicemonitors (prometheus-community#3888)

* fix range function to include relabelings and metricRelabelings

Signed-off-by: Christian Niehoff <christian.niehoff@dwpbank.de>

* bump chart version

Signed-off-by: Christian Niehoff <christian.niehoff@dwpbank.de>

---------

Signed-off-by: Christian Niehoff <christian.niehoff@dwpbank.de>
Signed-off-by: TheRealNoob <mike1118@live.com>

* [prometheus-smartctl-exporter] Add relabelings to servicemonitor (prometheus-community#3891)

* [prometheus-smartctl-exporter] Add relabelings to servicemonitor

Signed-off-by: nepomucen <szymon@aus.krakow.pl>

* bump chart version to 0.6.1

Signed-off-by: nepomucen <szymon@aus.krakow.pl>

* Revert "bump chart version to 0.6.1"

This reverts commit 124b85c.

Signed-off-by: nepomucen <szymon@aus.krakow.pl>

* Bump chart's minor version instead of patch version

Signed-off-by: nepomucen <szymon@aus.krakow.pl>

* Use with to control variable scoping

Signed-off-by: nepomucen <szymon@aus.krakow.pl>

---------

Signed-off-by: nepomucen <szymon@aus.krakow.pl>
Signed-off-by: TheRealNoob <mike1118@live.com>

* bump chart version

Signed-off-by: TheRealNoob <mike1118@live.com>

---------

Signed-off-by: TheRealNoob <mike1118@live.com>
Signed-off-by: Aaron Benton<bentonam@gmail.com>
Signed-off-by: Aaron Benton <bentonam@gmail.com>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Christian Niehoff <christian.niehoff@dwpbank.de>
Signed-off-by: nepomucen <szymon@aus.krakow.pl>
Signed-off-by: TheRealNoob <TheRealNoob@users.noreply.github.com>
Co-authored-by: Aaron <bentonam@gmail.com>
Co-authored-by: Jan-Otto Kröpke <github@jkroepke.de>
Co-authored-by: Christian Niehoff <mail@christian-niehoff.com>
Co-authored-by: Szymon Janczy <szymon@aus.krakow.pl>
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.

[kube-prometheus-stack] add serviceMonitor.additionalEndpoints field
6 participants