[amazon-cloudwatch-observability] feat: add ServiceMonitor/PodMonitor v2 OTLP scraping path - #329
Conversation
…path Route Prometheus ServiceMonitor/PodMonitor metrics through the otelContainerInsights v2 pipeline to CloudWatch using the Target Allocator's prometheusCR discovery: - _otel-container-insights-config.tpl: add the prometheus/cw_k8s_ci_v0_prometheuscr receiver (Target Allocator endpoint, mTLS) and the metrics/cw_k8s_ci_v0_prometheuscr pipeline (filter/scrape_metadata, metricstarttime, set_cluster_name, set_scope_prometheuscr, resourcedetection, batch) exported via otlphttp. - _helpers.tpl: add the cloudwatch-agent.otelCIScrapeEnabled helper (otelContainerInsights.enabled AND targetAgent AND serviceMonitor/podMonitor on). - cloudwatch-agent-custom-resource.yaml: render targetAllocator.enabled and prometheusCR.enabled on the AmazonCloudWatchAgent CR when otelCIScrape is on. - target-allocator-clusterrole.yaml / -clusterrolebinding.yaml: grant the TA RBAC to get/list/watch podmonitors and servicemonitors when otelCIScrape is on. - values.yaml: add otelContainerInsights.serviceMonitor.enabled and .podMonitor.enabled (default true). - operator-deployment.yaml: supporting wiring.
The prior comment attributed a $-collapsing pass to Helm. Helm passes the text
through unchanged; the two $$->$ collapses happen in the agent's OTel confmap
stack (legacy expandconverter + the resolver's built-in ${...} expansion), so
$$$1 lands as the literal $1 regex backreference. Comment-only; render unchanged.
| - "--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" |
There was a problem hiding this comment.
These --feature-gates values are hyphenated (multi-instrumentation), but the pinned operator only registers the unhyphenated forms, so it rejects the gate and CrashLoopBackOffs and nothing reconciles. This also looks out of scope here. Can we restore the unhyphenated line?
| ## Allocator's prometheusCR discovery. Defaults to true: when | ||
| ## otelContainerInsights is enabled, ServiceMonitor scraping is on unless | ||
| ## explicitly disabled here. Only takes effect when otelContainerInsights.enabled is true. | ||
| serviceMonitor: |
There was a problem hiding this comment.
serviceMonitor.enabled and podMonitor.enabled default to true, so the moment someone turns on otelContainerInsights they silently get prometheusCR discovery, whose CRD and RBAC this PR doesn't set up yet. Could we default them to false, or gate the path on the CRDs being present?
There was a problem hiding this comment.
The default-on behavior is intentional, the product goal is that enabling otelContainerInsights gives you working SM/PM scraping out of the box, and the surrounding work is built to make that safe: the operator's CRD-resilience change(#394) makes missing CRDs non-fatal (no crash/loop), and CRD bundling (#331) puts the CRDs in-cluster. So I'd like to keep SM/PM default-on rather than invert to opt-in.
| - set(attributes["cloudwatch.solution"], "k8s-otel-container-insights") | ||
| - set(attributes["cloudwatch.pipeline"], "efa") | ||
|
|
||
| transform/cw_k8s_ci_v0_set_scope_prometheuscr: |
There was a problem hiding this comment.
The scraping receiver and pipeline are guarded on serviceMonitor.enabled/podMonitor.enabled, but the transform/cw_k8s_ci_v0_set_scope_prometheuscr processor isn't, so turning both monitors off leaves it orphaned and the collector warns. Can we guard it the same way via otelCIScrapeEnabled?
| {{- if .Values.agent.enabled }} | ||
| {{- range $i, $customAgent := .Values.agents }} | ||
| {{- if and (and (hasKey ($customAgent.prometheus) "targetAllocator") (hasKey ($customAgent.prometheus.targetAllocator) "enabled")) $customAgent.prometheus.targetAllocator.enabled }} | ||
| {{- $otelCIScrape := eq (include "cloudwatch-agent.otelCIScrapeEnabled" (dict "agentName" $customAgent.name "context" $)) "true" }} |
There was a problem hiding this comment.
This renders the ClusterRole inside range .Values.agents with a static name, so two matching agents produce duplicate ClusterRoles and Kubernetes just keeps the last write. Could we namespace the name per agent (same for the ClusterRoleBinding), or document that only one agent is supported?
| {{- define "cloudwatch-agent.otelCIScrapeEnabled" -}} | ||
| {{- $ctx := .context -}} | ||
| {{- $agentName := .agentName -}} | ||
| {{- if and $ctx.Values.otelContainerInsights.enabled (eq $agentName $ctx.Values.otelContainerInsights.targetAgent) (or $ctx.Values.otelContainerInsights.serviceMonitor.enabled $ctx.Values.otelContainerInsights.podMonitor.enabled) -}} |
There was a problem hiding this comment.
This helper reads otelContainerInsights.serviceMonitor.enabled directly. Fine today since the PR ships those keys as defaults, but it goes nil if a user replaces the whole map, so a dig with a default fallback might be safer.
There was a problem hiding this comment.
Since the subkeys ship as chart defaults and Helm deep-merges them, they're present under any partial override, so this is safe as-is I'll leave the direct access
| prometheus/cw_k8s_ci_v0_prometheuscr: | ||
| target_allocator: | ||
| endpoint: https://{{ .Values.otelContainerInsights.targetAgent }}-target-allocator-service:80 | ||
| interval: {{ .Values.otelContainerInsights.metricResolution }} |
There was a problem hiding this comment.
interval here is the collector to Target Allocator refresh cadence, but it reuses otelContainerInsights.metricResolution, which is really the scrape cadence (prometheusCR.scrapeInterval). Worth a comment or a separate field so the two can diverge later?
There was a problem hiding this comment.
Agreed they're distinct knobs. I'd rather not add a second field preemptively, but the shared value being non-obvious is fair
The Target Allocator watches the ServiceMonitor/PodMonitor CRDs to start/stop informers as CRDs appear or disappear. Under the otelContainerInsights scraping path the TA ClusterRole granted list/watch on the monitors but not on customresourcedefinitions, so the operator-side CRD watch was denied, the CRD gate never opened, and SM/PM discovery failed quietly. Add the read-only get/list/watch grant on customresourcedefinitions beside the monitoring.coreos.com rule under the same $otelCIScrape gate. Comment-only render change; gated off by default.
This branch had flipped the operator --feature-gates to the hyphenated multi-instrumentation / multi-instrumentation.skip-container-validation form. The pinned operator (cloudwatch-agent-operator:3.5.0) registers the unhyphenated IDs (operator.autoinstrumentation.multiinstrumentation and .multiinstrumentation.skipcontainervalidation), so the hyphenated forms are unrecognized; the manager parses gates with ExitOnError and CrashLoopBackOffs, which stops all CR reconciliation and breaks the whole chart on install/upgrade. Out of scope for the scraping work; restore the unhyphenated IDs to match main.
…ment The transform/cw_k8s_ci_v0_set_scope_prometheuscr processor was defined unconditionally, while its receiver and pipeline are guarded by serviceMonitor.enabled OR podMonitor.enabled. With otelContainerInsights enabled but both monitors disabled, the processor rendered orphaned (declared, referenced by no pipeline), which the collector warns about. Wrap it in the same guard so the receiver, pipeline and processor stay in sync.
The TA ClusterRole and ClusterRoleBinding rendered inside range .Values.agents with a static object name, so if more than one agent satisfied the gate (which $otelCIScrape widens), two identically named objects rendered and Kubernetes applied last-write-wins, silently clobbering one. All TAs share one ServiceAccount (target-allocator-service-acct), so a single ClusterRole/Binding suffices: pre-scan agents into needsTA/needsCR and render exactly once, with the SM/PM + customresourcedefinitions rules included when any agent needs discovery. Renders once even with multiple matching agents; verified 1 object each.
|
Makes sense. The pinned cloudwatch-agent-target-allocator:1.0.0 predates the discovery path, and enabling scraping against it CrashLoops the TA rather than degrading, so a discovery-capable TA image must be pinned before this path is enabled (landing in the same release that exposes it, or the path stays off until then). |
…node-allocation Re-sync aws-observability#330 onto the updated scraping-path branch: inherit the feature-gate ID fix, co-gated CRD RBAC render-once ClusterRole/Binding, the $$$1 comment correction, and the prometheuscr scope-processor guard, alongside aws-observability#330's per-node work.
…bundling Re-sync onto the updated scraping-path branch to inherit the feature-gate ID fix, the co-gated customresourcedefinitions RBAC rule (render-once ClusterRole/ Binding), the $$$1 comment correction, and the prometheuscr scope-processor guard. Resolves the CRD-RBAC matrix assertion (now 11/0) and the feature-gate blocker on this branch.
…bundling Re-sync aws-observability#331 onto aws-observability#330 so the stack is linear (aws-observability#329 -> aws-observability#330 -> aws-observability#331): inherit the per-node allocationStrategy scoping/gating, the SM/PM node-stamping removal, and the context-aware allocationStrategy default, on top of aws-observability#331's CRD bundling.
…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.
2 participants
Summary
Add the ServiceMonitor/PodMonitor v2 OTLP scraping path to the chart: a
prometheuscrreceiver + pipeline driven by the Target Allocator's PrometheusCR discovery, plus the
enablement flags and gating helper. This is the foundation that lets the CloudWatch Agent
scrape community
monitoring.coreos.comServiceMonitor/PodMonitor CRs into CloudWatch viathe OTEL Container Insights path.
What this enables
When
otelContainerInsights.enabledis set, the chart wires the Target Allocator +prometheuscrreceiver so SM/PM CRs are discovered and scraped (default consistent-hashingallocation). ServiceMonitor and PodMonitor discovery can each be toggled via
otelContainerInsights.serviceMonitor.enabled/podMonitor.enabled(default on).Scope
prometheuscrreceiver + metrics pipeline in the OTEL Container Insights configotelCIScrapegating helperotelContainerInsights.serviceMonitor/podMonitor.enabledvalues (only take effect whenotelContainerInsights.enabled)This is the base of a small stack; per-node allocation + node-enrichment transforms land
in a follow-up that builds on this path.
Testing
helm template:otelContainerInsights.enabled=true→ theprometheuscrreceiver/pipeline renders.helm lintclean (pre-existingfluent-bit-windowsregion warning is unrelated).