[reckless-sherixx]

Summary

One values key, logs.collection, chooses where a cluster's container logs are collected:

• none (default): nothing changes, apart from one collector restart on upgrade.
• node-agent: the node-agent ships container stdout/stderr through the bundled OpenTelemetry collector into ClickHouse, except from kube-system and the agent's own namespace, and the heartbeat reports clickhouseLogsCollected: true.

The flag comes only from the switch, so it can't claim logs the chart doesn't collect (follow-up 1 from the #660 review).

Before merge: bump nodeAgent.image.tag to the release with nudgebee/node-agent#405, which adds the namespace/pod/container labels.

How to verify

1. Install with defaults: clickhouseLogsCollected: false, and the node-agent pod spec is unchanged (no LOGS_ENDPOINT or NODE_NAME), so node-agents don't restart.
2. Upgrade with --set logs.collection=node-agent: the node-agent gets LOGS_ENDPOINT (the bundled collector) and NODE_NAME, the heartbeat reports true, and otel_logs fills with pod logs, none from kube-system or the release namespace.
3. With the collector off, the node-agent off, or your own LOGS_ENDPOINT, the render fails and names the fix.

Risks

• Logs share ClickHouse's disk (50Gi) and 168h retention with traces; only the two namespaces are dropped.
• Delivery is at-most-once: lines written while the node-agent is down are lost.

Type of change

• New feature (non-breaking)

Chart version

• I bumped version in charts/nudgebee-agent/Chart.yaml (0.1.30; feat(chart,runner): query_data reads through a read-only ClickHouse user with query limits #671 took 0.1.29)

Test plan

• helm lint / ct lint (CI)
• helm template output reviewed; the new helm-dev-lint step asserts each mode (an empty value included), each refusal, NODE_NAME only in node-agent, and the namespace filter in the logs pipeline
• Installed on a kind cluster and verified the change

Related issues

• Follow-up 1 from the feat(runner): report ClickHouse logs table health beside the logs provider #660 review.
• Needs feat(logs): label exported container logs with Kubernetes attributes, stream and line time node-agent#405 in the node-agent image (see Before merge). Rebased onto feat(chart,runner): query_data reads through a read-only ClickHouse user with query limits #671, now merged; the kind e2e ran with both.

Details

• Collector filter. The collector's logs pipeline gains filter/drop_log_namespaces, so the collector restarts once on upgrade in every mode; the subchart's config is plain values, so the filter can't follow the mode. In none it drops only kube-system and release-namespace logs an operator sends the collector themselves (e.g. nodeAgent.logsEndpoint pointed at it).
• --reuse-values from an older release keeps that release's collector config, without the filter. Use --reset-then-reuse-values (Helm 3.14+) or pass the values file.
• Retention. A separate, shorter logs TTL is a follow-up: the ClickHouse exporter has one ttl for all its tables and applies it only when it creates them.
• Chart.
  • nudgebee-agent.logsCollection validates the value. The runner template always calls it, so a bad value fails even with the node-agent off.
  • otel-daemonset is reserved and fails with "not available yet".
  • An empty or missing value (--reuse-values from an older release) means none.
  • An operator's nodeAgent.logsEndpoint under none still works, but nothing reports it as collected.
  • In node-agent mode only, the node-agent gets NODE_NAME from spec.nodeName, unless nodeAgent.env already sets it. With defaults, with none, and with none plus nodeAgent.logsEndpoint, the node-agent DaemonSet's .spec renders byte-identical to main's.
  • The collector's logs pipeline runs filter/drop_log_namespaces after memory_limiter. It drops records whose k8s.namespace.name is kube-system or the release namespace; the subchart fills in {{ .Release.Namespace }}. Checked with otelcol-contrib validate (0.157.0). In a local run of that collector with the rendered config, of logs sent from kube-system, the release namespace, shop and no namespace, only shop and the unlabelled one came out.
• Runner. CLICKHOUSE_LOGS_COLLECTED becomes clickhouseLogsCollected on the heartbeat. It is always sent, so false overwrites a stale true.
• Tests. Config (default and set), the heartbeat field passing through, and the key always marshalled. The runner CI ran in Linux (gofmt -s, vet, golangci-lint v2.9.0, go test -race ./...), again after the rebase onto feat(chart,runner): query_data reads through a read-only ClickHouse user with query limits #671, as did every helm-dev-lint step (this PR's and feat(chart,runner): query_data reads through a read-only ClickHouse user with query limits #671's read-only user step).
• kind e2e (v1.31; helm v3.14.0; run before the second review's changes, which were checked by render diff and a local collector run):
  • Setup: this branch merged with feat(chart,runner): query_data reads through a read-only ClickHouse user with query limits #671, the runner image built from it, and the node-agent built from feat(logs): label exported container logs with Kubernetes attributes, stream and line time node-agent#405.
  • Heartbeats went to an in-cluster sink. Deployment checkout in namespace shop wrote to stdout and stderr.

Check	Result
node-agent env	LOGS_ENDPOINT=http://nudgebee-agent-opentelemetry-collector:4318/v1/logs, NODE_NAME from spec.nodeName
runner env	CLICKHOUSE_LOGS_COLLECTED=true
heartbeat	clickhouseLogsCollected: true, clickhouseLogsConnection: true, clickhouseLogsTable: default.otel_logs
otel_logs rows by namespace (before the namespace filter)	shop 42, nudgebee-agent 28, kube-system 11
shop stderr line	namespace shop, pod checkout-…, container app, k8s.deployment.name=checkout, log.iostream=stderr, time = kubectl logs --timestamps to the ns

[gemini-code-assist]

Code Review

This pull request introduces a new configuration option logs.collection to the Helm chart to manage container log collection modes (none or node-agent). It adds validation logic in _helpers.tpl to ensure proper configuration when node-agent is selected, updates the daemonset to set the appropriate log endpoint and node name, and propagates the collection status (ClickHouseLogsCollected) through the runner agent's configuration and telemetry. The review feedback suggests robustly handling cases where logs is defined as a flat string in the Helm values, avoiding redundant nil guards for nodeAgent in the helper template, and safely accessing nodeAgent properties in the daemonset template to prevent potential nil pointer errors.

[mayankpande88]

The chart and runner side looks right:

• logs.collection validates its value and fails with a clear message for each unsupported setup.
• The runner reports clickhouseLogsCollected from the switch only.
• The key is sent on every heartbeat, so false overwrites a stale true.

Three things before merge:

1. Blocked on nudgebee/node-agent#405. That PR is still open and nodeAgent.image.tag isn't bumped here. As the description says, without it the log rows have no namespace or pod labels. Order: merge and release #405, then bump the tag in this PR, then merge this.
2. NODE_NAME is added in every mode. The NODE_NAME block in daemonset.yaml sits outside the node-agent mode check, so every install gets a changed node-agent pod spec. Every cluster's node-agent DaemonSet then restarts on upgrade, including clusters on none. If only the log exporter needs it, gate it on eq (include "nudgebee-agent.logsCollection" .) "node-agent". If the restart is intended, say so in the description.
3. Disk. All container stdout lands on the same 50Gi ClickHouse volume, with the same 168h TTL as traces, and nothing filters it. On a busy cluster, logs can fill the disk, and then trace ingestion stops too. That risk is listed, but before this mode gets turned on for real clusters, add an exclude list (at least kube-system and the agent's own namespace) or a separate, shorter logs TTL. A follow-up is fine if you track it.

[reckless-sherixx]

@mayankpande88 thanks. Updated in ebd49a3, rebased on main and still one commit.

1. node-agent#405. Agreed on the order: once chore: updated version #405 is merged and released, I'll bump nodeAgent.image.tag here, and this merges after that.
2. NODE_NAME. Fixed: it now sits inside the node-agent branch next to LOGS_ENDPOINT, and is still skipped when nodeAgent.env sets it. Only the log push reads it.
   • In three cases the node-agent DaemonSet's .spec renders byte-identical to main's, so no node-agent restarts: defaults, logs.collection=none, and none plus nodeAgent.logsEndpoint. Only the chart-version label on the DaemonSet's own metadata differs.
   • helm-dev-lint now checks that NODE_NAME is absent in none (and with an empty value) and comes from spec.nodeName in node-agent.
3. Disk. I added the exclude list. A filter/drop_log_namespaces processor in the collector's logs pipeline, after memory_limiter, drops records whose k8s.namespace.name is kube-system or the release namespace. The node-agent has no namespace filter for logs (IGNORE_CONTROL_PLANE only covers connection tracking), so the filter lives in the collector.
   • Checks: otelcol-contrib validate passes on 0.157.0. I also ran that collector locally with the rendered config and sent logs from kube-system, the release namespace, shop, and one with no namespace. Only shop and the unlabelled record came out.
   • It can't follow the mode. The subchart's config is plain values our templates can't change, so the filter is in every mode. Two consequences:
     • The collector restarts once on upgrade.
     • In none, it also drops kube-system and release-namespace logs that an operator sends to the bundled collector themselves, e.g. nodeAgent.logsEndpoint pointed at it as in the dev values.
   • TTL. A separate, shorter logs TTL can't be done in the chart. The ClickHouse exporter has one ttl for all its tables and applies it only when it creates them, and every install already has otel_logs at 168h. It needs an ALTER TABLE … MODIFY TTL on existing installs. I'll open a follow-up issue for that and link it here.