feat(cli): opt-in OTLP/HTTP export for tracing spans#576
Open
thewoolleyman wants to merge 1 commit into
Open
Conversation
Adds an additive, opt-in OTLP/HTTP exporter for fabro's existing tracing spans. It activates only when an OTLP endpoint env var is set (OTEL_EXPORTER_OTLP_ENDPOINT / OTEL_EXPORTER_OTLP_TRACES_ENDPOINT); when unset, the tracing stack is byte-for-byte the current fmt-only setup and nothing is exported. No new instrumentation -- pure transport, so the spans fabro already logs also reach an OTLP collector when one is configured. Honors the standard OTLP env vars (endpoint, protocol, headers, service.name defaulting to "fabro"). Default protocol is http/json; http/protobuf via OTEL_EXPORTER_OTLP_PROTOCOL. Reuses the reqwest already present in the lockfile -- no new reqwest version is introduced. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thewoolleyman
marked this pull request as ready for review
July 16, 2026 08:41
Member
|
@thewoolleyman thanks for this! I think OTLP exports make sense for us to support. My primary question is about blocking vs. async. I noticed the comment saying this would be blocking, and I am wondering if it should be async given Fabro runs on the async Tokio runtime. Thoughts? |
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
Adds an opt-in OTLP/HTTP exporter for fabro's existing
tracingspans. It isadditive and inert unless an OTLP endpoint env var is set — when unset,
otel_layer()returns a no-opNonelayer, so the tracing stack is behaviorallyunchanged (the
fmtlayers and their output are untouched) and nothing isexported. This PR is pure transport: it adds no new instrumentation points
to fabro's code. When enabled, fabro's existing
tracingspans — the same span treethat structures its log output — are bridged to OTLP via the standard
tracing-opentelemetrylayer (which maps them to OTel using conventional semantics,e.g. error events → span status). The optional synthetic extras (busy/idle timing,
thread, source location) are disabled, so exported spans stay close to fabro's actual
spans.
What it does
lib/crates/fabro-cli/src/otel.rs: builds anopentelemetry-otlpspanexporter +
tracing-opentelemetrylayer, guarded byOTEL_EXPORTER_OTLP_ENDPOINT/
OTEL_EXPORTER_OTLP_TRACES_ENDPOINT. Returns a no-op (None) layer when unset.logging.rs: adds.with(otel::otel_layer())alongside the existingfmtlayer in all four subscriber-init paths (cli / server-stdout / worker /
worker-stdout).
main.rs: registersmod oteland callsotel::shutdown()late inmainfor abest-effort final drain on the normal-exit path (no-op when disabled).
fabro-static/src/env_vars.rs: registers the fiveOTEL_*names asEnvVarsconsts (fabro's env-var naming convention), referenced from
otel.rs.opentelemetry/opentelemetry_sdk0.30,opentelemetry-otlp0.30(
default-features = false;http-proto+http-json+reqwest-blocking-client— drops the unused OTLP logs exporter),
tracing-opentelemetry0.31. The OTLP HTTPfeatures enable
metricsunconditionally, so the trace+metrics SDK is compiled (atrace-only reduction isn't achievable via the HTTP exporter). These reuse the
reqwest 0.12already in the lockfile — no new reqwest version. The lockfilegains
tonic 0.13+prost 0.13transitively (viaopentelemetry-proto's protobufmessage types for
http/protobuf) — message codegen only; no gRPC transport iscompiled or used.
Env vars honored
This module resolves the endpoint itself (from
OTEL_EXPORTER_OTLP_TRACES_ENDPOINT,else
OTEL_EXPORTER_OTLP_ENDPOINT+/v1/traces) and pins the protocol(per-signal
OTEL_EXPORTER_OTLP_TRACES_PROTOCOLwins, elseOTEL_EXPORTER_OTLP_PROTOCOL; defaulthttp/protobuf, the OTLP spec default; sethttp/jsonto switch) and the service name (OTEL_SERVICE_NAME, defaultfabro).The
opentelemetry-otlpexporter reads the remaining OTLP vars itself — headers(
OTEL_EXPORTER_OTLP_HEADERS/..._TRACES_HEADERS) and timeouts(
OTEL_EXPORTER_OTLP_TIMEOUT). Export is gated on a non-empty resolved endpoint.Design / blast radius
otel_layer()returnsNone, which is a no-optracinglayer; thefmtlayers and their output are untouched.otelis a privatefabro-climodule(
pub(crate)); the only call sites are the four subscriber inits and themaindrain. (
fabro-staticgains five additiveEnvVarsconsts — new public surface,not a change to existing APIs.)
fabro_static::EnvVarsconsts (theOTEL_*set was added there);the module-level
#![expect(clippy::disallowed_methods, reason = ...)]covers onlythe
std::env::varread mechanism, per fabro's env-var convention.FABRO_LOGEnvFilteras fabro's log output:the OTLP layer sits alongside the
fmtlayer under that filter, so a span below theconfigured level is neither logged nor exported.
otel::shutdown()is placed right besidefabro_telemetry::shutdown()on thenormal
mainexit path. fabro's own telemetry shutdown is likewise only on thatpath — its
std::process::exit(...)bailout sites (preflight, run/*, doctor,server/status, …) do not drain
fabro_telemetryeither. Rerouting those sites todrain OTLP would make it more aggressive than fabro's own telemetry and touch
unrelated exit paths; the SDK batch processor exports periodically regardless, so
such an exit at most drops the last un-flushed batch. (A consistent drain of both
telemetries on every exit path would be a separate, broader change to fabro's exit
convention — out of scope for this transport PR.)
catch_unwind: the SDK batchprocessor
expects a successful background-thread spawn, so a (near-never)spawn failure disables export instead of taking fabro down. The panic still
passes through fabro's installed panic hook before it is caught, so such a
failure is reported once and then export is disabled rather than the process
aborting (
panic = unwind, so the catch is effective).OTLP env vars (per-signal
OTEL_EXPORTER_OTLP_TRACES_ENDPOINTused as-is, elseOTEL_EXPORTER_OTLP_ENDPOINT+/v1/traces; empty/whitespace treated as unset)and passed programmatically via
.with_endpoint(...). A malformed endpointfails the exporter build and disables export — there is no path where export is on
while the endpoint silently falls back to localhost. (Delegating to the exporter's
own env read would silently drop a malformed value and fall back to localhost.)
apply_worker_env, an env allowlist (env-clear +narrow copy) that does not include
OTEL_*; a server-launched worker thereforewill not export until those vars are forwarded — a separate decision, since
OTEL_EXPORTER_OTLP_HEADERScan carry secrets. Directly-invoked processes(cli / server /
__run-worker) export normally.opentelemetry-otlp's own internally-builtreqwest::blocking::Client, separate from fabro'sfabro_httpegress facade (soit does not consult
FABRO_HTTP_PROXY_POLICY). This is deliberate for an opt-intelemetry exporter that owns its
OTEL_EXPORTER_OTLP_TIMEOUT-configured client;routing OTLP through
fabro_http(via.with_http_client(...)) is a possiblefollow-up if proxy-policy parity is wanted.
Design decisions
http/protobuf— the OTLP spec default.http/jsonisavailable via
OTEL_EXPORTER_OTLP_PROTOCOL=http/json(both HTTP encodings arecompiled in).
BatchSpanProcessordrives the exporter on a dedicated thread viablock_on, sothe exporter must not require an ambient async runtime. This PR uses
reqwest-blocking-client(opentelemetry-otlp's own default) — no dependency on arunning tokio runtime at export time.
Open questions for review
could move behind an off-by-default
otelfeature if the added compile deps areunwelcome by default. Happy to gate it either way.
Testing
cargo +nightly-2026-04-14 fmt --check --all— clean.cargo +nightly-2026-04-14 clippy --locked --workspace --all-targets -- -D warnings— clean (the CI bar).
cargo +nightly-2026-04-14 nextest run -p fabro-cli -E 'test(otel::)'(orcargo test -p fabro-cli --bin fabro otel::) — green (both theparse_protocolandresolve_endpointunit tests).spans export over OTLP/HTTP.