feat(heimdall): OTel Collector log-shipper DaemonSet#10
Merged
Conversation
…dition to OTel plan The merged OTel Collector log-shipper plan hardcoded the upstream-default `argocd` control-plane namespace in Task 6's deploy/verify commands, but every cluster has used `argo` since the 2026-03-21 namespace rename. Correct the `kubectl -n argocd ...` invocations and the Task 6 interface note to `argo`, and add a parenthetical clarifying that this is the namespace — not the `argocd.argoproj.io/*` annotation domain, which is unchanged. Also add an "environment readiness (precondition)" Global Constraint: the plan assumes a target cluster where heimdall is already deployed (live `heimdall` namespace + Loki sink + the `heimdall` ArgoCD Application). If the target predates heimdall's activation — e.g. an older k3d whose ArgoCD app-of-apps is synced from a stale Gitea mirror that omits `heimdall-app.yaml` — the agent must first work through the relevant nordri/nidavellir setup so ArgoCD reconciles heimdall, then resume Task 1. Notes that the cluster-independent code work (Tasks 2-5, 7) can proceed without a live heimdall; only Task 1 preflight and Task 6 deploy/verify are gated on environment readiness. Surfaced while resuming the tafl-game-hosting arc against local k3d, where heimdall is not yet deployed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Close the platform's missing-log-collector gap. Add a `deploy-otel-collector` step to the Crossplane Composition — a provider-helm Release of the upstream `opentelemetry-collector` chart (pinned 0.159.0, k8s distro) as a DaemonSet that tails every node's pod stdout via the filelog receiver (logsCollection preset), enriches with pod/namespace/node metadata via the k8sattributes processor (kubernetesAttributes preset), and ships to the existing in-cluster Loki over native OTLP (`auth_enabled: false`, no tenant header). Placed after deploy-loki and before auto-ready (which stays last); no nidavellir change — the selfHeal ArgoCD app reconciles it.
Two kuttl e2e tests: `otel-collector-running` (DaemonSet Ready on all nodes) and `otel-loki-logs` (a throwaway pod's stdout marker reaches Loki via LogQL — proves real pod-log shipping, not just the collector's own logs). The collector-running test uses `{.items[*]...}` jsonpath (not `[0]`) so the graceful "no DaemonSet" message is reachable under `set -e`.
Docs: architecture.md pipeline list six→seven steps with the OTel bullet + the ServiceMonitor `release: <name>-kube-prometheus` discovery fact; README "Logs" guidance corrected to the accurate OTel-DaemonSet model with a `{k8s_namespace_name=...}` LogQL example.
Verified live: Release Ready, DaemonSet 3/3, both kuttl tests green (real pod stdout queryable in Loki).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 19 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds an OpenTelemetry Collector DaemonSet step to the Crossplane composition pipeline for shipping pod logs to Loki via OTLP, updates architecture/plan/README documentation to match, and adds two KUTTL e2e tests validating collector deployment health and end-to-end log ingestion into Loki. ChangesOpenTelemetry Collector log shipping
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Pod
participant OtelCollectorDaemonSet
participant LokiService
Pod->>OtelCollectorDaemonSet: stdout logs in /var/log/pods
OtelCollectorDaemonSet->>OtelCollectorDaemonSet: enrich with k8s attributes
OtelCollectorDaemonSet->>LokiService: export via otlphttp to /otlp
sequenceDiagram
participant TestStep
participant MarkerPod
participant OtelCollectorDaemonSet
participant LokiService
participant CurlQueryPod
TestStep->>MarkerPod: create and echo unique marker
MarkerPod->>OtelCollectorDaemonSet: emit stdout logs
OtelCollectorDaemonSet->>LokiService: export via OTLP
TestStep->>CurlQueryPod: run LogQL query against LokiService
CurlQueryPod->>LokiService: query for marker
LokiService-->>CurlQueryPod: JSON result
CurlQueryPod-->>TestStep: PASS/FAIL based on result
TestStep->>TestStep: cleanup pod and namespace
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crossplane/composition.yaml`:
- Around line 354-360: The current resource settings for the collector DaemonSet
are likely too low for the filelog and k8sattributes workload, which can lead to
restarts or OOMKills under heavy log volume. Update the resources block in the
collector-related manifest to raise the memory request/limit or add a matching
self-health alert for collector restarts/OOM, using the existing filelog and
k8sattributes DaemonSet configuration as the place to make the change.
- Around line 337-344: The DaemonSet log collection config is too broad and
captures stdout from every namespace, not just heimdall. Update the
OpenTelemetry Collector setup in the composition to scope or filter pod log
ingestion using the logsCollection/filelog receiver or a filter processor, and
explicitly exclude or document sensitive namespaces/content. Keep the change
localized to the `presets.logsCollection` / `kubernetesAttributes` configuration
so cluster-wide log aggregation is intentional.
In `@tests/e2e/otel-collector-running/00-cmd.yaml`:
- Around line 13-27: The test logic in the DaemonSet lookup is assuming a single
result from the label query, but `DS` is populated from `kubectl get daemonset
... jsonpath='{.items[*].metadata.name}'`, which can return multiple names and
break later `kubectl get daemonset "$DS"` calls. Update the command sequence in
`00-cmd.yaml` to explicitly guard against multiple matches by validating the
number of returned DaemonSets before using `DS`, and emit a clear failure if
more than one is found so the later `DESIRED` and `READY` lookups always operate
on exactly one daemonset.
- Around line 5-42: The otel-collector-running command script does a single
readiness check with no retry, which can fail during a fresh rollout before the
DaemonSet settles. Update the shell logic in the command step to poll for
readiness with a bounded retry/backoff loop around the kubectl reads for the
collector DaemonSet, using the existing DS/READY/DESIRED checks and failure
messages. Keep the final pass/fail output, but only exit once the collector
stays unready after the retries are exhausted.
In `@tests/e2e/otel-loki-logs/00-cmd.yaml`:
- Around line 51-62: The query pod launch in the test script does not enforce a
client-side timeout and the polling loop can fall through without clearly
failing when the pod never reaches a terminal state. Update the kubectl run/curl
flow in the query-pod handling block to add a curl timeout and introduce an
explicit completion check after the wait loop, using the existing QUERY_POD and
the sibling test’s DONE-style pattern as a guide so a non-Succeeded/Failed pod
is treated as a test failure instead of silently continuing.
- Around line 15-19: The cleanup in the test script leaves the fixed TEST_NS
namespace deletion asynchronous, which can cause reruns to collide with a
still-terminating namespace. Update the cleanup logic in the test YAML’s cleanup
function to ensure the namespace is fully removed before the next run starts,
and make the namespace recreation path in the same script tolerate or wait for
deletion completion. Use the TEST_NS and cleanup symbols to locate the affected
teardown flow.
- Around line 22-28: The Loki service lookup in the shell setup can fail before
the empty-check runs because `kubectl get svc` with the `jsonpath` selector may
exit non-zero under `set -e`. Update the service discovery logic around the
`LOKI_SVC` assignment and subsequent `if [ -z "$LOKI_SVC" ]` check to tolerate a
missing match and still print the intended friendly failure message, following
the same hardening pattern used in `tests/e2e/loki-ingestion/00-cmd.yaml`.
- Around line 7-9: The shell step in this e2e test needs an explicit per-command
timeout because the retry loop can exceed the suite’s 300s limit. Add a timeout
to the script step in the otel-loki-logs YAML so the command is allowed to run
for the full retry budget, and keep the existing retry behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a72f3a64-58b6-41f1-85ba-5a8b907ae0f1
📒 Files selected for processing (6)
README.mdcrossplane/composition.yamldocs/architecture.mddocs/plans/2026-06-28-otel-collector-log-shipper-plan.mdtests/e2e/otel-collector-running/00-cmd.yamltests/e2e/otel-loki-logs/00-cmd.yaml
There was a problem hiding this comment.
Pull request overview
Adds an OpenTelemetry Collector Helm release to the Heimdall Crossplane Composition to run a DaemonSet that tails /var/log/pods, enriches records with Kubernetes metadata, and exports logs to in-cluster Loki via OTLP. This fills the “no cluster log shipper” gap in Heimdall’s observability stack and updates docs/tests accordingly.
Changes:
- Adds a
deploy-otel-collectorpipeline step incrossplane/composition.yamlusing the upstreamopentelemetry-collectorchart (DaemonSet + Loki OTLP export). - Introduces two kuttl e2e tests to validate the collector is Ready on all nodes and that real pod stdout reaches Loki.
- Updates documentation (README + architecture + plan) to reflect the new log shipping architecture and discovery labels.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crossplane/composition.yaml | Adds a new pipeline step deploying the OTel Collector as a log-shipping DaemonSet exporting to Loki over OTLP. |
| tests/e2e/otel-collector-running/00-cmd.yaml | New e2e test ensuring the collector DaemonSet exists and is Ready on all nodes. |
| tests/e2e/otel-loki-logs/00-cmd.yaml | New e2e test proving real workload stdout is ingested into Loki via the collector. |
| README.md | Updates user guidance for logs to match the OTel-Collector-to-Loki pipeline and label usage. |
| docs/plans/2026-06-28-otel-collector-log-shipper-plan.md | Clarifies environment readiness preconditions and corrects ArgoCD namespace references. |
| docs/architecture.md | Expands the architecture pipeline description to include the new OTel Collector step and ServiceMonitor label note. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…#10) Address the #10 review (CodeRabbit + Copilot): Composition: - Document the cluster-wide log-collection scope as an accepted trade-off — tailing every namespace's pod stdout is the point (it closes the platform log-collector gap) and the sink is the private in-cluster Loki; per-namespace excludes / a redaction processor are noted as future work. - Bump the collector memory (request 128Mi→256Mi, limit 256Mi→512Mi) for headroom on the filelog buffers + k8sattributes metadata cache under real log volume; 256Mi risked silent OOM log-shipping gaps. otel-collector-running test: - Retry the readiness check (18×10s) so a fresh rollout mid-image-pull doesn't spuriously fail. - Assert the label selects exactly one DaemonSet (word-split + count) so a multi-match can't feed a bad name into the per-name `kubectl get` calls. otel-loki-logs test: - Add an explicit `timeout: 300` to the kuttl command and shrink the over-provisioned retry loop (10 attempts) so kuttl no longer kills the async-ingestion poll mid-loop. - Use a per-run unique namespace + marker (`$(date +%s)` suffix) so a rerun can't collide with a still-Terminating namespace and stale Loki data can't false-positive. - Resolve the Loki service with `{.items[*]...}` + first token (the `[0]` index aborts under `set -e` before the friendly error) and add `curl --max-time` so a hung request can't stall the poll silently. - Single-quote the marker in the container command to avoid word-splitting. Both tests re-verified green against the live collector (otel-collector-running + otel-loki-logs pass; marker ingested and queryable in Loki on attempt 1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
deploy-otel-collectorstep to the heimdall Crossplane Composition: a provider-helm Release of the upstreamopentelemetry-collectorchart (pinned0.159.0, k8s distro) as a DaemonSet that tails every node's pod stdout (filelog receiver via the logsCollection preset), enriches with pod/namespace/node metadata (k8sattributes processor via the kubernetesAttributes preset), and ships to the existing in-cluster Loki over native OTLP (auth_enabled: false, no tenant header). Closes the platform's missing-log-collector gap.deploy-lokiand beforeauto-ready(which stays last); no nidavellir change — the selfHeal ArgoCD app reconciles it.otel-collector-running(DaemonSet Ready on all nodes) andotel-loki-logs(a throwaway pod's stdout marker reaches Loki via LogQL — proves real pod-log shipping). The collector-running test uses{.items[*]...}jsonpath so the graceful "no DaemonSet" message is reachable underset -e.release: <name>-kube-prometheusdiscovery fact; README "Logs" guidance corrected to the accurate OTel-DaemonSet model.Test plan
helm templaterender-validation: the rendered logs pipeline keepsfilelog+k8sattributesand adds theotlphttp/lokiexporter at:3100/otlp; DaemonSet + ClusterRole/Binding/ServiceAccount present.heimdall-<xr>-otel-collectorReady, DaemonSet 3/3, collector logs error-free.bash test.sh --test otel-collector-running→ PASS;bash test.sh --test otel-loki-logs→ PASS (a fresh pod's stdout marker queryable in Loki withk8s_namespace_nameenrichment).Related