fix(obsidian): seed per-vault config and enable openclaw OTLP - #1450
Conversation
- 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).
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 2 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe pull request updates Obsidian configuration generation to derive a consistent Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 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 Obsidian config seeding for per-vault JSON files and enables OpenClaw OTLP diagnostics via What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces vault configuration seeding in the Obsidian activation script and enables OpenTelemetry diagnostics for OpenClaw. Feedback suggests improving the robustness of the Python seeding script by handling potential errors, ensuring proper file closure, and allowing the Python binary path to be configurable. Additionally, it is recommended to replace the brittle string-matching tests with functional tests that verify the actual creation of configuration files.
I am having trouble creating individual review comments. Click here to see my feedback.
config/obsidian/activate.sh (17-25)
The script relies on a global python3 binary, which may not be available in the restricted environment where Nix activation scripts run. It is better to pass the path to the Python binary from Nix, similar to how SED_BIN is handled. Additionally, the Python script could be more robust by handling cases where the vaults key might be null and ensuring file handles are closed properly.
"${4:-python3}" -c '
import json, sys, os
try:
with open(sys.argv[1]) as f:
cfg = json.load(f)
obs_dir = sys.argv[2]
for vid in (cfg.get("vaults") or {}):
p = os.path.join(obs_dir, vid + ".json")
if not os.path.exists(p):
with open(p, "w") as f: f.write("{}")
except Exception as e:
print(f"Warning: Failed to seed vault configs: {e}", file=sys.stderr)
' "$OBS_DIR/obsidian.json" "$OBS_DIR"
References
- Maintain consistency with established patterns for writing scripts that are extracted from Nix expressions.
spec/activate_config_spec.sh (150-153)
This test only verifies that the script contains certain strings (like vid.*json), which does not guarantee that the logic for seeding vault configurations actually works. It is a change detector test that is brittle and does not verify behavior. Consider adding a functional test that checks for the actual existence of the seeded files in a temporary directory.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
spec/activate_config_spec.sh (1)
150-153: Prefer behavior-based spec over pattern grep for seeding logic.Line [151] asserts source text patterns, so this can pass even if runtime seeding is broken. Consider running the script in a temp HOME and asserting actual file creation/preservation.
Example direction
-It 'seeds missing per-vault config files' -When run bash -c "grep 'wiki.json\|vault.*json\|vid.*json' '$SCRIPT'" -The output should not equal '' +It 'seeds missing per-vault config files' +When run bash -c ' + set -euo pipefail + tmp="$(mktemp -d)" + cfg="$tmp/obsidian.json" + cat > "$cfg" <<EOF +{"vaults":{"wiki":{"path":"__HOME_DIR__/wiki","ts":0,"open":true}}} +EOF + bash "$SCRIPT" "$cfg" "$tmp" sed + test -f "$tmp/.config/obsidian/wiki.json" +' +The status should equal 0 End🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/activate_config_spec.sh` around lines 150 - 153, Replace the fragile grep-based assertion in the "seeds missing per-vault config files" example with a behavior-driven test that runs the target script ($SCRIPT) in an isolated temporary HOME and asserts that the expected files are actually created/preserved on disk; specifically, create a temp dir (mktemp -d), set HOME to it, invoke bash -c "$SCRIPT", then check for existence of the concrete files (e.g., wiki.json, vault*.json, vid*.json) with test -f or [ -e ] and fail the spec if any expected file is missing, ensuring you reference the same test name ("seeds missing per-vault config files") and the $SCRIPT variable when updating the spec.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/openclaw/openclaw.template.json`:
- Around line 902-911: The template enables telemetry logs by default
("diagnostics" -> "otel" -> "logs": true); change that value to false so runtime
logs are opt-in rather than exported by default, i.e., update the otel.logs
setting under the diagnostics object (the "diagnostics" / "otel" block) to
"logs": false.
In `@config/openclaw/openclaw.tpl.json`:
- Around line 902-911: The template enables OTLP logs by default which is too
permissive; change the default under the "diagnostics" -> "otel" block so "logs"
is false (or remove it to require explicit opt-in) and update any related
documentation or config comments to instruct consumers to opt in if they need
log export; look for the "diagnostics", "otel", "serviceName" and "logs" keys in
the openclaw.tpl.json and make "logs": false (or drop the key) to make log
export opt-in.
---
Nitpick comments:
In `@spec/activate_config_spec.sh`:
- Around line 150-153: Replace the fragile grep-based assertion in the "seeds
missing per-vault config files" example with a behavior-driven test that runs
the target script ($SCRIPT) in an isolated temporary HOME and asserts that the
expected files are actually created/preserved on disk; specifically, create a
temp dir (mktemp -d), set HOME to it, invoke bash -c "$SCRIPT", then check for
existence of the concrete files (e.g., wiki.json, vault*.json, vid*.json) with
test -f or [ -e ] and fail the spec if any expected file is missing, ensuring
you reference the same test name ("seeds missing per-vault config files") and
the $SCRIPT variable when updating the spec.
🪄 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: CHILL
Plan: Pro
Run ID: ce198287-adf1-4a06-9565-defcd193f288
📒 Files selected for processing (4)
config/obsidian/activate.shconfig/openclaw/openclaw.template.jsonconfig/openclaw/openclaw.tpl.jsonspec/activate_config_spec.sh
| "diagnostics": { | ||
| "enabled": true, | ||
| "otel": { | ||
| "enabled": true, | ||
| "protocol": "http/protobuf", | ||
| "serviceName": "openclaw", | ||
| "traces": true, | ||
| "metrics": true, | ||
| "logs": true | ||
| } |
There was a problem hiding this comment.
Keep telemetry logs opt-in in the concrete template as well.
Line 910 sets "logs": true here too, so sensitive runtime logs may be exported by default. Align this with a safer default (false) unless explicitly required per environment.
Proposed change
"diagnostics": {
"enabled": true,
"otel": {
"enabled": true,
"protocol": "http/protobuf",
"serviceName": "openclaw",
"traces": true,
"metrics": true,
- "logs": true
+ "logs": false
}
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/openclaw/openclaw.template.json` around lines 902 - 911, The template
enables telemetry logs by default ("diagnostics" -> "otel" -> "logs": true);
change that value to false so runtime logs are opt-in rather than exported by
default, i.e., update the otel.logs setting under the diagnostics object (the
"diagnostics" / "otel" block) to "logs": false.
| "diagnostics": { | ||
| "enabled": true, | ||
| "otel": { | ||
| "enabled": true, | ||
| "protocol": "http/protobuf", | ||
| "serviceName": "openclaw", | ||
| "traces": true, | ||
| "metrics": true, | ||
| "logs": true | ||
| } |
There was a problem hiding this comment.
Default OTLP log export is too permissive for a base template.
Line 910 sets "logs": true globally. In a shared template, that can forward sensitive runtime content by default. Safer default is opt-in logs.
Proposed change
"diagnostics": {
"enabled": true,
"otel": {
"enabled": true,
"protocol": "http/protobuf",
"serviceName": "openclaw",
"traces": true,
"metrics": true,
- "logs": true
+ "logs": false
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "diagnostics": { | |
| "enabled": true, | |
| "otel": { | |
| "enabled": true, | |
| "protocol": "http/protobuf", | |
| "serviceName": "openclaw", | |
| "traces": true, | |
| "metrics": true, | |
| "logs": true | |
| } | |
| "diagnostics": { | |
| "enabled": true, | |
| "otel": { | |
| "enabled": true, | |
| "protocol": "http/protobuf", | |
| "serviceName": "openclaw", | |
| "traces": true, | |
| "metrics": true, | |
| "logs": false | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/openclaw/openclaw.tpl.json` around lines 902 - 911, The template
enables OTLP logs by default which is too permissive; change the default under
the "diagnostics" -> "otel" block so "logs" is false (or remove it to require
explicit opt-in) and update any related documentation or config comments to
instruct consumers to opt in if they need log export; look for the
"diagnostics", "otel", "serviceName" and "logs" keys in the openclaw.tpl.json
and make "logs": false (or drop the key) to make log export opt-in.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="config/obsidian/activate.sh">
<violation number="1">
P1: Activation no longer seeds missing per-vault `<vault-id>.json` files, so new/missing vault configs will trigger ENOENT errors again.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
1 participant
Summary
activate.shnow seeds missing<vault-id>.jsonfiles (e.g.wiki.json) in~/.config/obsidian/on activation. This eliminates theENOENT: no such file or directory, open '/home/ubuntu/.config/obsidian/wiki.json'error logged on every daemon restart.diagnostics-otelplugin and adds top-leveldiagnosticsconfig block to both template files with traces, metrics, and logs overhttp/protobuf.chmod 644onobsidian.jsonso Obsidian can write back to it (update timestamps, vault state) withoutEACCESerrors during activation races.Test plan
shellspec spec/activate_config_spec.shpasses (25 examples, 0 failures)shellspec spec/openclaw_hydrate_spec.shpasses (20 examples, 0 failures)wiki.jsonfor new vaults and preserves existing oneswiki.jsonon kyber - next obsidian restart should be cleanSummary by cubic
Turns on OpenClaw OTLP telemetry (http/protobuf) and auto-wires the endpoint to the in-cluster Alloy collector during hydration. Adds a top-level diagnostics block and enables the
diagnostics-otelplugin in both templates.openclaw.template.jsonandopenclaw.tpl.json(serviceName "openclaw"; traces, metrics, logs enabled; endpoint placeholder__OTEL_ENDPOINT__).__OTEL_ENDPOINT__tohttp://<Alloy ClusterIP>:4318viakubectl, falling back tohttp://localhost:4318.Written for commit 3e5a7f6. Summary will update on new commits.