chart(selenium-grid): add ServiceMonitor and PodMonitor support for Prometheus Operator#3121
Open
DrFaust92 wants to merge 1 commit intoSeleniumHQ:trunkfrom
Open
chart(selenium-grid): add ServiceMonitor and PodMonitor support for Prometheus Operator#3121DrFaust92 wants to merge 1 commit intoSeleniumHQ:trunkfrom
DrFaust92 wants to merge 1 commit intoSeleniumHQ:trunkfrom
Conversation
Add Prometheus Operator ServiceMonitor and PodMonitor resources for the metrics exporter, allowing users to integrate with kube-prometheus-stack without manually configuring additionalScrapeConfigs. - Name the exporter container port `metrics` for explicit port referencing - Keep service port name `http-port` (no breaking change); targetPort now references the named port `metrics` - Add `monitoring-service-monitor.yaml`: ServiceMonitor scraping via the exporter Service on port `http-port` - Add `monitoring-pod-monitor.yaml`: PodMonitor scraping pods directly on port `metrics` (alternative to ServiceMonitor) - Add `monitoring.serviceMonitor.*` and `monitoring.podMonitor.*` values (enabled, namespace, labels, annotations, path, interval, scrapeTimeout, relabelings, metricRelabelings); both disabled by default Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
Review Summary by QodoAdd ServiceMonitor and PodMonitor support for Prometheus Operator
WalkthroughsDescription• Add Prometheus Operator ServiceMonitor and PodMonitor resources • Name exporter container port metrics for explicit referencing • Update Service targetPort to reference named port metrics • Add configurable monitoring values for both resource types Diagramflowchart LR
A["Exporter Deployment"] -->|named port metrics| B["Service"]
B -->|ServiceMonitor| C["Prometheus Operator"]
A -->|PodMonitor| C
D["values.yaml"] -->|serviceMonitor config| B
D -->|podMonitor config| A
File Changes1. charts/selenium-grid/templates/monitoring-exporter-deployment.yaml
|
Contributor
Code Review by Qodo
1. Cross-namespace monitor fails
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+5
to
+20
| name: {{ template "seleniumGrid.monitoring.exporter.fullname" $ }} | ||
| namespace: {{ .Values.monitoring.serviceMonitor.namespace | default .Release.Namespace }} | ||
| {{- with .Values.monitoring.serviceMonitor.annotations }} | ||
| annotations: | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| labels: | ||
| {{- include "seleniumGrid.commonLabels" $ | nindent 4 }} | ||
| {{- with .Values.monitoring.serviceMonitor.labels }} | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| spec: | ||
| selector: | ||
| matchLabels: | ||
| app.kubernetes.io/name: {{ template "seleniumGrid.monitoring.exporter.fullname" $ }} | ||
| endpoints: |
Contributor
There was a problem hiding this comment.
1. Cross-namespace monitor fails 🐞 Bug ≡ Correctness
ServiceMonitor/PodMonitor can be deployed to a user-specified namespace, but the exporter Service/Pods are always created in the release namespace. Because the monitors don't set any namespace selector, moving them to another namespace prevents them from selecting the exporter targets, so scraping won't happen.
Agent Prompt
### Issue description
`ServiceMonitor`/`PodMonitor` support `.Values.monitoring.{serviceMonitor,podMonitor}.namespace`, but the exporter Service/Pods always live in `.Release.Namespace`. Without a `spec.namespaceSelector`, deploying the monitor to a different namespace makes it unable to discover targets.
### Issue Context
The exporter resources are namespaced to `.Release.Namespace`, while the new monitor templates optionally change `metadata.namespace`.
### Fix Focus Areas
- charts/selenium-grid/templates/monitoring-service-monitor.yaml[5-33]
- charts/selenium-grid/templates/monitoring-pod-monitor.yaml[5-33]
- charts/selenium-grid/values.yaml[1044-1084]
### Expected fix
1. Add a configurable `namespaceSelector` block for both ServiceMonitor and PodMonitor (e.g., `monitoring.serviceMonitor.namespaceSelector` / `monitoring.podMonitor.namespaceSelector`).
2. Default it so targets are selected from `.Release.Namespace` (or allow empty `{}` to mean “same namespace” but document clearly).
3. If you keep the `namespace` override, ensure the default behavior (monitor in release namespace) still works unchanged.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Users running kube-prometheus-stack currently have no native way to scrape the Selenium Grid metrics exporter via Prometheus Operator. The only option is to configure
additionalScrapeConfigsmanually. This PR adds first-classServiceMonitorandPodMonitorsupport.Changes
metrics) on the exporter Deployment — explicit port referencing instead of a magic numbertargetPortupdated to reference the named portmetrics(port namehttp-portpreserved — no breaking change)monitoring-service-monitor.yaml— newServiceMonitorresource scraping the exporter Service on porthttp-portmonitoring-pod-monitor.yaml— newPodMonitorresource scraping exporter Pods directly on portmetrics(alternative for users without a Service)values.yaml— newmonitoring.serviceMonitor.*andmonitoring.podMonitor.*blocks; both disabled by defaultUsage
Testing
Both render valid
monitoring.coreos.com/v1resources with correct port references.