fix(openclaw): use nix-provided kubectl for OTLP endpoint resolution - #1452
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change extends the OpenClaw Nix hydration script configuration to substitute a Nix-managed kubectl binary into template variables during script generation. The substituted binary is conditionally resolved based on host configuration, and is used by the hydration script when retrieving OTEL endpoint information in gateway mode. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRFixes OpenTelemetry (OTLP) endpoint resolution for OpenClaw by dynamically determining the Alloy service's ClusterIP using a Nix-provided What changed?
Description generated by Mesa. Update settings |
- Obsidian headless daemon logs ENOENT on every start because
wiki.json (per-vault config) was never created by activate.sh.
Now seeds missing <vault-id>.json files with {} on activation.
- Enable diagnostics-otel plugin and top-level diagnostics config
in both openclaw templates (traces, metrics, logs via http/protobuf).
0442e4c to
90bc1aa
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces OpenTelemetry (OTEL) diagnostics to the OpenClaw configuration. Key changes include adding kubectl to the Nix environment, updating the hydration script to dynamically resolve the OTEL endpoint via Kubernetes service discovery, and enabling the diagnostics section and plugin in the configuration templates. A review comment identifies a potential issue in the hydration script where the endpoint URL could become invalid if the kubectl command returns an empty string, and provides a suggestion to handle this using shell parameter expansion.
|
|
||
| # Resolve Alloy OTLP ClusterIP for local telemetry export | ||
| OTEL_ENDPOINT="http://$(kubectl get svc alloy -n alloy -o jsonpath='{.spec.clusterIP}' 2>/dev/null || echo localhost):4318" | ||
| OTEL_ENDPOINT="http://$(@kubectl@ get svc alloy -n alloy -o jsonpath='{.spec.clusterIP}' 2>/dev/null || echo localhost):4318" |
There was a problem hiding this comment.
The fallback logic for OTEL_ENDPOINT does not work as intended when @kubectl@ is substituted with /usr/bin/true (which occurs on non-Kyber hosts). Since true exits with code 0 but produces no output, the command substitution $(...) returns an empty string, resulting in an invalid endpoint URL: http://:4318.
Using shell parameter expansion ${VAR:-default} ensures that the fallback to localhost works correctly even when the command succeeds with empty output.
| OTEL_ENDPOINT="http://$(@kubectl@ get svc alloy -n alloy -o jsonpath='{.spec.clusterIP}' 2>/dev/null || echo localhost):4318" | |
| OTEL_ENDPOINT=$(@kubectl@ get svc alloy -n alloy -o jsonpath='{.spec.clusterIP}' 2>/dev/null || echo localhost) | |
| OTEL_ENDPOINT="http://${OTEL_ENDPOINT:-localhost}:4318" |
References
- Maintain consistency with established patterns for writing scripts that are extracted from Nix expressions, ensuring placeholders like @kubectl@ are handled correctly even when substituted with no-op commands.
Summary
localhost:4318becausekubectlwasn't in PATH during home-manager activation@kubectl@placeholder tohydrate.shresolved viapkgs.kubectlindefault.nix/usr/bin/true(falls back to localhost gracefully)Test plan
shellspec spec/openclaw_hydrate_spec.shpasseskubectl get svc alloy -n alloyresolves to10.43.138.193on kyberSummary by cubic
Fixes OTLP endpoint resolution by using Nix-provided
kubectlduring hydration so telemetry points to the Alloy service instead oflocalhost:4318. Enablesdiagnostics-otelwith a safe fallback on non-Kyber hosts; Obsidian activation changes were reverted and are not included.New Features
diagnosticsanddiagnostics-otelin both OpenClaw templates, using__OTEL_ENDPOINT__withhttp/protobufand service nameopenclaw.Bug Fixes
@kubectl@mapped to${pkgs.kubectl}/bin/kubectlto read the Alloy service ClusterIP.@kubectl@to/usr/bin/trueto fall back tolocalhost:4318.Written for commit 90bc1aa. Summary will update on new commits.