Commit Graph

6 Commits

  • codex-otel: add configurable trace metadata (#21556)
    Add Codex config for static trace span attributes and structured W3C
    tracestate field upserts. The config flows through OtelSettings so
    callers can attach trace metadata without touching every span call site.
    
    Apply span attributes with an SDK span processor so every exported
    trace span carries the configured metadata. Model tracestate as nested
    member fields so configured keys can be upserted while unrelated
    propagated state in the same member is preserved.
    
    Validate configured tracestate before installing provider-global state,
    including header-unsafe values the SDK does not reject by itself. This
    keeps Codex from propagating malformed trace context from config.
    
    Update the config schema, public docs, and OTLP loopback coverage for
    config parsing, span export, propagation, and invalid-header rejection.
  • chore: clean up argument-comment lint and roll out all-target CI on macOS (#16054)
    ## Why
    
    `argument-comment-lint` was green in CI even though the repo still had
    many uncommented literal arguments. The main gap was target coverage:
    the repo wrapper did not force Cargo to inspect test-only call sites, so
    examples like the `latest_session_lookup_params(true, ...)` tests in
    `codex-rs/tui_app_server/src/lib.rs` never entered the blocking CI path.
    
    This change cleans up the existing backlog, makes the default repo lint
    path cover all Cargo targets, and starts rolling that stricter CI
    enforcement out on the platform where it is currently validated.
    
    ## What changed
    
    - mechanically fixed existing `argument-comment-lint` violations across
    the `codex-rs` workspace, including tests, examples, and benches
    - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` and
    `tools/argument-comment-lint/run.sh` so non-`--fix` runs default to
    `--all-targets` unless the caller explicitly narrows the target set
    - fixed both wrappers so forwarded cargo arguments after `--` are
    preserved with a single separator
    - documented the new default behavior in
    `tools/argument-comment-lint/README.md`
    - updated `rust-ci` so the macOS lint lane keeps the plain wrapper
    invocation and therefore enforces `--all-targets`, while Linux and
    Windows temporarily pass `-- --lib --bins`
    
    That temporary CI split keeps the stricter all-targets check where it is
    already cleaned up, while leaving room to finish the remaining Linux-
    and Windows-specific target-gated cleanup before enabling
    `--all-targets` on those runners. The Linux and Windows failures on the
    intermediate revision were caused by the wrapper forwarding bug, not by
    additional lint findings in those lanes.
    
    ## Validation
    
    - `bash -n tools/argument-comment-lint/run.sh`
    - `bash -n tools/argument-comment-lint/run-prebuilt-linter.sh`
    - shell-level wrapper forwarding check for `-- --lib --bins`
    - shell-level wrapper forwarding check for `-- --tests`
    - `just argument-comment-lint`
    - `cargo test` in `tools/argument-comment-lint`
    - `cargo test -p codex-terminal-detection`
    
    ## Follow-up
    
    - Clean up remaining Linux-only target-gated callsites, then switch the
    Linux lint lane back to the plain wrapper invocation.
    - Clean up remaining Windows-only target-gated callsites, then switch
    the Windows lint lane back to the plain wrapper invocation.
  • fix(otel): make HTTP trace export survive app-server runtimes (#14300)
    ## Summary
    
    This PR fixes OTLP HTTP trace export in runtimes where the previous
    exporter setup was unreliable, especially around app-server usage. It
    also removes the old `codex_otel::otel_provider` compatibility shim and
    switches remaining call sites over to the crate-root
    `codex_otel::OtelProvider` export.
    
    ## What changed
    
    - Use a runtime-safe OTLP HTTP trace exporter path for Tokio runtimes.
    - Add an async HTTP client path for trace export when we are already
    inside a multi-thread Tokio runtime.
    - Make provider shutdown flush traces before tearing down the tracer
    provider.
    - Add loopback coverage that verifies traces are actually sent to
    `/v1/traces`:
      - outside Tokio
      - inside a multi-thread Tokio runtime
      - inside a current-thread Tokio runtime
    - Remove the `codex_otel::otel_provider` shim and update remaining
    imports.
    
    ## Why
    
    I hit cases where spans were being created correctly but never made it
    to the collector. The issue turned out to be in exporter/runtime
    behavior rather than the span plumbing itself. This PR narrows that gap
    and gives us regression coverage for the actual export path.
  • chore(otel): reorganize codex-otel crate (#13800)
    ## Summary
    This is a structural cleanup of `codex-otel` to make the ownership
    boundaries a lot clearer.
    
    For example, previously it was quite confusing that `OtelManager` which
    emits log + trace event telemetry lived under
    `codex-rs/otel/src/traces/`. Also, there were two places that defined
    methods on OtelManager via `impl OtelManager` (`lib.rs` and
    `otel_manager.rs`).
    
    What changed:
    - move the `OtelProvider` implementation into `src/provider.rs`
    - move `OtelManager` and session-scoped event emission into
    `src/events/otel_manager.rs`
    - collapse the shared log/trace event helpers into
    `src/events/shared.rs`
    - pull target classification into `src/targets.rs`
    - move `traceparent_context_from_env()` into `src/trace_context.rs`
    - keep `src/otel_provider.rs` as a compatibility shim for existing
    imports
    - update the `codex-otel` README to reflect the new layout
    
    ## Why
    `lib.rs` and `otel_provider.rs` were doing too many different jobs at
    once: provider setup, export routing, trace-context helpers, and session
    event emission all lived together.
    
    This refactor separates those concerns without trying to change the
    behavior of the crate. The goal is to make future OTEL work easier to
    reason about and easier to review.
    
    ## Notes
    - no intended behavior change
    - `OtelManager` remains the session-scoped event emitter in this PR
    - the `otel_provider` shim keeps downstream churn low while the
    internals move around
    
    ## Validation
    - `just fmt`
    - `cargo test -p codex-otel`
    - `just fix -p codex-otel`
  • feat(core): persist trace_id for turns in RolloutItem::TurnContext (#13602)
    This PR adds a durable trace linkage for each turn by storing the active
    trace ID on the rollout TurnContext record stored in session rollout
    files.
    
    Before this change, we propagated trace context at runtime but didn’t
    persist a stable per-turn trace key in rollout history. That made
    after-the-fact debugging harder (for example, mapping a historical turn
    to the corresponding trace in datadog). This sets us up for much easier
    debugging in the future.
    
    ### What changed
    - Added an optional `trace_id` to TurnContextItem (rollout schema).
    - Added a small OTEL helper to read the current span trace ID.
    - Captured `trace_id` when creating `TurnContext` and included it in
    `to_turn_context_item()`.
    - Updated tests and fixtures that construct TurnContextItem so
    older/no-trace cases still work.
    
    ### Why this approach
    TurnContext is already the canonical durable per-turn metadata in
    rollout. This keeps ownership clean: trace linkage lives with other
    persisted turn metadata.
  • feat(app-server): add tracing to all app-server APIs (#13285)
    ### Overview
    This PR adds the first piece of tracing for app-server JSON-RPC
    requests.
    
    There are two main changes:
    - JSON-RPC requests can now take an optional W3C trace context at the
    top level via a `trace` field (`traceparent` / `tracestate`).
    - app-server now creates a dedicated request span for every inbound
    JSON-RPC request in `MessageProcessor`, and uses the request-level trace
    context as the parent when present.
    
    For compatibility with existing flows, app-server still falls back to
    the TRACEPARENT env var when there is no request-level traceparent.
    
    This PR is intentionally scoped to the app-server boundary. In a
    followup, we'll actually propagate trace context through the async
    handoff into core execution spans like run_turn, which will make
    app-server traces much more useful.
    
    ### Spans
    A few details on the app-server span shape:
    - each inbound request gets its own server span
    - span/resource names are based on the JSON-RPC method (`initialize`,
    `thread/start`, `turn/start`, etc.)
    - spans record transport (stdio vs websocket), request id, connection
    id, and client name/version when available
    - `initialize` stores client metadata in session state so later requests
    on the same connection can reuse it