feat: Phase 2 telemetry hardening + Phase 3 Docker/CI publish#4
Conversation
…iner publish Phase 2 — Telemetry Hardening (TEL-01–04): - W3CTraceContextMiddleware extracts traceparent/tracestate on every request - install_propagator() sets global CompositePropagator with TraceContextTextMapPropagator - configure_telemetry() wires AI on connection string; no-op otherwise - current_traceparent() helper surfaces active span ID for downstream events - 10 new telemetry tests (session-scoped OTel provider, exporter cleared per test) - Fixed: opentelemetry.context.attach/detach (not trace.context) Phase 3 — Docker + CI Publish (DOCK-01–03): - Multi-stage Dockerfile (builder+runtime), non-root USER app, STOPSIGNAL SIGTERM - docker-compose.yml for local smoke testing - CI: docker build → smoke test → GHCR push on main merge - 47/47 tests green, coverage 91.94% Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc14911702
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def configure_telemetry(settings: Settings) -> None: | ||
| install_propagator() | ||
|
|
||
| if settings.applicationinsights_connection_string: |
There was a problem hiding this comment.
Install an SDK tracer provider when export is disabled
When APPLICATIONINSIGHTS_CONNECTION_STRING is absent, which is the default local configuration, this function only installs the propagator and never installs an SDK TracerProvider. Consequently the newly added API/workflow spans are non-recording no-op spans, so local requests produce no telemetry and the extracted inbound trace context is replaced when child spans start. The tests conceal this because session_otel_exporter manually installs a provider; configure a provider in the no-exporter path as well.
Useful? React with 👍 / 👎.
| orchestrator = WorkflowOrchestrator(service, app_settings.repository) | ||
| try: | ||
| result = orchestrator.execute(envelope) | ||
| except WorkflowAgentServiceError: |
There was a problem hiding this comment.
Emit the failed lifecycle event for unexpected workflow errors
If the workflow service or orchestrator raises anything other than WorkflowAgentServiceError—for example a local adapter bug or result-validation failure—the exception bypasses this handler, leaving the API span with workflow.started but no workflow.failed event. WorkflowOrchestrator.execute explicitly rethrows all exceptions, so the failure event should be added for the general exception path while preserving the existing 502 translation only for WorkflowAgentServiceError.
Useful? React with 👍 / 👎.
Summary
Test plan
Generated with Claude Code