From 02f68b7599f55a968c090889bf180e26fb89686d Mon Sep 17 00:00:00 2001 From: Todd Yan Date: Thu, 29 Feb 2024 23:30:40 -0800 Subject: [PATCH 1/4] Adding disableautomaticprometheues --- .../default-prometheus-annotation-bugfix.yaml | 16 ++++++++++++++++ apis/v1alpha1/collector_webhook.go | 4 ++++ apis/v1alpha1/opentelemetrycollector_types.go | 7 +++++++ internal/manifests/collector/annotations.go | 4 ++-- internal/manifests/collector/annotations_test.go | 1 + 5 files changed, 30 insertions(+), 2 deletions(-) create mode 100755 .chloggen/default-prometheus-annotation-bugfix.yaml diff --git a/.chloggen/default-prometheus-annotation-bugfix.yaml b/.chloggen/default-prometheus-annotation-bugfix.yaml new file mode 100755 index 0000000000..d3f29c26c0 --- /dev/null +++ b/.chloggen/default-prometheus-annotation-bugfix.yaml @@ -0,0 +1,16 @@ +# 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. operator, target allocator, github action) +component: operator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Changing one of the fields to follow naming conventions with lowerCamelCase + +# One or more tracking issues related to the change +issues: [2608] + +# (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: diff --git a/apis/v1alpha1/collector_webhook.go b/apis/v1alpha1/collector_webhook.go index 97ee9cc3a4..2b9ecfdb19 100644 --- a/apis/v1alpha1/collector_webhook.go +++ b/apis/v1alpha1/collector_webhook.go @@ -260,6 +260,10 @@ func (c CollectorWebhook) validate(ctx context.Context, r *OpenTelemetryCollecto maxReplicas = r.Spec.MaxReplicas } + if r.Spec.Observability.Metrics.DisablePrometheusAnnotations { + warnings = append(warnings, "DisablePrometheusAnnotations is deprecated, use DisableAutomaticPrometheusAnnotations instead") + } + var minReplicas *int32 if r.Spec.Autoscaler != nil && r.Spec.Autoscaler.MinReplicas != nil { minReplicas = r.Spec.Autoscaler.MinReplicas diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 1647ca54a9..c1fa0144a8 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -527,7 +527,14 @@ type MetricsConfigSpec struct { // // +optional // +kubebuilder:validation:Optional + // Deprecated: use Spec.Observability.Metrics.DisableAutomaticPrometheusAnnotations instead. DisablePrometheusAnnotations bool `json:"DisablePrometheusAnnotations,omitempty"` + // DisableAutomaticPrometheusAnnotations controls the automatic addition of default Prometheus annotations + // ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path') + // + // +optional + // +kubebuilder:validation:Optional + DisableAutomaticPrometheusAnnotations bool `json:"disableAutomaticPrometheusAnnotations,omitempty"` } // ObservabilitySpec defines how telemetry data gets handled. diff --git a/internal/manifests/collector/annotations.go b/internal/manifests/collector/annotations.go index 5f4c7c41be..55fc059f30 100644 --- a/internal/manifests/collector/annotations.go +++ b/internal/manifests/collector/annotations.go @@ -27,8 +27,8 @@ func Annotations(instance v1beta1.OpenTelemetryCollector) (map[string]string, er // new map every time, so that we don't touch the instance's annotations annotations := map[string]string{} - // Enable Prometheus annotations by default if DisablePrometheusAnnotations is nil or true - if !instance.Spec.Observability.Metrics.DisablePrometheusAnnotations { + // Enable Prometheus annotations by default if DisablePrometheusAnnotations|DisableAutomaticPrometheusAnnotations is nil or true + if !instance.Spec.Observability.Metrics.DisablePrometheusAnnotations || !instance.Spec.Observability.Metrics.DisableAutomaticPrometheusAnnotations { // Set default Prometheus annotations annotations["prometheus.io/scrape"] = "true" annotations["prometheus.io/port"] = "8888" diff --git a/internal/manifests/collector/annotations_test.go b/internal/manifests/collector/annotations_test.go index d65bb41b27..1fa09085c2 100644 --- a/internal/manifests/collector/annotations_test.go +++ b/internal/manifests/collector/annotations_test.go @@ -72,6 +72,7 @@ func TestNonDefaultPodAnnotation(t *testing.T) { Observability: v1beta1.ObservabilitySpec{ Metrics: v1beta1.MetricsConfigSpec{ DisablePrometheusAnnotations: true, + DisableAutomaticPrometheusAnnotations: true, }, }, }, From 3a1306e9b54171bd53ce3442a87af821928d40bf Mon Sep 17 00:00:00 2001 From: Todd Yan Date: Thu, 29 Feb 2024 23:38:53 -0800 Subject: [PATCH 2/4] Adding docs --- apis/v1alpha1/opentelemetrycollector_types.go | 2 +- ...entelemetry.io_opentelemetrycollectors.yaml | 12 +++++++++++- ...entelemetry.io_opentelemetrycollectors.yaml | 12 +++++++++++- docs/api.md | 18 ++++++++++++++++-- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index c1fa0144a8..f6907e1fcd 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -527,7 +527,7 @@ type MetricsConfigSpec struct { // // +optional // +kubebuilder:validation:Optional - // Deprecated: use Spec.Observability.Metrics.DisableAutomaticPrometheusAnnotations instead. + // Deprecated: use "OpenTelemetryCollector.Spec.Observability.Metrics.DisableAutomaticPrometheusAnnotations" instead. DisablePrometheusAnnotations bool `json:"DisablePrometheusAnnotations,omitempty"` // DisableAutomaticPrometheusAnnotations controls the automatic addition of default Prometheus annotations // ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path') diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index cf15404595..4393f6a3ee 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -3870,7 +3870,12 @@ spec: DisablePrometheusAnnotations: description: DisablePrometheusAnnotations controls the automatic addition of default Prometheus annotations ('prometheus.io/scrape', - 'prometheus.io/port', and 'prometheus.io/path') + 'prometheus.io/port', and 'prometheus. + type: boolean + disableAutomaticPrometheusAnnotations: + description: DisableAutomaticPrometheusAnnotations controls + the automatic addition of default Prometheus annotations + ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path') type: boolean enableMetrics: description: EnableMetrics specifies if ServiceMonitor or @@ -5214,6 +5219,11 @@ spec: DisablePrometheusAnnotations: description: DisablePrometheusAnnotations controls the automatic addition of default Prometheus annotations + ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus. + type: boolean + disableAutomaticPrometheusAnnotations: + description: DisableAutomaticPrometheusAnnotations controls + the automatic addition of default Prometheus annotations ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path') type: boolean enableMetrics: diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 5ed95c6145..458d338066 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -3867,7 +3867,12 @@ spec: DisablePrometheusAnnotations: description: DisablePrometheusAnnotations controls the automatic addition of default Prometheus annotations ('prometheus.io/scrape', - 'prometheus.io/port', and 'prometheus.io/path') + 'prometheus.io/port', and 'prometheus. + type: boolean + disableAutomaticPrometheusAnnotations: + description: DisableAutomaticPrometheusAnnotations controls + the automatic addition of default Prometheus annotations + ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path') type: boolean enableMetrics: description: EnableMetrics specifies if ServiceMonitor or @@ -5211,6 +5216,11 @@ spec: DisablePrometheusAnnotations: description: DisablePrometheusAnnotations controls the automatic addition of default Prometheus annotations + ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus. + type: boolean + disableAutomaticPrometheusAnnotations: + description: DisableAutomaticPrometheusAnnotations controls + the automatic addition of default Prometheus annotations ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path') type: boolean enableMetrics: diff --git a/docs/api.md b/docs/api.md index a586f024e8..ae445563a9 100644 --- a/docs/api.md +++ b/docs/api.md @@ -17815,7 +17815,14 @@ Metrics defines the metrics configuration for operands. DisablePrometheusAnnotations boolean - DisablePrometheusAnnotations controls the automatic addition of default Prometheus annotations ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path')
+ DisablePrometheusAnnotations controls the automatic addition of default Prometheus annotations ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.
+ + false + + disableAutomaticPrometheusAnnotations + boolean + + DisableAutomaticPrometheusAnnotations controls the automatic addition of default Prometheus annotations ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path')
false @@ -20325,7 +20332,14 @@ Metrics defines the metrics configuration for operands. DisablePrometheusAnnotations boolean - DisablePrometheusAnnotations controls the automatic addition of default Prometheus annotations ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path')
+ DisablePrometheusAnnotations controls the automatic addition of default Prometheus annotations ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.
+ + false + + disableAutomaticPrometheusAnnotations + boolean + + DisableAutomaticPrometheusAnnotations controls the automatic addition of default Prometheus annotations ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path')
false From e984e99c1f3e5a00bf502672e8350aac9d152903 Mon Sep 17 00:00:00 2001 From: Todd Yan Date: Thu, 29 Feb 2024 23:51:13 -0800 Subject: [PATCH 3/4] formatting --- apis/v1alpha1/opentelemetrycollector_types.go | 2 +- internal/manifests/collector/annotations_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index f6907e1fcd..ec9b403111 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -527,7 +527,7 @@ type MetricsConfigSpec struct { // // +optional // +kubebuilder:validation:Optional - // Deprecated: use "OpenTelemetryCollector.Spec.Observability.Metrics.DisableAutomaticPrometheusAnnotations" instead. + // Deprecated: use "OpenTelemetryCollector.Spec.Observability.Metrics.DisableAutomaticPrometheusAnnotations" instead DisablePrometheusAnnotations bool `json:"DisablePrometheusAnnotations,omitempty"` // DisableAutomaticPrometheusAnnotations controls the automatic addition of default Prometheus annotations // ('prometheus.io/scrape', 'prometheus.io/port', and 'prometheus.io/path') diff --git a/internal/manifests/collector/annotations_test.go b/internal/manifests/collector/annotations_test.go index 1fa09085c2..fa7d042b9c 100644 --- a/internal/manifests/collector/annotations_test.go +++ b/internal/manifests/collector/annotations_test.go @@ -71,7 +71,7 @@ func TestNonDefaultPodAnnotation(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{ Observability: v1beta1.ObservabilitySpec{ Metrics: v1beta1.MetricsConfigSpec{ - DisablePrometheusAnnotations: true, + DisablePrometheusAnnotations: true, DisableAutomaticPrometheusAnnotations: true, }, }, From 9bf454ed303ae5fa0fed701d491a6f3172e176c7 Mon Sep 17 00:00:00 2001 From: Todd Yan Date: Wed, 6 Mar 2024 20:51:30 -0800 Subject: [PATCH 4/4] Add additional logic for these disable variables --- internal/manifests/collector/annotations.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/internal/manifests/collector/annotations.go b/internal/manifests/collector/annotations.go index 55fc059f30..10759afbd9 100644 --- a/internal/manifests/collector/annotations.go +++ b/internal/manifests/collector/annotations.go @@ -27,8 +27,25 @@ func Annotations(instance v1beta1.OpenTelemetryCollector) (map[string]string, er // new map every time, so that we don't touch the instance's annotations annotations := map[string]string{} + var enableAnnotations bool + // enableAnnotations is true only if both disable variables are false. + // DisableAutomaticPrometheusAnnotations takes precedence over DisablePrometheusAnnotations. + if instance.Spec.Observability.Metrics.DisableAutomaticPrometheusAnnotations { + enableAnnotations = !instance.Spec.Observability.Metrics.DisableAutomaticPrometheusAnnotations + } else if instance.Spec.Observability.Metrics.DisablePrometheusAnnotations { + enableAnnotations = !instance.Spec.Observability.Metrics.DisablePrometheusAnnotations + } else { + enableAnnotations = true + } + if enableAnnotations { + // Set default Prometheus annotations + annotations["prometheus.io/scrape"] = "true" + annotations["prometheus.io/port"] = "8888" + annotations["prometheus.io/path"] = "/metrics" + } + // Enable Prometheus annotations by default if DisablePrometheusAnnotations|DisableAutomaticPrometheusAnnotations is nil or true - if !instance.Spec.Observability.Metrics.DisablePrometheusAnnotations || !instance.Spec.Observability.Metrics.DisableAutomaticPrometheusAnnotations { + if !instance.Spec.Observability.Metrics.DisableAutomaticPrometheusAnnotations { // Set default Prometheus annotations annotations["prometheus.io/scrape"] = "true" annotations["prometheus.io/port"] = "8888"