Plan: integrate request correlation across HTTP, tasks, and outbound provider calls (4.1.3) - #280
Draft
leynos wants to merge 6 commits into
Draft
Plan: integrate request correlation across HTTP, tasks, and outbound provider calls (4.1.3)#280leynos wants to merge 6 commits into
leynos wants to merge 6 commits into
Conversation
Add docs/execplans/4-1-3-integrate-request-correlation.md, the execution plan for roadmap item 4.1.3, covering Falcon ASGI middleware wiring, runtime configuration, Celery propagation, and outbound provider-call correlation. The plan records several findings established by probing the runtime rather than by reading documentation: - falcon-correlate has no tags or releases and is absent from PyPI, so the dependency must be pinned to a commit SHA. - femtologging's log_context fields never reach records emitted through get_logger, so ContextualLogFilter is unusable; the plan decorates messages inside the existing logging helpers instead. - Celery's before_task_publish does not fire in eager mode, and eager tasks inherit the caller's context, so a naive eager-mode test would pass against an unwired application. The verification plan splits the Celery obligations to avoid that vacuity. - An rpc:// result backend silently disables publish-time propagation. - Falcon's req.remote_addr is the peer address and falls back to 127.0.0.1 when the ASGI scope omits the client, which makes trusting loopback a real hazard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rename the axiom identifiers from AX-n to AXIOM-n so the typos spelling gate stops reading "AX" as a misspelling of "AXE", remove the leading spaces from inside code spans that tripped MD038, and rewrap three over-length lines. make markdownlint now reports 0 issues. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add the execplan to docs/contents.md alongside the other phase-4 API plans so the documentation index stays complete. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A design-review panel stress-tested the draft and falsified four of its load-bearing premises. Each was verified by probing the runtime, and each changed the plan: - make check-architecture cannot enforce the boundary the plan claimed it enforced. Hecate's include_external_packages defaults to false, so it sees zero third-party imports. Constraint C2 is reworded and INV-9 pins the import graph instead. - Not every log call funnels through the episodic/logging.py helpers: 29 of 48 call sites bypass them, including the authorization denial the roadmap names, which logs at DEBUG through a direct logger.log call. The draft's marquee transcript was fabricated; it is replaced with the message the code emits. - import falcon_correlate registers the Celery signal handlers globally at import time, so configure_celery_correlation(app) is declarative and INV-4a's negative control was unfalsifiable. It is now signal disconnection. - episodic/observability.py is 399 lines against a blocking 400-line Pylint limit, so the planned edit would have failed make lint. EP-M3a now splits it. Two proposed fixes that looked correct were tested and rejected: importing a falcon_correlate submodule does not avoid the Celery side effect, because Python runs the package __init__ first; and hypothesis.event() cannot enforce class coverage, so INV-1 now uses @example anchors. Also added: unconditional identifier sanitization after verifying that a 60,000-byte header and a logfmt-injection payload both round-trip today; a logging bootstrap, without which the whole milestone emits no bytes in the container; a bounded correlation-source counter; an rpc:// hard error; prefork context-leak and import-cost obligations; and an explicit client factory signature after the draft's **kwargs: object failed ty with 18 errors. Risk R2 is rewritten: Granian does populate scope["client"], so the draft aimed at a non-hazard, while the real one is that Traefik pod IPs give operators no stable range to trust and relay the header verbatim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Your free Security trial is over. An organization admin can activate billing to continue. |
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueWarning Your free Security trial is over. An organization admin can activate billing to continue. Comment |
Reviewer's GuideAdds a detailed execution plan document for roadmap item 4.1.3 (request correlation across HTTP, Celery, and outbound provider calls) and indexes it in the docs contents, with no production code changes. Sequence diagram for planned HTTP request correlationsequenceDiagram
participant Client
participant Traefik
participant Falcon
participant Authorization
participant Celery
participant Provider
Client->>Traefik: HTTP request
Traefik->>Traefik: Overwrite X-Correlation-ID
Traefik->>Falcon: Forward request
Falcon->>Falcon: CorrelationIDMiddlewareASGI.process_request
Falcon->>Authorization: AuthorizationMiddleware.process_request
Authorization-->>Falcon: Allow or deny
Falcon->>Celery: Publish task with correlation_id
Falcon->>Provider: HTTP request with X-Correlation-ID
Falcon-->>Client: Response with X-Correlation-ID
Sequence diagram for planned Celery correlation propagationsequenceDiagram
participant HTTP as HTTP Service
participant Celery as Celery Publish Signal
participant Worker
participant Task
HTTP->>Celery: before_task_publish
Celery->>Celery: propagate_correlation_id_to_celery
Celery-->>Worker: Message with correlation_id
Worker->>Worker: task_prerun restores correlation_id
Worker->>Task: Execute task
Task-->>Worker: task_postrun clears correlation_id
Sequence diagram for planned outbound provider correlationsequenceDiagram
participant Request as Request Context
participant Adapter as OpenAICompatibleLLMAdapter
participant Client as httpx AsyncClient
participant Provider as OpenAI-Compatible Provider
Request->>Adapter: LLM operation
Adapter->>Client: Build correlated client
Client->>Provider: HTTP request with X-Correlation-ID
Provider-->>Client: Provider response
Client-->>Adapter: Response
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Upstream cut v0.1.0, resolving to caea7a6ac804f851f7226ccf9acb3d256cc2d5d4 — the same commit every empirical finding in the plan was probed against, so no finding needs re-verifying. Verified that the tag installs and reports version 0.1.0. Decision D1 now pins the tag unconditionally, matching the df12-python-lints tag pin rather than the femtologging and tei-rapporteur commit pins, and states the rule: prefer a tag whenever upstream offers one. Risk R1 drops from medium/certain to low/low. What survives is narrower and worth keeping: the package is still absent from PyPI, has a single author, is alpha-status, and carries the trust decision in R2 plus the private-attribute coupling in R6. A Git tag is also mutable where a commit is not, so EP-M0 now asks the implementer to confirm the locked revision in uv.lock is caea7a6a; a moved tag then surfaces as a reviewable lockfile change rather than a silent behaviour swap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eview Cross-reference the eight issues filed against femtologging, falcon-correlate, and episodic from the discoveries that produced them, so an implementer reading a Surprises entry can see whether the underlying defect is being fixed at source. Also replaces EP-M0's "file two upstream issues" pre-step: both are now filed (falcon-correlate#158 and #159), and neither blocks this plan. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 the execution plan for roadmap item 4.1.3, integrating request
correlation across the Falcon HTTP boundary, Celery tasks, and outbound
OpenAI-compatible provider calls.
docs/execplans/4-1-3-integrate-request-correlation.mddocs/roadmap.mdlines 606-626docs/episodic-podcast-generation-system-design.md:2033-2044This PR contains no production code — only the plan and its index entry.
What the plan covers
Ten milestones: pin
falcon-correlate; build an episodic-owned correlationseam with sanitization and runtime configuration; wire
CorrelationIDMiddlewareASGIahead of the authorization middleware; splitepisodic/observability.pybelow the 400-line limit; bootstrap logging andcarry the identifier into log lines; configure Celery propagation; correlate
outbound provider calls; two behavioural-test milestones; and documentation
with a new ADR 018.
Design review
The first draft was stress-tested by a six-lens review panel — structure,
alternatives, scaling and observability, contracts, failure modes, and
long-term viability. It falsified four load-bearing premises. Each was verified
by probing the runtime rather than by reading documentation, and each changed
the plan.
make check-architecturecannot enforce the boundary the draft relied on.Hecate's
include_external_packagesdefaults tofalseand[tool.hecate]does not set it, so the gate sees zero third-party imports and would pass a
domain module importing
httpxdirectly. Constraint C2 is reworded as a reviewobligation and a new import-graph test replaces the imagined gate.
Not every log call funnels through the logging helpers. 29 of 48 call sites
bypass them — including
_log_authorization_denial, the roadmap's own headlineexample, which logs at
DEBUGthrough a directlogger.logcall. The draft'smarquee transcript did not correspond to any message the codebase emits; it is
replaced with the real one, and the plan now schedules the edits instead of
claiming coverage it did not have.
import falcon_correlateregisters the Celery signal handlers globally, atimport time, and pulls Celery in at ~87 ms.
configure_celery_correlation(app)ignores its argument entirely, so the draft's negative control — "delete the
call and the test must fail" — was unfalsifiable. It is now signal
disconnection.
episodic/observability.pyis 399 lines against a blocking 400-line Pylintlimit, so the planned edit would have failed
make linton the first run. Anew milestone splits it first, including the ~20 Skylos entry-point rules that
must be renamed in lockstep.
Two fixes that looked right were tested and rejected: importing a
falcon_correlatesubmodule does not avoid the Celery side effect, becausePython executes the package
__init__first; andhypothesis.event()cannotenforce class coverage, because it only feeds the statistics report and a
module-level counter is unsafe under
pytest-xdist.Security findings folded in
Verified against Granian: a 60,000-byte correlation header is accepted,
stored, and echoed in full, and the payload
abc method=GET path=/admin principal_id=rootround-trips intact — audit-trail forgery in a last-winslogfmt parser, since the identifier is appended last. Sanitization is now
unconditional at the seam rather than dependent on an optional upstream
validator.
Risk R2 is rewritten. Granian does populate
scope["client"], so thedraft's stated hazard was largely theoretical. The real one is that Traefik on
DOKS gives operators no stable range to trust — pod IPs change on every
reschedule — while relaying the header verbatim, so the path of least
resistance trusts every pod in the cluster. The plan now ships a Traefik
middleware that overwrites the header at the edge.
Notes for the reviewer
correlate_clienthelper beyond the roadmap's literal "falcon-correlate transport support",
because every existing adapter fixture injects a client and the transport
approach structurally cannot reach those. Flagged as a reviewer gate before
EP-M5.
falcon-correlateis pinned atv0.1.0, which upstream cut in responseto the draft. It resolves to
caea7a6a— the exact commit every finding herewas probed against — so nothing needed re-verifying. Risk R1 drops from
medium/certain to low/low; what survives is that the package is still absent
from PyPI, single-author, and alpha-status. Because a Git tag is mutable where
a commit is not, EP-M0 asks the implementer to confirm the locked revision in
uv.lock, so a moved tag surfaces as a reviewable lockfile change.make markdownlint(which runs the spelling gate) passes with 0 issues.References
🤖 Generated with Claude Code
Summary by Sourcery
Define the implementation plan for end-to-end request correlation across Episodic’s HTTP, worker, logging, and provider boundaries.
Enhancements:
Documentation: