Skip to content

feat(server): forward OTLP export config to the worker subprocess (O1 Lever B)#1

Merged
thewoolleyman merged 2 commits into
factory-integrationfrom
worker-otel-reinject
Jul 17, 2026
Merged

feat(server): forward OTLP export config to the worker subprocess (O1 Lever B)#1
thewoolleyman merged 2 commits into
factory-integrationfrom
worker-otel-reinject

Conversation

@thewoolleyman

Copy link
Copy Markdown
Owner

Activates the inert worker OTLP exporter for the dark factory (livespec O1 bd-ib-98c.4, Lever B). Companion to the transport PR fabro-sh#576 (already carried in factory-integration).

Problem

The server spawns __run-worker via apply_worker_env, which env_clear()s and copies a narrow allowlist (WORKER_ENV_ALLOWLIST) that omits all OTEL_*. The worker installs otel::otel_layer() (fabro-cli) but, with no OTEL env, never exports — so the agent-side spans are lost.

Change

Re-inject the server's OTLP export config into the worker after apply_worker_env, via a new apply_worker_otel_export_env:

  • forwards the 5 non-secret export vars (endpoint / protocol / service-name, base + per-signal);
  • explicitly env_removes the credential-bearing OTEL_EXPORTER_OTLP_HEADERS / _TRACES_HEADERS.

Security invariant

The headers vars carry the collector's egress credential (a Honeycomb API key), and the opentelemetry-otlp exporter reads OTEL_EXPORTER_OTLP_HEADERS from the env itself. They must never reach the sandboxed worker — the worker exports to the local collector, which adds egress auth. A unit test proves the five non-secret vars forward and both headers spellings are stripped even when pre-set.

Additive / opt-in

With no OTLP env on the server, nothing is forwarded and there is no behavior change.

Validation (pinned nightly-2026-04-14)

  • cargo fmt --check --all — clean
  • cargo clippy --locked -p fabro-server --all-targets -- -D warnings — clean
  • cargo test -p fabro-server --lib spawn_env — 3 passed

Draft pending Codex + Fable adversarial review and the maintainer's integrate + re-pin decision (Lever B pairs with Lever A, the server-start OTEL env; see the livespec O1 plan). End-to-end proof-dispatch is deferred until the pinned host binary is rebuilt from this + the server restarted with OTEL env.

thewoolleyman added 2 commits July 16, 2026 17:52
The server spawns `__run-worker` via apply_worker_env, which env_clear()s
and copies a narrow allowlist that omits OTEL_*. The worker installs the
otel_layer (fabro-cli) but never sees the exporter env, so its spans are
never exported.

Re-inject the server's OTLP export config into the worker after
apply_worker_env: forward the NON-SECRET export vars (endpoint / protocol
/ service-name, base + per-signal), and explicitly strip the credential-
bearing OTEL_EXPORTER_OTLP_HEADERS / _TRACES_HEADERS so the collector's
egress key can never reach the sandboxed worker. The worker exports to the
LOCAL collector, which adds egress auth itself.

Additive and opt-in: with no OTLP env on the server nothing is forwarded
and there is no behavior change. A unit test asserts the five non-secret
vars forward and both headers spellings are stripped even when pre-set.
Address the Codex + Fable adversarial review of the worker OTLP re-injection
(all findings low/minor/nit; the security invariant was confirmed to hold):

- Forward OTEL_EXPORTER_OTLP_TIMEOUT / _TRACES_TIMEOUT too (Codex): the
  opentelemetry-otlp exporter reads them from env, so the worker's exporter
  now mirrors the server's full non-secret export config, not just
  endpoint/protocol/service. Listed by literal name (no fabro_static constant).
- Pre-set BOTH headers vars in the unit test before apply_otel_export (Codex +
  Fable): previously only OTEL_EXPORTER_OTLP_HEADERS was pre-set, so the
  _TRACES_HEADERS denylist entry was not regression-guarded. Both strips are now
  load-bearing. Timeout forwarding is asserted too.
- Correct the allowlist comment (Fable): the fail-closed guarantee is env_clear
  + allowlist-only forwarding; the headers denylist is belt-and-suspenders for an
  accidental explicit set after the call, not the primary control. Note the
  local-no-auth-collector topology assumption.
- Make the ordering requirement explicit at the worker_runtime.rs call site
  (Fable): the re-injection MUST stay after apply_worker_env's env_clear.

fmt/clippy(-D warnings)/spawn_env tests green under nightly-2026-04-14.
@thewoolleyman
thewoolleyman marked this pull request as ready for review July 16, 2026 16:06
@thewoolleyman

Copy link
Copy Markdown
Owner Author

Adversarial review complete (Codex + Fable), findings addressed

Reviewed with the transport's two-reviewer pattern. Fable (executing) ran clippy + tests and confirmed the security invariant holds: env_clear + allowlist-only forwarding is fail-closed; no path lets OTEL_EXPORTER_OTLP_HEADERS/_TRACES_HEADERS or any other secret reach the worker; endpoint/protocol precedence matches fabro-cli/src/otel.rs. Codex (static) found no leak path and confirmed correct ordering. All findings were low/minor/nit:

  • Forward the timeout knobs (Codex) — OTEL_EXPORTER_OTLP_TIMEOUT/_TRACES_TIMEOUT are read by the exporter too; now forwarded so the worker exporter fully mirrors the server's non-secret export config.
  • Harden the strip test (Codex + Fable) — the test now pre-sets both headers vars before apply_otel_export, so both denylist entries are regression-guarded (previously only the base spelling was exercised).
  • Correct the comments (Fable) — the fail-closed guarantee is env_clear + allowlist-only forwarding; the headers denylist is belt-and-suspenders. Added the local-no-auth-collector topology assumption.
  • Ordering made explicit (Fable) — the re-injection MUST stay after apply_worker_env's env_clear; documented at the call site.

Not added: a process-env integration test over command_for_spec. Fable suggested mirroring the FABRO_WORKER_TOKEN assertion, but that var is spec-field-sourced (deterministic), whereas the OTEL forwarding reads the ambient process env — like apply_worker_env's allowlist, which fabro itself tests via the injected-lookup unit test (mirrored here), not an integration test. A command_for_spec OTEL test would need unsafe global set_var; the ordering it would guard is fail-safe (telemetry loss, never a leak) and is now pinned by the explicit call-site comment.

Validated under the pinned nightly-2026-04-14: fmt clean, clippy -p fabro-server --all-targets -D warnings clean, test spawn_env 3/3.

Ready to integrate into factory-integration when the host binary is rebuilt + re-pinned (pairs with Lever A, the server-start OTEL env). End-to-end proof-dispatch follows that repin + restart.

livespec-pr-bot Bot pushed a commit to thewoolleyman/livespec-orchestrator-beads-fabro that referenced this pull request Jul 16, 2026
…ook landed

Update the handoff NEXT ACTION: Lever B is no longer a TODO — it is built,
adversarially reviewed (Codex + Fable), and ready as fork PR thewoolleyman/fabro#1,
green under the pinned toolchain. Lever A's runbook step landed (#702). What
remains is a single operator window: merge #1, rebuild + re-pin the host binary,
restart with the Lever A OTEL env, then proof-dispatch. O2 follows O1's proof.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thewoolleyman
thewoolleyman merged commit c543446 into factory-integration Jul 17, 2026
livespec-pr-bot Bot pushed a commit to thewoolleyman/livespec-orchestrator-beads-fabro that referenced this pull request Jul 17, 2026
…IGPIPE

O1 is proven end-to-end (2026-07-17): the full operator cutover ran — merged
fork PR thewoolleyman/fabro#1, re-pinned the host binary (c543446, Lever B),
restarted the server with the Lever A OTEL env, and proof-dispatched bd-ib-dqt
(green, PR #706 merged). Honeycomb's fabro dataset shows the run span at
count=2/root_count=2 — the server + worker run-span pair (two distinct traces),
service.name=fabro, via the receiver. Handoff NEXT ACTION updated accordingly;
O2 (traceparent) is next.

Also fix a pre-existing SIGPIPE in build-and-verify.sh: the version probe used
the multi-line 'fabro version' subcommand piped to 'head -1', which closed the
pipe early and panicked the Rust binary (fatal under set -o pipefail). Use the
single-line 'fabro --version' instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant