fix(bench): bind-mount a writable capture dir for the demo collector (PR-N3.1) - #74
Conversation
…(PR-N3.1)
First capture dispatch (run 26714187216) failed: the
fileexporter couldn't open /tmp/otel-corpus.jsonl
("no such file or directory") and crash-looped the collector.
The demo's collector is a distroless image with no writable
/tmp, and the fileexporter does not create a missing parent
directory.
Fix: bind-mount a runner-side directory into the collector at
/capture via a compose override layered with COMPOSE_FILE, and
point the exporter at /capture/logs.jsonl. The directory is
mkdir'd on the host before `up` so the mount target exists, and
the corpus now lands straight on the runner filesystem — the
docker cp extract step is replaced by a non-empty verification +
teardown.
The rest of the demo came up fine on the runner (all ~20 images
pulled, collector started, OTLP receiver bound) — this was the
only blocker.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR changes the OTel demo corpus capture to mount a host directory into the otel-collector, update the collector exporter to write to /capture/logs.jsonl, and modify the workflow to create the host directory, verify the captured file, and tear down the stack without using docker compose cp. ChangesBind-mounted corpus capture flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes the workflow-dispatched OTel Demo corpus capture by writing the collector fileexporter output to a bind-mounted host directory instead of an in-container /tmp path, aligning the capture path with the demo collector’s distroless filesystem constraints.
Changes:
- Adds a Compose override that mounts
${OURIOS_CAPTURE_DIR}into the demo collector at/capture. - Updates the collector fileexporter path to
/capture/logs.jsonl. - Updates the workflow to create the host capture directory, verify the captured file directly, and tear down the demo stack.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.github/workflows/capture-otel-demo-corpus.yml |
Adds shared capture/Compose env, creates the bind-mounted directory, and verifies the host-side corpus file. |
.github/otel-demo-capture-extras.yml |
Changes the fileexporter destination from /tmp to /capture/logs.jsonl and documents why. |
.github/otel-demo-capture-compose-override.yml |
Adds the Compose service override that bind-mounts the runner capture directory into the collector. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/capture-otel-demo-corpus.yml (1)
167-167: ⚡ Quick winReuse
$OURIOS_CAPTURE_DIRinstead of re-deriving the path.This step hardcodes
$GITHUB_WORKSPACE/captured, duplicating theOURIOS_CAPTURE_DIRdefinition. If the capture dir is ever changed in theenv:block, this step silently diverges.- cd "$GITHUB_WORKSPACE/captured" + cd "$OURIOS_CAPTURE_DIR"🤖 Prompt for 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. In @.github/workflows/capture-otel-demo-corpus.yml at line 167, The workflow step currently changes directory with a hardcoded path cd "$GITHUB_WORKSPACE/captured" which duplicates the OURIOS_CAPTURE_DIR definition; update that step to use the environment variable instead (cd "$OURIOS_CAPTURE_DIR") so the step follows the single source of truth for the capture directory (look for the step containing the cd command and the env var OURIOS_CAPTURE_DIR).
🤖 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 @.github/otel-demo-capture-compose-override.yml:
- Around line 19-22: The bind-mounted capture directory used by the
otel-collector service (volume entry `${OURIOS_CAPTURE_DIR}:/capture` in the
otel-collector service) must be writable by UID 10001 because the
opentelemetry-collector-contrib image runs as USER 10001:10001; ensure the host
directory referenced by `${OURIOS_CAPTURE_DIR}` is owned by or writable to UID
10001 (e.g., chown -R 10001:10001 ${OURIOS_CAPTURE_DIR} or set permissive mode
like chmod 0777) so the collector's fileexporter can create logs.jsonl without
permission denied errors.
---
Nitpick comments:
In @.github/workflows/capture-otel-demo-corpus.yml:
- Line 167: The workflow step currently changes directory with a hardcoded path
cd "$GITHUB_WORKSPACE/captured" which duplicates the OURIOS_CAPTURE_DIR
definition; update that step to use the environment variable instead (cd
"$OURIOS_CAPTURE_DIR") so the step follows the single source of truth for the
capture directory (look for the step containing the cd command and the env var
OURIOS_CAPTURE_DIR).
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0066e438-e256-4140-8f88-77881881b217
📒 Files selected for processing (3)
.github/otel-demo-capture-compose-override.yml.github/otel-demo-capture-extras.yml.github/workflows/capture-otel-demo-corpus.yml
What
Fixes the first OTel Demo capture dispatch (run 26714187216), which failed at "Bring up the demo".
Root cause
The fileexporter couldn't open its output path:
The demo's collector is a distroless image (v0.142.0) with no writable
/tmp, and the fileexporter does not create a missing parent directory — so it failed at startup and the collector crash-looped (restart: unless-stopped), never reaching a running state. My readiness guard correctly failed the step.The rest of the demo came up fine on the runner — all ~20 images pulled, the collector process started, the OTLP receiver bound
4317/4318. The output path was the only blocker.Fix
Bind-mount a runner-side directory into the collector at
/captureand point the exporter at/capture/logs.jsonl:.github/otel-demo-capture-compose-override.ymladds${OURIOS_CAPTURE_DIR}:/captureto theotel-collectorservice.COMPOSE_FILE(colon-separated; base relative to thedemoworkdir, override absolute so it's project-dir-independent).mkdirs$OURIOS_CAPTURE_DIR(=${{ github.workspace }}/captured) beforeup, so the mount target exists and the collector (root) can create the file in it.docker cpextract step is replaced by a non-empty verification + teardown (and dumps collector logs if the capture is empty).otel-demo-capture-extras.ymlupdated/tmp/...→/capture/logs.jsonl, with a comment explaining the distroless-/tmptrap.Verification
yaml.safe_load.workflow_dispatch-only; the real test is the next dispatch after merge (which I'll run + report the corpus shape from).🤖 Generated with Claude Code
Summary by CodeRabbit