feat: unify the recording/forwarding configuration interface. - #6
Merged
Conversation
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
Recording storage was configured cluster-wide (
ProxyClusterBuilder::storage(...)) while replay was configured per-upstream (aReplaySourcepassed toadd_upstream_with). The two halves of the same record→replay round-trip were wired through different mechanisms and didn't reference each other — so it was easy to set a replay source per upstream and assume recording would land in the same place, when in fact recording went to a single shared sink (or, with no.storage(...)call, to an in-memory ring that was silently discarded on shutdown).This PR unifies the two: storage is now configured per upstream, and a single medium drives both directions.
What changed
Public API (breaking)
ProxyClusterBuilder::storage(...)and the cluster-wide storage field.Snapshotsbundle type — the single per-upstream storage knob:Snapshots::from_storage(storage, strategy)— a durable medium (e.g. a JSONL file) used as both the replay source (loaded + indexed atrun()) and the recording sink (appended to inMode::Record).Snapshots::in_memory(exchanges, strategy)— a replay-only source, never written back (tests/fixtures).add_upstream_with/add_upstream_with_modenow takeOption<Snapshots>instead ofOption<ReplaySource>.Recorder::with_storage(cfg, Option<SharedStorage>)→Recorder::with_routes(cfg, HashMap<String, SharedStorage>).Recorder::storage()→Recorder::storage_for(&str).Internals
QueryTraffic/AssertSeen/AssertCountare unchanged), but durable storage became a route map keyed by upstream name:record()appends each exchange to its own upstream's medium (durable-before-memory ordering preserved), andflush()fences every backend.run()(already async), each upstream'sSnapshotsis resolved — its existing contents stream into the indexedReplaySource, and a durable medium is registered as that upstream's recording sink.Versioning
0.3.0 → 0.4.0. The API removals/signature changes are breaking; under Cargo's semver for0.xthat's a minor bump, which clears the failingcargo semver-checksgate. Workspace version + internal dep entries +Cargo.lockupdated in lock-step.Docs
SPECIFICATION.md§3.3, §4, §4.1, §6.4, §8, §9.1, §9.2, §20.1 updated to describe per-upstreamSnapshots; removed the deadpersist_path/ProxyClusterBuilder::storagereferences.Migration
Behavior note
Because the medium feeds replay too, a re-run in
Mode::Recordreplays any request already present in the file rather than re-recording it (the snapshot acts as a dedup cache, §8.3) and only forwards genuinely new requests. Delete the file to force a clean re-capture.Testing
cargo build --workspace --all-targets— cleancargo test -p partly-proxy-lib— all suites pass (record 5/5, replay 11/11, lib units 73/73)cargo clippy -p partly-proxy-lib --all-targets— clean