Repository navigation
feat(amazon-cloudwatch-observability): ServiceMonitor/PodMonitor scraping via the Target Allocator - #376
feat(amazon-cloudwatch-observability): ServiceMonitor/PodMonitor scraping via the Target Allocator#376Aakash-Dantre wants to merge 1 commit into
Conversation
| {{- define "amazon-cloudwatch-observability.prometheusCRDsEnabled" -}} | ||
| {{- $install := (dig "prometheusScrape" "crds" "install" "auto" .Values.otelContainerInsights) -}} | ||
| {{- /* Back-compat: honor the legacy top-level prometheusCRDs.install if set (deprecated). */ -}} | ||
| {{- if hasKey .Values "prometheusCRDs" -}} |
There was a problem hiding this comment.
prometheusCRDs.install and otelContainerInsights.serviceMonitor/podMonitor (lines 409/417) are handled as "deprecated legacy" keys, but none of them exist on main, so there's nothing to stay compatible with. They look like leftovers from earlier revisions. The fail on line 31 also names prometheusCRDs.install: setting prometheusScrape.crds.install=bogus gives "prometheusCRDs.install must be one of...", which names a key that isn't documented in values.yaml. Suggest removing the legacy paths and pointing the error at otelContainerInsights.prometheusScrape.crds.install.
There was a problem hiding this comment.
Leftovers from earlier revisions. Removed in 7c5e084, and the error now names otelContainerInsights.prometheusScrape.crds.install. Covered in prometheus_crds_matrix.sh.
| # normalized and bounded the same way. Richer OTel enrichment (k8sattributes | ||
| # pod/node, workload, etc.) is intentionally deferred to the Phase 3 enrichment work. | ||
| processors: | ||
| - filter/cw_k8s_ci_v0_scrape_metadata |
There was a problem hiding this comment.
This pipeline leaves out transform/cw_k8s_ci_v0_set_unit, while the cluster-scraper's prometheuscr pipeline and every other Prometheus-scraped node pipeline include it. So the same monitor gets different units depending on whether it's annotated for the cluster-scraper. For example, *_seconds gets unit s when routed and no unit when scraped per-node. Intended?
There was a problem hiding this comment.
Not intended. Added in 7c5e084, with a render test that checks both prometheusCR pipelines.
| */ -}} | ||
| {{- if eq (include "amazon-cloudwatch-observability.prometheusCRDsEnabled" .) "true" }} | ||
| {{- $crdName := "servicemonitors.monitoring.coreos.com" }} | ||
| {{- $existing := lookup "apiextensions.k8s.io/v1" "CustomResourceDefinition" "" $crdName }} |
There was a problem hiding this comment.
The values.yaml note covers Argo/Flux, but how does this behave for the EKS add-on? If the add-on renders the chart without cluster access, lookup is always empty, so the bundled v0.92.0 CRDs would always be emitted. On clusters that already run kube-prometheus-stack, I'd expect either an add-on conflict on install or, with OVERWRITE, their CRDs replaced by ours. Is crds.install defaulting to auto safe for the add-on?
There was a problem hiding this comment.
I don't think auto is safe there yet. With scraping now off by default the add-on renders nothing new, so I'll confirm how the add-on renders before enabling it, and use never for that path if lookup is always empty. Do you know the right contact?
|
Since aws/amazon-cloudwatch-agent-operator#414 and aws/amazon-cloudwatch-agent-operator#415 aren't merged or released yet and |
8129142 to
7c5e084
Compare
7c5e084 to
31720bd
Compare
Routes a ServiceMonitor/PodMonitor to the central cluster-scraper agent via the cloudwatch.aws.amazon.com/scraper: cluster-scraper annotation on the monitor CR. The cluster-scraper gets its own Target Allocator and prometheusCR with scraperRole: cluster-scraper and consistent-hashing allocation; the per-node agent keeps the default (empty) role and per-node allocation. The Target Allocator ClusterRole/ClusterRoleBinding render once (all Target Allocators share the target-allocator-service-acct service account), so a second Target-Allocator-bearing agent no longer produces a duplicate-named object. The bundled CRD's scraperRole field carries the enum: [cluster-scraper] constraint generated by the operator. Companion: aws/amazon-cloudwatch-agent-operator#399. otelContainerInsights.prometheusScrape.enabled defaults to false until operator and Target Allocator images with prometheusCR discovery, per-node allocation and scraper_role are released; enabling it requires pinning those images together. The per-node prometheusCR pipeline applies the same unit normalization as the cluster-scraper one, so a monitor's units do not depend on which agent scrapes it.
31720bd to
58bdae2
Compare
|
Replaced by #406: same commit, opened from a branch in this repository so the EKS integration jobs can run. They need repository secrets, which fork PRs do not get. All earlier review comments here are answered. |
Summary
ServiceMonitor/PodMonitor (
monitoring.coreos.com/v1) scraping for OTEL Container Insights, through the Target Allocator's prometheusCR discovery, under a single switch:otelContainerInsights.prometheusScrape.Supersedes #327, #329, #330, #331, #332 and #339. Those were successive revisions of one change: each later PR contained the earlier ones, and all targeted
mainwhile editing the samevalues.yamland_helpers.tpl. This PR is #339's content rebased onto currentmain. #327 is included in full (its CRD-watch RBAC rule is identical here) without the unrelated file deletions in that branch.What
otelContainerInsights.prometheusScrape.enabledturns on SM/PM scraping end to end: CRDs, Target Allocator, prometheusCR discovery, and RBAC.prometheusScrape.allocationStrategy(defaultper-node). Targets are assigned to the agent on their node; node-less targets fall back to consistent hashing.prometheusScrape.crds.install: auto | always | never(defaultauto). The SM/PM CRDs are pinned to prometheus-operator v0.92.0 to match the operator's client.helm.sh/resource-policy: keep, sohelm uninstalldoes not delete every ServiceMonitor in the cluster.clusterScraperAgentgets its own Target Allocator withscraperRole: cluster-scraperand a prometheusCR pipeline. Monitors annotatedcloudwatch.aws.amazon.com/scraper: cluster-scrapergo to it; the per-node agent keeps everything else.ClusterRole/ClusterRoleBindingrender once, since every TA sharestarget-allocator-service-acct. This fixes duplicate-named objects when two agents run a TA. The role addsget/list/watchoncustomresourcedefinitions, which the TA's CRD watch needs.prometheusScrape.serviceMonitor.enabled/podMonitor.enabledlimit discovery to one monitor type.Off by default until compatible images ship
This configures behaviour that exists only in aws/amazon-cloudwatch-agent-operator#414 and aws/amazon-cloudwatch-agent-operator#415 (CRD watch,
per-node,scraper_role). With an operator or TA image that predates those PRs, the Target Allocator crashloops at startup instead of degrading.prometheusScrape.enabledtherefore defaults tofalse, so enablingotelContainerInsightsalone renders nothing new, and this PR can merge independently of the operator release. Planned sequence:agent.prometheus.targetAllocator.image(currentlycloudwatch-agent-target-allocator:1.0.0) and the operator image to those releases, and flipsprometheusScrape.enabledtotrueGitOps note
The skip-if-exists CRD guard uses
lookup, which returns nothing underhelm template. In render-then-apply flows (Argo CD, Flux) the bundled CRDs are therefore always emitted and can overwrite a separately managed copy. Those users should setcrds.install: never. This is documented at the value.Testing
helm templateon this commit:scraperRoleper-nodeotelContainerInsights.enabled: false)otelContainerInsights.enabled: true(scraping still off by default)otelContainerInsights.enabled: true,prometheusScrape.enabled: truemain,otelContainerInsights.enabled: trueNew
tests/prometheus_crds_matrix.sh: 36 passed, 0 failed.PR Checklist
makepasses locally - on this commit:check_secrets,helm-lint,helm-flag-matrix(8/8),helm-yaml-injection(17/17) pass, andtests/prometheus_crds_matrix.shpasses 36/36.Build(make all) and all 36 Minikube scenarios: run. The three EKS jobs failed atConfigure AWS Credentialsbecause the fork has no AWS role secret, so the EKS scenarios have not run.TestServiceMonitorPodMonitorCRDsBundled,TestPerNodeAllocation,TestScraperRoleWiring,TestAnnotationRoutingPartitionand others.tests/prometheus_crds_matrix.sh(36 render cases) covers the switch and its off default, CRD install modes and the validation message, monitor-type toggles, RBAC rendering, unit normalization on both prometheusCR pipelines, and the rejection of invalid combinations.prometheusScrape.enabledis set totrue, which requires the operator and TA images from feat(otel-ci): add Ray metrics solution #414/feat(otel-ci): add Ray Serve request traces via the agent OTLP endpoint #415.