-
Notifications
You must be signed in to change notification settings - Fork 66
enhancements: add SEP for metrics integration #2028
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
Open
mayleighnmyers
wants to merge
5
commits into
istio-ecosystem:main
Choose a base branch
from
mayleighnmyers:enhancements/sep3-observability
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+144
−0
Open
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| |Status | Authors | Created | | ||
| |---------------------------------------------------|--------------|------------| | ||
| |Implementation | @mayleighnmyers, @yxun | 2026-06-01 | | ||
|
|
||
| # Observability Integration (Metrics) | ||
|
|
||
| ## Overview | ||
|
|
||
| Upstream Istio generates telemetry metrics for the control plane and sidecar proxies. Users can customize Istio metrics with the Telemetry API and create scraping jobs by updating a Prometheus ConfigMap. For example, [prometheus.yaml](https://raw.githubusercontent.com/istio/istio/master/samples/addons/prometheus.yaml). However, that approach requires restarting the Prometheus instance and users may not have enough permission to update the existing Prometheus's ConfigMap. | ||
|
|
||
| Alternatively, those can be configured by using two custom resources `ServiceMonitor` and `PodMonitor`. We want to automate the creation of `ServiceMonitor` and `PodMonitor` resources with default values in Sail Operator. And then users can query Istio metrics in Prometheus or Kiali dashboard without manual configuration step. | ||
|
|
||
| ## Goals | ||
|
|
||
| * Provide a monitoring controller that reconciles `ServiceMonitor` and `PodMonitor` resources for Istio control plane and sidecar proxy metrics. | ||
| * Apply platform-appropriate default relabeling rules on Kubernetes and OpenShift. | ||
|
|
||
| ## Non-goals | ||
|
|
||
| * Deploying observability stack components (Prometheus, OpenShift Cluster Observability Operator, OpenShift User-Workload Monitoring, etc.). We assume those are installed separately. | ||
| * Updating `ServiceMonitor` and `PodMonitor` for user customization of scraping paths, ports, or relabeling rules via the Sail Operator API. | ||
| * Observability Distributed Tracing integration. | ||
|
|
||
| ## Design | ||
|
|
||
| ### User Stories | ||
|
|
||
| 1. As a user running Istio with a Prometheus monitoring stack, I want the Sail Operator to configure metrics scraping jobs for Istio-generated telemetry metrics without restart the Prometheus instance. | ||
| 2. As a user running OpenShift Service Mesh(OSSM), I want the Sail Operator to create `PodMonitor` resources with OSSM-documented relabeling rules so metrics appear correctly in the OpenShift console and Kiali dashboard. | ||
|
|
||
| ### API Changes | ||
|
|
||
| We will add a boolean field `spec.monitoring.enabled` in the `Istio` Custom Resource(CR). For example, | ||
|
|
||
| ```yaml | ||
| apiVersion: sailoperator.io/v1 | ||
| kind: Istio | ||
| metadata: | ||
| name: default | ||
| spec: | ||
| monitoring: | ||
| enabled: false | ||
| ``` | ||
|
|
||
| This field defaults to `false`. When it is set to `true`, the monitoring controller reconciles `ServiceMonitor` and `PodMonitor` CRs for each owned `IstioRevision`. | ||
|
|
||
| #### Alternatives API Considered | ||
|
|
||
| A broader Observability Integration API with target references to a monitoring and tracing stacks is being discussed in the initial PR comments. We plan to design and implement the monitoring controller and broader Integration API controller separately. This enhancement proposal is focused on the monitoring controller with API change in the `Istio` Custom Resource. We will work toward a broader Integration API design and implementation in next phase. | ||
|
|
||
| ### Architecture | ||
|
|
||
| We assume `ServiceMonitor` and `PodMonitor` CRDs are available under `monitoring.coreos.com/v1` and/or `monitoring.rhobs/v1` group. The Sail Operator ClusterRole grants permissions for both API groups' `ServiceMonitor` and `PodMonitor` custom resources. | ||
|
|
||
| A monitoring controller watches `IstioRevision` resources and reconciles `ServiceMonitor` and `PodMonitor` CRs when the boolean field `spec.monitoring.enabled` is set to `true` in the parent `Istio` CR. It also watches namespaces with the `istio-injection=enabled` label and reconciles `PodMonitor` CRs for Istio sidecar injection namespaces. | ||
|
|
||
| The monitoring controller applies platform-specific relabeling defaults when creating `ServiceMonitor` and `PodMonitor` CRs. The monitoring controller starts and detects the running platform at the Sail Operator startup time. | ||
|
|
||
| #### Kubernetes vs OpenShift | ||
|
|
||
| On **Kubernetes** platform, applying a single `PodMonitor` CR in the control plane namespace with `namespaceSelector.any: true` is the preferred approach. That is matching upstream Istio Prometheus configuration example. | ||
| On **OpenShift** platform, the User-Workload Monitoring stack ignores `namespaceSelector` field, so we need create a `PodMonitor` CR in each sidecar injection enabled namespace. | ||
|
|
||
| ### Performance Impact | ||
|
|
||
| The `ServiceMonitor` reconciliation is only required in the control plane namespace and it has low performance impact. The `PodMonitor` reconciliation watches namespaces with the `istio-injection=enabled` label on OpenShift platform and it may impact performance on clusters when there are large number of existing namespaces. | ||
|
|
||
| ## Implementation Plan | ||
|
|
||
| v1alpha1 | ||
| - [x] Monitoring controller reconciling `ServiceMonitor` and `PodMonitor` for `IstioRevision` | ||
| - [x] `Istio.spec.monitoring.enabled` API | ||
| - [x] Platform-specific PodMonitor relabeling defaults | ||
| - [x] Unit tests for controller and relabeling package | ||
| - [ ] External CRD/API group detection (`monitoring.coreos.com` vs `monitoring.rhobs`) | ||
| - [ ] Kubernetes PodMonitor strategy using `namespaceSelector.any: true` | ||
| - [ ] Validation when monitoring CRDs are unavailable | ||
|
yxun marked this conversation as resolved.
Outdated
|
||
| - [ ] Integration tests (`tests/integration/api/monitoring_test.go`) | ||
| - [ ] KinD e2e tests | ||
| - [ ] User-facing documentation | ||
|
|
||
| v1alpha2 | ||
| - [ ] Integration API with target references for metrics and tracing | ||
|
|
||
| ## Test Plan | ||
|
|
||
| Functionality will be tested in unit tests, integration tests (envtest), and KinD e2e tests. | ||
|
|
||
| ## Updates | ||
|
|
||
| * 2026-06-11: Updated with platform relabeling implementation, remaining v1alpha1 work items. | ||
| * 2026-06-23: Updated Overview, Goals, Non-goals, Design descriptions. Added Alternatives API Considered. Removed OSSM JIRA references. | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does v1alpha1 mean in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It refers to API changes we will introduce for enabling this controller. The initial plan was simply adding a
spec.monitoring.enabledfield in the existing Istio CRD so we don't need thisv1alpha1.Nick suggested an alternative new API , see his comment in :
#1959 (comment)
If we will introduce a new API CRD such as
Integration, then we need to put it in av1alpha1version.We described this alternative API in the section "Alternatives API Considered" above.
That approach will introduce additional discussions about API changes and more general integration controller(s). I feel those need to be in follow up SEPs and this SEP focuses on metric monitoring controller first.
I will remove this
v1alpha1here. Thanks.