[amazon-cloudwatch-observability] refactor: single-switch SM/PM scraping under otelContainerInsights.prometheusScrape by wenegiemepraise · Pull Request #332 · aws-observability/helm-charts

wenegiemepraise · 2026-07-09T11:23:04Z

Summary

Collapse the SM/PM scraping enablement into a single switch. Previously, turning on the
feature spanned several interacting flags (prometheusCRDs.install,
otelContainerInsights.serviceMonitor/podMonitor.enabled, deep
agent.prometheus.targetAllocator.*). This consolidates them under one nested bundle:

otelContainerInsights:
  enabled: true                 # the one switch: CRDs + Target Allocator + prometheusCR + RBAC
  prometheusScrape:
    enabled: true               # single opt-out: false => OTEL CI without SM/PM scraping
    allocationStrategy: per-node
    crds:
      install: auto             # auto|always|never
    serviceMonitor: { enabled: true }
    podMonitor:     { enabled: true }

otelContainerInsights.enabled: true now makes SM/PM scraping work end to end; a single
prometheusScrape.enabled: false opts out while keeping the rest of OTEL Container Insights.

Notes for reviewers

API/UX change. These keys never shipped in a released chart version, so there's no
external contract to preserve — the old prometheusCRDs / otelContainerInsights. serviceMonitor|podMonitor top-level keys are moved, not aliased.
Gating helpers in _helpers.tpl (otelCIScrapeEnabled, prometheusCRDsEnabled) now read
from the prometheusScrape bundle.

Dependencies

Top of the helm stack — refactors keys introduced by #329 (v2 OTLP path), #330
(per-node) and #331 (CRD bundling). Merge those first; this PR's diff includes them until
they land.

Testing

helm template:

otelContainerInsights.enabled=true → CRDs render (2), prometheuscr path renders,
allocationStrategy: "per-node".
+ prometheusScrape.enabled=false → prometheuscr and bundled CRDs both drop to 0
(scraping off, rest of CI intact).

helm lint clean (pre-existing fluent-bit-windows region warning is unrelated).

musa-asad · 2026-07-21T15:38:05Z

        - "--auto-instrumentation-nodejs-image={{ template "auto-instrumentation-nodejs.image" . }}"
        - "--target-allocator-image={{ template "target-allocator.image" (merge .Values.agent.prometheus.targetAllocator.image (dict "region" $.Values.region)) }}"
-        - "--feature-gates=operator.autoinstrumentation.multiinstrumentation,operator.autoinstrumentation.multiinstrumentation.skipcontainervalidation"
+        - "--feature-gates=operator.autoinstrumentation.multi-instrumentation,operator.autoinstrumentation.multi-instrumentation.skip-container-validation"


Same --feature-gates issue as #329: hyphenated multi-instrumentation IDs the pinned operator doesn't register, so it CrashLoopBackOffs. Could we fix it in #329 and rebase?

musa-asad · 2026-07-21T15:38:06Z

+  ## Allocator's prometheusCR discovery. When otelContainerInsights.enabled is true,
+  ## enabling this gives SM/PM scraping end-to-end (CRDs + Target Allocator +
+  ## prometheusCR + RBAC). Only takes effect when otelContainerInsights.enabled is true.
+  prometheusScrape:


The prometheusScrape bundle moves prometheusCRDs.install and the monitor flags to new prometheusScrape.* keys with no alias, so an existing --set prometheusCRDs.install=never is silently ignored. If we can't guarantee merge order, could we add aliases or a deprecation path?

musa-asad · 2026-07-21T15:38:06Z

  - nonResourceURLs: ["/metrics"]
    verbs: ["get"]
-  {{- if and (hasKey ($customAgent.prometheus.targetAllocator) "prometheusCR") $customAgent.prometheus.targetAllocator.prometheusCR.enabled }}
+  {{- if or (and (hasKey ($customAgent.prometheus) "targetAllocator") (hasKey ($customAgent.prometheus.targetAllocator) "prometheusCR") $customAgent.prometheus.targetAllocator.prometheusCR.enabled) $otelCIScrape }}


Same here: the SM/PM rule is present but the customresourcedefinitions rule (in #327) isn't, so this branch needs #327 to land alongside for the target-allocator to list CRDs. Could we make sure they merge together under the same gate?

musa-asad · 2026-07-21T15:38:07Z

@@ -0,0 +1,1429 @@
+{{- /*
+PodMonitor CRD (monitoring.coreos.com/v1), pinned to prometheus-operator v0.91.0 to


Header pins these CRDs to prometheus operator v0.91.0 but go.mod is on v0.92.0, so the API server prunes any field added between the two from a user's monitor. Could we refetch both from v0.92.0 (or fix the header)? Same in servicemonitor-crd.yaml.

musa-asad · 2026-07-21T15:38:08Z

@@ -6405,9 +6405,10 @@ spec:
                  allocationStrategy:


allocationStrategy gains per-node only in the crds/ copy, which Helm doesn't touch on upgrade, so upgrading a release whose CRD predates the enum makes the API server reject the rendered per-node. Could we move this CRD into templates/ and gate it like the bundled monitor CRDs?

musa-asad · 2026-07-21T15:38:09Z

+
+printf "\n${Y}== Target Allocator CRD RBAC ==${N}\n"
+# The TA needs customresourcedefinitions get;list;watch on the prometheus-CR path.
+expect_count "CRD RBAC present when otelCI on"       "$CRD_RBAC" 1 --set otelContainerInsights.enabled=true


The CRD_RBAC assertions on lines 76 to 78 expect customresourcedefinitions RBAC that lands in a separate branch, so this branch renders zero and set -euo pipefail fails the whole suite. Could we move them into that branch or guard them until the rule exists?

musa-asad · 2026-07-21T15:38:10Z

+  otelContainerInsights.prometheusScrape.enabled are both true.
+Returns the string "true" when CRDs should be rendered, empty otherwise.
+*/}}
+{{- define "amazon-cloudwatch-observability.prometheusCRDsEnabled" -}}


prometheusCRDsEnabled only matches always/never exactly, so a typo like nevr falls through and silently bundles the CRDs when otelCI is on. Could we add a fail branch for anything that isn't auto, always, or never?

musa-asad · 2026-07-21T15:38:11Z

+the configured targetAgent, and at least one of serviceMonitor/podMonitor is enabled.
+Accepts a dict with "agentName" (string) and "context" (root context $).
+*/}}
+{{- define "cloudwatch-agent.otelCIScrapeEnabled" -}}


otelCIScrapeEnabled no longer short circuits when both serviceMonitor.enabled and podMonitor.enabled are false, so prometheusScrape.enabled=true renders an idle target-allocator that bundles CRDs but discovers nothing. Could we reject that both false case and add a matrix assertion?

…cture Re-sync aws-observability#332 onto the updated stack (aws-observability#329->aws-observability#330->aws-observability#331). Resolutions favor the settled older-PR fixes applied onto aws-observability#332's prometheusScrape.* restructure: - prometheusCRDsEnabled: keep aws-observability#331's explicit auto branch + invalid-value fail, gated on aws-observability#332's otelContainerInsights.enabled && prometheusScrape.enabled. - allocationStrategy: use aws-observability#332's precomputed $allocationStrategy (per-node on the otelCI path via prometheusScrape.allocationStrategy, consistent-hashing for direct-TA) -- supersedes aws-observability#330's ternary while preserving its intent. - Fix a stale old-key guard (otelContainerInsights.serviceMonitor.enabled) left by auto-merge in the prometheuscr scope-processor to prometheusScrape.enabled. - Keep aws-observability#330's SM/PM node-stamping removal; port aws-observability#331's GitOps opt-out caveat into the new prometheusScrape.crds doc. Verified: lint clean, CRD matrix 21/0, flag matrix 8/8.

…ion-scraper-routing Re-sync aws-observability#339 (scraper routing) onto the full updated stack. Preserve aws-observability#339's annotation-routing feature (scraperRole: cluster-scraper) while adopting the older/settled fixes: the prometheusScrape restructure, legacy-key aliases + deprecation notice, the serviceMonitorEnabled/podMonitorEnabled helpers for the SM/PM selector guards, the feature-gate/CRD-RBAC/render-once/v0.92.0-CRD fixes, and per-node scoping. Verified: lint clean, CRD matrix 24/0, flag matrix 8/8, scraperRole renders, legacy prometheusCRDs.install alias honored.

…ion-scraper-routing Propagates the both-false prometheusScrape validation (reject scrape.enabled with no monitors). # Conflicts: # charts/amazon-cloudwatch-observability/tests/prometheus_crds_matrix.sh

…ion-scraper-routing Propagates the needsCR least-privilege fix (require targetAllocator.enabled).

…ion-scraper-routing Brings in aws-observability#332 plus origin/main (including the resolved cloudwatch-agent-custom-resource.yaml conflict). The only conflict here was the otelCIScrapeEnabled doc comment; the merged text keeps this branch's two-agent description (targetAgent or clusterScraperAgent) and aws-observability#332's note that individual monitor types are not checked by this helper.

…ion-scraper-routing Brings in the full chain (aws-observability#332, aws-observability#331, aws-observability#330, aws-observability#329) and origin/main. The cluster-scraper OTEL config conflicted structurally: main added the Karpenter/KEDA solution pipelines (aws-observability#325, aws-observability#345) in the same receivers / processors / pipelines sections where this branch adds its prometheuscr blocks, and git interleaved the {{- if }}/{{- end }} boundaries. Rather than untangle the markers, the file was reconstructed from main's version with this branch's three self-contained, individually-gated prometheuscr blocks re-inserted at their anchors (after each kubeStateMetrics block). Verified: all five cluster-scraper pipelines render (prometheuscr, karpenter, keda, apiserver, kube_state_metrics), scraperRole is still set only on the cluster-scraper, and the TA ClusterRole/Binding still render exactly once.

Aakash-Dantre · 2026-09-23T10:58:46Z

Checklist posted for the PR author to move into the PR description under a ## PR Checklist heading.

PR Checklist

Commits are squashed into a logical, reviewable set — single commit, authored by the PR author
Commits and PR description comply with Amazon internal guidelines
make passes locally — helm lint clean, helm template renders, tests/prometheus_crds_matrix.sh 30 passed / 0 failed
All GitHub Actions checks on the PR are passing
Integration test evidence: link to a passing run, or state N/A with the reason
New or updated integration test coverage: [test/otel/pernode] Add per-node + zero-step CRD E2E suite and EKS harness aws/amazon-cloudwatch-agent-test#720 (per_node_test.go, crd_bundling_test.go)
New functionality has unit tests; bug fixes have a reproducing test — chart render tests under tests/
Config translation changes include updated golden files — N/A, chart-only change
Breaking or customer-visible changes are called out in the PR description

Note for the description: this is a values-key restructure. The former top-level prometheusCRDs
key and otelContainerInsights.serviceMonitor / .podMonitor keys are moved, not aliased, to
otelContainerInsights.prometheusScrape.*. Any values file setting the old paths needs updating.

…ights.prometheusScrape Collapses SM/PM scraping enablement into a single switch. otelContainerInsights.enabled: true now makes SM/PM scraping work end to end; prometheusScrape.enabled: false opts out while keeping the rest of OTEL Container Insights. BREAKING (unreleased keys): the former top-level prometheusCRDs key and otelContainerInsights.serviceMonitor / .podMonitor keys are MOVED, not aliased, to otelContainerInsights.prometheusScrape.*. Any values file setting the old paths needs updating. These keys have not shipped in a released chart version, so there is no external contract to preserve.

wenegiemepraise mentioned this pull request Jul 15, 2026

[amazon-cloudwatch-observability] Route SM/PM to cluster-scraper by annotation #339

Open

musa-asad reviewed Jul 21, 2026

View reviewed changes

wenegiemepraise mentioned this pull request Jul 22, 2026

[amazon-cloudwatch-observability] feat: bundle ServiceMonitor/PodMonitor CRDs (zero-step install) #331

Open

wenegiemepraise added a commit to wenegiemepraise/helm-charts that referenced this pull request Jul 24, 2026

Merge helm-flag-restructure (aws-observability#332) into helm-annotat…

355cbdd

…ion-scraper-routing Propagates the needsCR least-privilege fix (require targetAllocator.enabled).

musa-asad previously approved these changes Jul 24, 2026

View reviewed changes

Aakash-Dantre dismissed musa-asad’s stale review via c09e0bd September 23, 2026 09:09

Aakash-Dantre force-pushed the helm-flag-restructure branch from bd1b7f2 to cc4de2a Compare September 23, 2026 12:28

Aakash-Dantre force-pushed the helm-flag-restructure branch from cc4de2a to af86da1 Compare September 23, 2026 12:42

Aakash-Dantre mentioned this pull request Oct 1, 2026

feat(amazon-cloudwatch-observability): ServiceMonitor/PodMonitor scraping via the Target Allocator #376

Closed

8 of 9 tasks

Aakash-Dantre mentioned this pull request Oct 20, 2026

feat(amazon-cloudwatch-observability): ServiceMonitor/PodMonitor scraping via the Target Allocator #406

Open

10 tasks done

Navigation Menu

Repository navigation

[amazon-cloudwatch-observability] refactor: single-switch SM/PM scraping under otelContainerInsights.prometheusScrape - #332

[amazon-cloudwatch-observability] refactor: single-switch SM/PM scraping under otelContainerInsights.prometheusScrape#332
wenegiemepraise wants to merge 1 commit into
aws-observability:mainfrom
wenegiemepraise:helm-flag-restructure

wenegiemepraise commented Jul 9, 2026

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

Aakash-Dantre commented Sep 23, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,1429 @@
		{{- /*
		PodMonitor CRD (monitoring.coreos.com/v1), pinned to prometheus-operator v0.91.0 to

Conversation

wenegiemepraise commented Jul 9, 2026

Summary

Notes for reviewers

Dependencies

Testing

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aakash-Dantre commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading

Aakash-Dantre commented Sep 23, 2026 •

edited

Loading