Skip to content

Body-logging decorators and StepMetadata are public surface with no in-tree consumer #70

@OmarAlJarrah

Description

@OmarAlJarrah

We ship two pieces of public toolkit surface that nothing in the tree actually drives. I want to decide whether to wire them, reframe them, or trim them, because right now they read like first-class features but behave like unused building blocks.

Current design

LoggableRequestBody / LoggableResponseBody (packages/dexpace-sdk-core/src/dexpace/sdk/core/http/request/loggable_request_body.py, .../http/response/loggable_response_body.py) are RequestBody/ResponseBody decorators that mirror forwarded bytes into an in-memory tap and expose snapshot(). They carry genuinely delicate behaviour:

  • the tap is reset at the start of every iter_bytes so a replayed body (retry, 307) captures a single copy rather than accumulating body + body (lines 72-87);
  • the tap is soft-capped, and snapshot(max_bytes=...) bounds the slice through a memoryview so a large payload is never fully materialised;
  • the response side does a one-time drain into a bytes cache so subsequent reads replay.

The shipped logging path does not use any of this. LoggingPolicy (.../pipeline/policies/logging_policy.py) emits method, redacted URL, status, duration, and trace id — headers and status only. It never wraps request.body or the response, and never calls snapshot(). A search of the pipeline (policies, runners, transports) finds no component that instantiates either decorator. The behaviours above are exercised only by unit tests.

StepMetadata (.../pipeline/step/config.py) is a frozen dataclass (name, description, version, tags) whose docstring says it is "Used by logging, tracing, and tooling that needs to identify a step at runtime." No runner, logging policy, or tracing policy reads it. The PipelineStep Protocol in the same package is a bare callable (value, context) -> output that neither carries nor requires a StepMetadata. Outside its own module it appears only in pipeline/__init__.py and __all__.

Trade-off / concern

This is speculative public surface, and it sits against our own narrow-API and zero-tech-debt conventions in CLAUDE.md ("Public API is narrow," "Helpers and concrete adapter classes are module-private").

For the Loggable* decorators, the hardest-to-get-right code in the body layer (tap reset on replay, partial-read retention, capped/bounded snapshot) only pays off when something drives a wrapped body through retries/redirects and then reads snapshot() at the right moment. No shipped component does. A consumer who wants body logging has to discover the decorator, insert it at the correct pipeline position relative to retry's to_replayable() body swap (so the snapshot reflects the instance actually sent), and call snapshot() at the right time — a lot of undocumented assembly for what docs/bodies.md presents as a ready feature under "Body capture for logging." Nothing in the docs states that the shipped LoggingPolicy does not capture bodies, or how the wrapper interacts with retry's body swap.

For StepMetadata, the docstring asserts a consumer relationship that does not exist. It freezes a public dataclass shape against a guess about what a future logging/tracing integration will want, with nothing to validate that guess.

Proposed direction

Loggable* — pick one:

  1. Wire it. Give LoggingPolicy an opt-in flag that wraps request.body (positioned so it holds the body actually sent after retry's to_replayable() swap) and the response, and emits snapshot() at a configured level, with an integration test that drives a wrapped body through a retry and asserts the snapshot is the last attempt's payload, not body + body. This adds a logging-path surface and an assembly contract, but it puts the delicate code behind something that exercises it.
  2. Reframe it honestly. Keep the decorators as documented building blocks, but state in docs/bodies.md exactly where to insert the wrapper in a pipeline, how it interacts with retry's to_replayable() swap, when to call snapshot(), and explicitly that the shipped LoggingPolicy does not capture bodies. This keeps surface small at the cost of leaving the hardest code validated only in isolation.

StepMetadata — pick one:

  1. Wire it into something real — e.g. runners or the logging/tracing policies reading it to label spans/records — so the docstring's claim becomes true; or
  2. Drop it from the public surface until a consumer exists, rather than freezing its shape against a non-existent integration.

Acknowledging the current rationale

The CHANGELOG's "Honest scope boundaries" is explicit that async logging/tracing was deferred and that body logging beyond status/headers is limited, and it does describe the Loggable* bodies as bounded/repeatable primitives. So the absence of a body-logging policy is a known scope line, not an accident. The point of revisiting is narrower: the primitives are presented (docstring on StepMetadata, the "Body capture for logging" section in docs/bodies.md) as having consumers and a clear usage story they don't yet have. Either closing that gap (wiring) or making the docs match reality (reframing/trimming) keeps the public surface honest under the project's own narrow-API bar without expanding scope we deliberately deferred.

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationenhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions