Add monitoring controller for metrics by mayleighnmyers · Pull Request #1959 · istio-ecosystem/sail-operator

mayleighnmyers · 2026-05-18T18:57:50Z

Summary

Implements SEP8471: a monitoring controller that automatically creates Prometheus Operator ServiceMonitor and PodMonitor resources so Istio control-plane and sidecar metrics can be scraped without manual Prometheus ConfigMap edits.

Opt-in via annotation on the Istio CR:

apiVersion: sailoperator.io/v1
kind: Istio
metadata:
  name: default
  annotations:
    sailoperator.io/monitoring: enabled
spec:
  namespace: istio-system

Relates to: OSSM-15085

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Related Issue/PR # OSSM-15085

Additional information:

linux-foundation-easycla · 2026-05-18T18:57:57Z

The committers listed above are authorized under a signed CLA.

✅ login: mayleighnmyers / name: Mayleigh Tjapkes (42511f0, 4adeec0, 95feca9, b1cd237, c4cbbf0, e8bfa31)
✅ login: yxun / name: Yuanlin Xu (63ec0fe)

istio-testing · 2026-05-18T18:58:08Z

Hi @mayleighnmyers. Thanks for your PR.

I'm waiting for a istio-ecosystem or istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

nrfox · 2026-05-28T12:58:27Z

Hi @mayleighnmyers,

How would users control this if they wanted to opt in or out? What will the API be for this? You can either open an issue and we can discuss it there or better yet create a SEP for this. We need to discuss how this will integrate with the operator before we can move forward with this.

mayleighnmyers · 2026-05-28T13:09:03Z

Hi @mayleighnmyers,

How would users control this if they wanted to opt in or out? What will the API be for this? You can either open an issue and we can discuss it there or better yet create a SEP for this. We need to discuss how this will integrate with the operator before we can move forward with this.

So this is still in progress. Good call for the opt in/out. Something we can consider is adding a field either in the Istio CR or the istioRevision CR to allow users to opt in since right now it is currently always enabled. Yuanlin and I will be talking about this more in some future meeting when we have all ducks in a row. :)

nrfox · 2026-06-09T00:52:47Z

The implementation of this broadly looks fine but I think the API will need to be larger than a boolean and will require the sail operator to have additional permissions which may not be relevant for non-openshift users. With ZTunnel and IstioRevisionTag we have targetRef fields that reference the Istio resource. Maybe we could do something similar with this integration.

There could be a standalone Integration resource that has 1. an Istio ref 2. refs to metrics or tracing resources. Something like:

  apiVersion: integration.ossm/v1alpha1
  kind: Integration
  metadata:
    name: my-integration
    namespace: istio-system
  spec:
    istioRef:
      name: default
    metrics:
      type: ClusterObservability
      clusterObservability:
        monitoringStackRef:
          name: my-stack
          namespace: monitoring
    tracing:
      type: TempoStack
      tempoStackRef:
        name: my-tempo
        namespace: tracing

Here istioRef is a reference to an Istio resource, monitoringStackRef is a reference to a MonitoringStack, and tempoStackRef is a reference to a TempoStack. For each of these references the controller would lookup the relevant fields in the reference and configure the Istio accordingly. For example, the controller would use the monitoringStackRef to find the resource label for the PodMonitor/ServiceMonitor resources.

This could even be its own deployment with its own ServiceAccount so that the Sail Operator wouldn't need to be granted additional permissions. Since a lot of the integrations may be OSSM specific, it may even make more sense to only have this available in ossm/sail-operator. Not sure.

Server side apply can ensure that any updates that the integrations controller makes to the Istio resource do not conflict with user supplied values.

The implementation included here would largely stay the same. The main change would be in the API to manage these integrations.

mayleighnmyers · 2026-07-10T12:02:52Z

/test scorecard

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Signed-off-by: Yuanlin Xu <yuanlin.xu@redhat.com> Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Signed-off-by: Yuanlin Xu <yuanlin.xu@redhat.com>

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

The gencheck job runs make gen, which updates bundle manifests from chart CRDs. The bundle copy of sailoperator.io_istios.yaml was missing the new spec.monitoring schema added for the monitoring controller. Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

mayleighnmyers · 2026-09-11T14:20:47Z

/retest

nrfox

A few minor comments otherwise LGTM

nrfox · 2026-09-17T16:17:46Z

+			if tt.expectSMRevision != "" {
+				sm := &monitoringv1.ServiceMonitor{}
+				sm.SetGroupVersionKind(monitoringv1.SchemeGroupVersion.WithKind("ServiceMonitor"))
+				err := cl.Get(ctx, types.NamespacedName{
+					Name:      tt.expectSMRevision + serviceMonitorNameSuffix,
+					Namespace: istioNamespace,
+				}, sm)
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(sm.Name).To(Equal(tt.expectSMRevision + serviceMonitorNameSuffix))
+				expectMonitoringLabels(g, sm.Labels, serviceMonitorMonitoring)
+			}


Seems like we need an else here to check the negative conditions where we don't expect the service monitor to exist?

nrfox · 2026-09-17T16:18:02Z

+			if tt.expectPMNamespace != "" {
+				pm := &monitoringv1.PodMonitor{}
+				pm.SetGroupVersionKind(monitoringv1.SchemeGroupVersion.WithKind("PodMonitor"))
+				err := cl.Get(ctx, types.NamespacedName{
+					Name:      istioName + podMonitorNameSuffix,
+					Namespace: tt.expectPMNamespace,
+				}, pm)
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(pm.Name).To(Equal(istioName + podMonitorNameSuffix))
+				expectMonitoringLabels(g, pm.Labels, podMonitorMonitoring)
+				g.Expect(pm.OwnerReferences).To(HaveLen(1))
+				g.Expect(pm.OwnerReferences[0].Kind).To(Equal(v1.IstioKind))
+				g.Expect(pm.OwnerReferences[0].Name).To(Equal(istioName))
+				g.Expect(pm.OwnerReferences[0].Controller).ToNot(BeNil())
+				g.Expect(*pm.OwnerReferences[0].Controller).To(BeTrue())
+			}


nrfox · 2026-09-17T16:19:34Z

+				{
+					ObjectMeta: revisionMeta,
+					Spec: v1.IstioRevisionSpec{
+						Version:   "v1.24.0",


Why is the version for Istio v1.29.2 and the version of the IstioRevision v1.24.0? It may not affect the tests but looks odd. You may not need to specify the version at all. It should get defaulted.

nrfox · 2026-09-17T16:26:48Z

+	}
+}
+
+func TestReconcilePodMonitors(t *testing.T) {


Are these tests duplicates of TestReconcile?

nrfox · 2026-09-17T16:28:15Z

+			OwnerReferences: []metav1.OwnerReference{
+				{
+					APIVersion: v1.GroupVersion.String(),
+					Kind:       v1.IstioRevisionKind,


Shouldn't this be the Istio resource and not the IstioRevision similar to the PodMonitor?

nrfox · 2026-09-17T16:34:15Z

+helm repo add prometheus-community https://prometheus-community.github.io/helm-charts --force-update
+helm repo update
+
+# Admission webhooks require a certgen Job that talks to the webhook Service. On KinD
+# that Job frequently times out (the webhook Service is not reachable until kube-proxy
+# is healthy), which would also make helm --wait hang. The monitoring e2e only needs CRDs
+# and scrape targets, so webhooks are disabled. --wait then tracks the operator Deployment.
+helm install "${PROM_RELEASE}" prometheus-community/kube-prometheus-stack \


Doing helm repo add and helm repo update will have side effects on the machine this hack script is run on. Can you install the helm chart using --repo and the url directly?

nrfox · 2026-09-17T16:44:54Z

We'll probably want to expose a util package for installing the integration dependencies and handling the differences between helm vs. OLM but just keep this as is for now.

istio-testing added the do-not-merge/work-in-progress label May 18, 2026

istio-testing added size/XL needs-ok-to-test labels May 18, 2026

istio-testing added needs-rebase size/XXL and removed size/XL labels May 22, 2026

mayleighnmyers changed the title ~~Add monitoring controller for COO (Cluster Observability Operator)~~ [WIP] Add monitoring controller for COO (Cluster Observability Operator) May 28, 2026

yxun mentioned this pull request Jun 1, 2026

Add draft SEP for observability integration #1986

Closed

nrfox reviewed Jun 8, 2026

View reviewed changes

Comment thread enhancements/SEP3-observability-integration.md Outdated

Comment thread enhancements/SEP3-observability-integration.md Outdated

Comment thread enhancements/SEP3-observability-integration.md Outdated

mayleighnmyers changed the title ~~[WIP] Add monitoring controller for COO (Cluster Observability Operator)~~ Add monitoring controller for metrics Jun 15, 2026

mayleighnmyers marked this pull request as ready for review June 15, 2026 17:38

mayleighnmyers requested a review from a team as a code owner June 15, 2026 17:38

istio-testing removed the do-not-merge/work-in-progress label Jun 15, 2026

mayleighnmyers force-pushed the feature/coo-monitoring-controller branch from 22566a7 to c43fabb Compare June 16, 2026 16:36

yxun added the ok-to-test label Jun 16, 2026

mayleighnmyers force-pushed the feature/coo-monitoring-controller branch from c43fabb to 21661f3 Compare June 16, 2026 16:48

istio-testing removed needs-rebase needs-ok-to-test labels Jun 16, 2026

mayleighnmyers force-pushed the feature/coo-monitoring-controller branch from 795caee to 3373e36 Compare June 16, 2026 19:22

istio-testing added the needs-rebase label Jun 24, 2026

mayleighnmyers force-pushed the feature/coo-monitoring-controller branch from 25aa652 to 3c63e82 Compare June 24, 2026 14:14

istio-testing removed the needs-rebase label Jun 24, 2026

mayleighnmyers mentioned this pull request Jun 24, 2026

enhancements: add SEP for metrics integration #2028

Merged

5 tasks

istio-testing added the needs-rebase label Jul 14, 2026

mayleighnmyers and others added 25 commits September 9, 2026 11:20

Made changes based off request

7ff4d23

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Add monitoring.enabled field to Istio CR for COO integration

313facb

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Changed back to structured

eb7e612

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Add draft SEP for observability integration

5bd5317

Signed-off-by: Yuanlin Xu <yuanlin.xu@redhat.com> Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Updated SEP, Relabel update

d87d187

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

update sep3 doc

7898717

Signed-off-by: Yuanlin Xu <yuanlin.xu@redhat.com>

Add MetricTuningRelabelings rules

e4de6ba

Signed-off-by: Yuanlin Xu <yuanlin.xu@redhat.com>

Fixing CI job test failures

e1eec26

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Make gen

582d73e

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

remove SEP

2e0a0ad

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Added istio.io/rev label to be watched

f2226c0

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Fix rhobs monitoring scheme registration

923600e

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Add E2E testing as well as integration of kube-prometheus-stack

0b78ff7

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Fix lint errors

bf5f2f6

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Align monitoring labels and KinD e2e with kube-prometheus-stack defaults

a899255

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Fixed link and ran make gen

fb5792e

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Simplifying API changes with an annotation

c3a30ec

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Removed things dealing with COO

4612006

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Made updated changes

80a9ff0

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Adding test coverages

b2e2425

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Changed it to reconcile Istio

904feef

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Made changes based off comments

d219172

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Add Changelog

0bdbe1e

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

Suggested changes

134238e

Signed-off-by: Mayleigh Tjapkes <mamyers@redhat.com>

mayleighnmyers force-pushed the feature/coo-monitoring-controller branch from 5d0d01c to 134238e Compare September 11, 2026 13:34

istio-testing removed the needs-rebase label Sep 12, 2026

mayleighnmyers requested a review from nrfox September 15, 2026 16:14

nrfox reviewed Sep 17, 2026

View reviewed changes

Conversation

mayleighnmyers commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Additional information:

Uh oh!

linux-foundation-easycla Bot commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

istio-testing commented May 18, 2026

Uh oh!

nrfox commented May 28, 2026

Uh oh!

mayleighnmyers commented May 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nrfox commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mayleighnmyers commented Jul 10, 2026

Uh oh!

mayleighnmyers commented Sep 11, 2026

Uh oh!

nrfox left a comment

Choose a reason for hiding this comment

Uh oh!

nrfox Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

nrfox Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

nrfox Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

nrfox Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

nrfox Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

nrfox Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

nrfox Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mayleighnmyers commented May 18, 2026 •

edited

Loading

linux-foundation-easycla Bot commented May 18, 2026 •

edited

Loading

nrfox commented Jun 9, 2026 •

edited

Loading