fix(collector): remove prometheus annotations on disablePrometheusAnnotations update (#5069)

praveen9354 · web-flow · commit 68c3c423f885 · 2026-07-13T11:21:25.000-04:00
* fix(collector): track operator-stamped prometheus annotations for clean removal When spec.observability.metrics.disablePrometheusAnnotations is toggled from false to true on an existing OpenTelemetryCollector, the pod template mutate path previously preserved any annotation that existed on the current resource but was absent from the desired render. This left the prometheus.io/scrape, prometheus.io/port, and prometheus.io/path annotations stuck on the rolled pods. The operator now stamps an ownership marker (collector.opentelemetry.io/prometheus-annotations-added=true) whenever PodAnnotations adds at least one default prometheus.io/* annotation. mutatePodTemplate consults that marker on the existing pod template and strips the prometheus.io/* keys + the marker only when the marker is present AND the keys are absent from the desired pod template. This preserves prometheus.io/* annotations the user set out of band on the same collector — the design @swiatekm landed at the 2026-06-04 SIG meeting. If the user pre-sets all three default prometheus.io/* values via metadata.annotations or spec.podAnnotations, the operator adds nothing on top and the marker is not stamped — the operator claims nothing. Upgrade compatibility: pre-upgrade collectors have no marker. On the first reconcile after upgrade with DisablePrometheusAnnotations=false, the marker is stamped and subsequent toggles work normally. If a user upgrades and immediately sets DisablePrometheusAnnotations=true, the operator never stamps the marker on the existing pod template (the stamp path is gated on !Disable), so the pre-upgrade prom annotations survive that single toggle as a documented one-time cost. The mutate test covers this scenario. Fixes #5043 Signed-off-by: praveen9354 <praveen9354@gmail.com> * fix(collector): rename prometheus-annotations-added marker to operator. prefix Per @swiatekm review on #5069, align the ownership-marker key with the operator.opentelemetry.io/ prefix already used for Service-tracking annotations in this repo (collector-headless-service, collector-monitoring-service, collector-service-type) rather than the collector.opentelemetry.io/ prefix introduced in the previous revision. Signed-off-by: praveen9354 <praveen9354@gmail.com> --------- Signed-off-by: praveen9354 <praveen9354@gmail.com>
diff --git a/.chloggen/fix-disable-prometheus-annotations-removal.yaml b/.chloggen/fix-disable-prometheus-annotations-removal.yaml
@@ -0,0 +1,26 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
+component: collector
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: "Honor `spec.observability.metrics.disablePrometheusAnnotations: true` on update by removing the operator-stamped prometheus.io annotations from the pod template, not just stopping new ones from being added."
+
+# One or more tracking issues related to the change
+issues: [5043]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  Previously the pod-template mutate path preserved any annotation that
+  existed on the current resource but was absent from the desired render,
+  so toggling disablePrometheusAnnotations from false to true on an
+  existing OpenTelemetryCollector left the prometheus.io/scrape, port,
+  and path annotations stuck on the rolled pods. The operator now stamps
+  an ownership marker (operator.opentelemetry.io/prometheus-annotations-added)
+  whenever it adds one of the default prometheus.io/* annotations, and
+  the mutate path strips those annotations only when the marker is
+  present on the existing pod template. This preserves prometheus.io/*
+  annotations the user set out of band on the same collector.
diff --git a/internal/controllers/testdata/build_collector_base.yaml b/internal/controllers/testdata/build_collector_base.yaml
@@ -23,6 +23,7 @@ spec:
     metadata:
       annotations:
         opentelemetry-operator-config/sha256: 2d266e55025628659355f1271b689d6fb53648ef6cd5595831f5835d18e59a25
+        operator.opentelemetry.io/prometheus-annotations-added: "true"
         prometheus.io/path: /metrics
         prometheus.io/port: "8888"
         prometheus.io/scrape: "true"
diff --git a/internal/controllers/testdata/build_collector_ingress.yaml b/internal/controllers/testdata/build_collector_ingress.yaml
@@ -23,6 +23,7 @@ spec:
     metadata:
       annotations:
         opentelemetry-operator-config/sha256: 2d266e55025628659355f1271b689d6fb53648ef6cd5595831f5835d18e59a25
+        operator.opentelemetry.io/prometheus-annotations-added: "true"
         prometheus.io/path: /metrics
         prometheus.io/port: "8888"
         prometheus.io/scrape: "true"
diff --git a/internal/controllers/testdata/build_collector_service_account.yaml b/internal/controllers/testdata/build_collector_service_account.yaml
@@ -23,6 +23,7 @@ spec:
     metadata:
       annotations:
         opentelemetry-operator-config/sha256: 2d266e55025628659355f1271b689d6fb53648ef6cd5595831f5835d18e59a25
+        operator.opentelemetry.io/prometheus-annotations-added: "true"
         prometheus.io/path: /metrics
         prometheus.io/port: "8888"
         prometheus.io/scrape: "true"
diff --git a/internal/controllers/testdata/build_collector_ta_cr_base.yaml b/internal/controllers/testdata/build_collector_ta_cr_base.yaml
@@ -24,6 +24,7 @@ spec:
     metadata:
       annotations:
         opentelemetry-operator-config/sha256: 42773025f65feaf30df59a306a9e38f1aaabe94c8310983beaddb7f648d699b0
+        operator.opentelemetry.io/prometheus-annotations-added: "true"
         prometheus.io/path: /metrics
         prometheus.io/port: "8888"
         prometheus.io/scrape: "true"
diff --git a/internal/controllers/testdata/build_collector_ta_cr_enable_metrics.yaml b/internal/controllers/testdata/build_collector_ta_cr_enable_metrics.yaml
@@ -24,6 +24,7 @@ spec:
     metadata:
       annotations:
         opentelemetry-operator-config/sha256: 42773025f65feaf30df59a306a9e38f1aaabe94c8310983beaddb7f648d699b0
+        operator.opentelemetry.io/prometheus-annotations-added: "true"
         prometheus.io/path: /metrics
         prometheus.io/port: "8888"
         prometheus.io/scrape: "true"
diff --git a/internal/manifests/collector/daemonset_test.go b/internal/manifests/collector/daemonset_test.go
@@ -49,10 +49,11 @@ func TestDaemonSetNewDefault(t *testing.T) {
 
 	// verify sha256 podAnnotation
 	expectedAnnotations := map[string]string{
-		"opentelemetry-operator-config/sha256": "fbcdae6a02b2115cd5ca4f34298202ab041d1dfe62edebfaadb48b1ee178231d",
-		"prometheus.io/path":                   "/metrics",
-		"prometheus.io/port":                   "8888",
-		"prometheus.io/scrape":                 "true",
+		"opentelemetry-operator-config/sha256":                   "fbcdae6a02b2115cd5ca4f34298202ab041d1dfe62edebfaadb48b1ee178231d",
+		"prometheus.io/path":                                     "/metrics",
+		"prometheus.io/port":                                     "8888",
+		"prometheus.io/scrape":                                   "true",
+		"operator.opentelemetry.io/prometheus-annotations-added": "true",
 	}
 	assert.Equal(t, expectedAnnotations, d.Spec.Template.Annotations)
 
@@ -294,11 +295,12 @@ func TestDaemonsetPodAnnotations(t *testing.T) {
 		"prometheus.io/path":                   "/metrics",
 		"prometheus.io/port":                   "8888",
 		"prometheus.io/scrape":                 "true",
+		"operator.opentelemetry.io/prometheus-annotations-added": "true",
 	}
 
 	// verify
 	assert.Equal(t, "my-instance-collector", ds.Name)
-	assert.Len(t, ds.Spec.Template.Annotations, 5)
+	assert.Len(t, ds.Spec.Template.Annotations, 6)
 	assert.Equal(t, expectedAnnotations, ds.Spec.Template.Annotations)
 }
 
diff --git a/internal/manifests/collector/deployment_test.go b/internal/manifests/collector/deployment_test.go
@@ -92,10 +92,11 @@ func TestDeploymentNewDefault(t *testing.T) {
 
 	// verify sha256 podAnnotation
 	expectedAnnotations := map[string]string{
-		"opentelemetry-operator-config/sha256": "fbcdae6a02b2115cd5ca4f34298202ab041d1dfe62edebfaadb48b1ee178231d",
-		"prometheus.io/path":                   "/metrics",
-		"prometheus.io/port":                   "8888",
-		"prometheus.io/scrape":                 "true",
+		"opentelemetry-operator-config/sha256":                   "fbcdae6a02b2115cd5ca4f34298202ab041d1dfe62edebfaadb48b1ee178231d",
+		"prometheus.io/path":                                     "/metrics",
+		"prometheus.io/port":                                     "8888",
+		"prometheus.io/scrape":                                   "true",
+		"operator.opentelemetry.io/prometheus-annotations-added": "true",
 	}
 	assert.Equal(t, expectedAnnotations, d.Spec.Template.Annotations)
 
@@ -157,10 +158,11 @@ func TestDeploymentPodAnnotations(t *testing.T) {
 		"prometheus.io/path":                   "/metrics",
 		"prometheus.io/port":                   "8888",
 		"prometheus.io/scrape":                 "true",
+		"operator.opentelemetry.io/prometheus-annotations-added": "true",
 	}
 
 	// verify
-	assert.Len(t, d.Spec.Template.Annotations, 5)
+	assert.Len(t, d.Spec.Template.Annotations, 6)
 	assert.Equal(t, "my-instance-collector", d.Name)
 	assert.Equal(t, expectedPodAnnotationValues, d.Spec.Template.Annotations)
 }
diff --git a/internal/manifests/collector/statefulset_test.go b/internal/manifests/collector/statefulset_test.go
@@ -53,10 +53,11 @@ func TestStatefulSetNewDefault(t *testing.T) {
 
 	// verify sha256 podAnnotation
 	expectedAnnotations := map[string]string{
-		"opentelemetry-operator-config/sha256": "fbcdae6a02b2115cd5ca4f34298202ab041d1dfe62edebfaadb48b1ee178231d",
-		"prometheus.io/path":                   "/metrics",
-		"prometheus.io/port":                   "8888",
-		"prometheus.io/scrape":                 "true",
+		"opentelemetry-operator-config/sha256":                   "fbcdae6a02b2115cd5ca4f34298202ab041d1dfe62edebfaadb48b1ee178231d",
+		"prometheus.io/path":                                     "/metrics",
+		"prometheus.io/port":                                     "8888",
+		"prometheus.io/scrape":                                   "true",
+		"operator.opentelemetry.io/prometheus-annotations-added": "true",
 	}
 	assert.Equal(t, expectedAnnotations, ss.Spec.Template.Annotations)
 
@@ -237,6 +238,7 @@ func TestStatefulSetPodAnnotations(t *testing.T) {
 		"prometheus.io/path":                   "/metrics",
 		"prometheus.io/port":                   "8888",
 		"prometheus.io/scrape":                 "true",
+		"operator.opentelemetry.io/prometheus-annotations-added": "true",
 	}
 	// verify
 	assert.Equal(t, "my-instance-collector", ss.Name)
diff --git a/internal/manifests/manifestutils/annotations.go b/internal/manifests/manifestutils/annotations.go
@@ -11,6 +11,14 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
 )
 
+// PrometheusAnnotationsAddedKey is the pod-template annotation the operator
+// stamps when it adds the default prometheus.io/* scrape annotations. The
+// mutate path uses its presence on an existing resource to decide whether the
+// operator is the owner of the prometheus.io/* annotations and may therefore
+// remove them when DisablePrometheusAnnotations is toggled to true. This
+// avoids clobbering prometheus.io/* annotations that the user set out of band.
+const PrometheusAnnotationsAddedKey = "operator.opentelemetry.io/prometheus-annotations-added"
+
 // Annotations return the annotations for OpenTelemetryCollector resources.
 func Annotations(instance v1beta1.OpenTelemetryCollector, filterAnnotations []string) (map[string]string, error) {
 	// new map every time, so that we don't touch the instance's annotations
@@ -59,11 +67,21 @@ func PodAnnotations(instance v1beta1.OpenTelemetryCollector, filterAnnotations [
 			"prometheus.io/path":   "/metrics",
 		}
 		// Default Prometheus annotations do not override existing
+		stamped := false
 		for kMeta, vMeta := range prometheusAnnotations {
 			if _, ok := podAnnotations[kMeta]; !ok {
 				podAnnotations[kMeta] = vMeta
+				stamped = true
 			}
 		}
+		// Stamp a marker only when the operator actually added at least one
+		// default prometheus.io/* annotation. The mutate path uses the marker
+		// to distinguish operator-stamped prom annotations from prom
+		// annotations the user supplied out of band. Both are removed together
+		// when DisablePrometheusAnnotations is toggled to true.
+		if stamped {
+			podAnnotations[PrometheusAnnotationsAddedKey] = "true"
+		}
 	}
 
 	// make sure sha256 for configMap is always calculated
diff --git a/internal/manifests/manifestutils/annotations_test.go b/internal/manifests/manifestutils/annotations_test.go
@@ -37,6 +37,8 @@ func TestDefaultAnnotations(t *testing.T) {
 	assert.Equal(t, "true", podAnnotations["prometheus.io/scrape"])
 	assert.Equal(t, "8888", podAnnotations["prometheus.io/port"])
 	assert.Equal(t, "/metrics", podAnnotations["prometheus.io/path"])
+	// the operator stamps an ownership marker when it adds default prom annotations
+	assert.Equal(t, "true", podAnnotations[PrometheusAnnotationsAddedKey])
 	assert.Equal(t, "5b3b62aa5e0a3c7250084c2b49190e30b72fc2ad352ffbaa699224e1aa900834", podAnnotations["opentelemetry-operator-config/sha256"])
 }
 
@@ -70,6 +72,8 @@ func TestNonDefaultPodAnnotation(t *testing.T) {
 	assert.NotContains(t, podAnnotations, "prometheus.io/scrape", "Prometheus scrape annotation should not exist in pod annotations")
 	assert.NotContains(t, podAnnotations, "prometheus.io/port", "Prometheus port annotation should not exist in pod annotations")
 	assert.NotContains(t, podAnnotations, "prometheus.io/path", "Prometheus path annotation should not exist in pod annotations")
+	// the ownership marker is not stamped when DisablePrometheusAnnotations=true
+	assert.NotContains(t, podAnnotations, PrometheusAnnotationsAddedKey, "operator-owned marker should not be stamped when DisablePrometheusAnnotations=true")
 	assert.Equal(t, "fbcdae6a02b2115cd5ca4f34298202ab041d1dfe62edebfaadb48b1ee178231d", podAnnotations["opentelemetry-operator-config/sha256"])
 }
 
@@ -104,6 +108,10 @@ func TestUserAnnotations(t *testing.T) {
 	assert.Equal(t, "false", annotations["prometheus.io/scrape"])
 	assert.Equal(t, "1234", annotations["prometheus.io/port"])
 	assert.Equal(t, "/test", annotations["prometheus.io/path"])
+	// the ownership marker is not stamped when the user pre-set all three
+	// default prom annotations on metadata.annotations — the operator added
+	// nothing on top, so it claims nothing.
+	assert.NotContains(t, podAnnotations, PrometheusAnnotationsAddedKey, "operator-owned marker should not be stamped when user supplied all default prom annotations")
 	assert.Equal(t, "29cb15a4b87f8c6284e7c3377f6b6c5c74519f5aee8ca39a90b3cf3ca2043c4d", podAnnotations["opentelemetry-operator-config/sha256"])
 }
 
diff --git a/internal/manifests/mutate.go b/internal/manifests/mutate.go
@@ -23,6 +23,7 @@ import (
 	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
+	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
 )
 
 type ImmutableFieldChangeErr struct {
@@ -390,11 +391,42 @@ func mutateIssuer(existing, desired *cmv1.Issuer) {
 	existing.Spec = desired.Spec
 }
 
+// operatorPrometheusAnnotationKeys is the set of pod-template annotation keys
+// the operator stamps when it adds the default prometheus.io/* scrape
+// annotations. The mutate path treats these as operator-owned only when the
+// marker manifestutils.PrometheusAnnotationsAddedKey is present on the
+// existing pod template; in that case any key in this set that is absent from
+// the desired pod template is stripped from the existing pod template before
+// the preserve-external-annotations merge runs. This lets the
+// spec.observability.metrics.disablePrometheusAnnotations feature toggle take
+// effect on already-running collectors without clobbering prometheus.io/*
+// annotations the user set out of band.
+var operatorPrometheusAnnotationKeys = []string{
+	"prometheus.io/scrape",
+	"prometheus.io/port",
+	"prometheus.io/path",
+	manifestutils.PrometheusAnnotationsAddedKey,
+}
+
 func mutatePodTemplate(existing, desired *corev1.PodTemplateSpec) error {
 	if err := mergeWithOverride(&existing.Labels, desired.Labels); err != nil {
 		return err
 	}
 
+	// Strip operator-stamped prometheus annotations when the marker on the
+	// existing pod template signals operator ownership and the desired pod
+	// template no longer includes them. See the comment on
+	// operatorPrometheusAnnotationKeys for the rationale.
+	if existing.Annotations != nil {
+		if _, hasMarker := existing.Annotations[manifestutils.PrometheusAnnotationsAddedKey]; hasMarker {
+			for _, key := range operatorPrometheusAnnotationKeys {
+				if _, inDesired := desired.Annotations[key]; !inDesired {
+					delete(existing.Annotations, key)
+				}
+			}
+		}
+	}
+
 	if err := mergeWithOverride(&existing.Annotations, desired.Annotations); err != nil {
 		return err
 	}
diff --git a/internal/manifests/mutate_test.go b/internal/manifests/mutate_test.go
