diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 900ae15..d4a0028 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -1,19 +1,29 @@ -# Docs: publish the mkdocs site to GitHub Pages on every push to main. +# Docs: build the versioned documentation site with mike (Material for MkDocs) +# and publish it to the `gh-pages` branch, which GitHub Pages serves. # # One-time repository setup (Settings -> Pages): -# Source: "GitHub Actions". +# Source: "Deploy from a branch" -> branch `gh-pages`, folder `/ (root)`. # -# The build also runs in `ci.yml`, on every pull request, so documentation -# breakage is caught before it reaches main rather than at the deploy. +# This replaced an `upload-pages-artifact` + `deploy-pages` pair, and with it the +# "Source: GitHub Actions" setting. mike needs a branch it owns: it keeps one +# built site per version there and maintains the `versions.json` the header +# switcher reads, neither of which survives a model where every deploy replaces +# the whole artifact. # -# Deployed through the Pages actions rather than by pushing a `gh-pages` -# branch: no branch to maintain, and the artifact is what Pages serves. +# Versioning: +# - A final release tag (`v1.2.3`) publishes that release under its bare +# version number and moves the `latest` alias, which the site root redirects +# to. +# - A PRE-RELEASE tag (`v1.2.3-rc.1`, or any tag with a `-` qualifier) +# publishes nothing. Release candidates are cut from `develop` to exercise +# the release pipeline; their documentation is in-progress documentation, +# already covered by `dev`, and publishing it only adds throwaway entries to +# the switcher that someone then has to delete by hand. +# - A push to `main` publishes the in-progress documentation under `dev`. +# - `workflow_dispatch` re-runs whatever ref it is pointed at. # -# Not versioned. `mike` would put a version switcher in the header and keep a -# page per release, which is worth doing once there are releases to switch -# between; there are none yet, and an empty switcher is worse than none. When -# the first tag lands, add `extra.version.provider: mike` to `mkdocs.yml` and -# replace the build step below with `mike deploy`. +# The strict build also runs in `ci.yml` on every pull request, so documentation +# breakage is caught before it reaches main rather than at the deploy. name: Docs @@ -21,30 +31,38 @@ on: push: branches: - main + tags: + # Final releases only: the negative pattern excludes any pre-release tag + # (a `-` qualifier), so an RC never starts this workflow at all. + - "v*" + - "!v*-*" workflow_dispatch: +# mike pushes commits to the gh-pages branch. No `pages: write` or +# `id-token: write` any more -- nothing here talks to the Pages deployment API. permissions: - contents: read - pages: write - # Required by actions/deploy-pages: it exchanges this for a deployment token - # rather than using a stored secret. - id-token: write + contents: write -# One deploy at a time, and do not cancel one that is already publishing -- -# an interrupted deployment can leave Pages serving a half-uploaded site. +# Serialize deploys so two runs never race on pushing gh-pages. Not cancelled +# in progress: an interrupted mike run can leave the branch with a site +# committed and its `versions.json` not yet updated. concurrency: - group: pages + group: docs-deploy cancel-in-progress: false jobs: - build: - name: Build the site + deploy: + name: Build and deploy versioned docs runs-on: ubuntu-latest timeout-minutes: 10 steps: - name: Checkout repository uses: actions/checkout@v7 + with: + # mike reads and rewrites the gh-pages branch, so it needs history + # rather than the default shallow clone of one ref. + fetch-depth: 0 # Pinned to an exact release, unlike every other action here. astral-sh # publishes `v9.0.0` but stopped publishing the sliding major tag after @@ -52,26 +70,43 @@ jobs: - name: Install uv uses: astral-sh/setup-uv@v9.0.0 - - name: Build the site (strict) - run: uv run --with-requirements docs/requirements.txt mkdocs build --strict + - name: Configure git identity for mike + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - - name: Upload the site as a Pages artifact - uses: actions/upload-pages-artifact@v5 - with: - # `site/` is what mkdocs writes and what .gitignore excludes. - path: site + # `|| true`: the branch does not exist before the first deploy, and mike + # creates it. A missing branch is not a failure here. + - name: Make the gh-pages branch available to mike + run: git fetch origin gh-pages --depth=1 || true - deploy: - name: Deploy to GitHub Pages - needs: build - runs-on: ubuntu-latest - timeout-minutes: 10 + - name: Deploy + run: | + set -euo pipefail + mike() { + uv run --with-requirements docs/requirements.txt mike "$@" + } - environment: - name: github-pages - url: ${{ steps.deployment.outputs.page_url }} + if [ "${GITHUB_REF_TYPE}" != "tag" ]; then + echo "Publishing development docs (alias: dev)" + mike deploy --push --update-aliases dev + exit 0 + fi - steps: - - name: Deploy - id: deployment - uses: actions/deploy-pages@v5 + version="${GITHUB_REF_NAME#v}" + # The tag filter above already keeps pre-releases out, but + # workflow_dispatch can be pointed at any ref -- so refuse here too + # rather than trusting the trigger alone. + case "${version}" in + *-*) + echo "${version} is a pre-release; skipping docs publication." + exit 0 + ;; + esac + + echo "Publishing release docs ${version} (alias: latest)" + mike deploy --push --update-aliases "${version}" latest + # Idempotent, and cheap enough to repeat: it writes the redirect at the + # site root. Doing it on every release means a gh-pages branch that was + # ever rebuilt by hand does not need this done separately. + mike set-default --push latest diff --git a/CHANGES.md b/CHANGES.md index 15347fd..b470639 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,6 +8,59 @@ release promotes it to a version heading; the `bump-version` skill does that. ## Development +### Changed + +- The documentation site is versioned with `mike`: one built copy per release on + the `gh-pages` branch, a switcher in the header, and the site root redirecting + to the newest release. It was a single unversioned site, so a reader on 0.1.0 + had no way to reach the documentation for the release they were running, and + publishing 0.2.0 silently replaced it. Pre-release tags publish nothing; a push + to `main` publishes `dev`. + +## 0.2.0 -- 2026-08-03 + +### Changed + +- A matching mock is now decided before `loss` and `latency`, not after, so + `replace` is the share of matching requests a mock answers rather than the + share of those that survived a loss roll. Previously `loss: 0.5` halved every + `replace` in the proxy, and no configuration could ask a mock to answer half + of its matching requests while any loss was set. The proxy's `loss` and + `latency` no longer apply to a request a mock answered: they describe the real + backend, and a mock replaces it. +- A run of slashes at the start of a request path is collapsed to one before + mocks are matched, so `//api/v1/index/` matches a mock declared + `^/api/v1/index/$`. Clients produce the doubled form by joining a base URL + ending in `/` to a path beginning with `/`; it is legal HTTP, nothing rejected + it, and the only symptom was an anchored mock silently not firing. Empty + segments elsewhere in the path are left alone. +- An injected `latency` is now a target for the whole response rather than an + addition to it: the time the upstream really took is subtracted, and only the + remainder is waited out. A 500ms latency in front of a backend answering in + 120ms delays by 380ms, where before it delayed by 500 and produced 620ms + total -- so the number written in the configuration was unreachable by + construction, and moved with whatever the upstream happened to be doing. An + upstream slower than the target leaves no remainder and is passed straight + through; the setting is a floor, never a ceiling. `latency_injected_ms` in the + log line is the wait actually taken and reads `0` in that case, while + `doppel_latency_injected_total` still counts the request. + +### Fixed + +- A mock's `proxy.loss` and `proxy.latency` are applied. They were parsed, + validated and compiled into the runtime, and then never read, so a mock + declaring either was silently answering every request it matched. They now + apply to the requests the mock answers, after it has won its `replace` roll, + and go through the same `decide` as the proxy's -- so loss short-circuits + latency there too. +- What a mock inherits from its proxy is now settled per setting rather than by + accident: `replace` and `latency` fall back to the proxy's, `loss` does not. + `latency` describes how slow the proxy is to answer, which holds whatever + answers, so a mocked response is delayed like any other and a mock's own value + overrides rather than adds to it. `loss` is excluded because a mock inheriting + it would be dropped by the proxy's loss, which is the coupling between `loss` + and `replace` the ordering above exists to remove. + ## 0.1.0 -- 2026-08-02 The first release. Everything below is new, so the sections are what a reader diff --git a/Cargo.lock b/Cargo.lock index 73d2141..3b86671 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -591,7 +591,7 @@ dependencies = [ [[package]] name = "doppel-admin" -version = "0.1.0" +version = "0.2.0" dependencies = [ "async-trait", "axum", @@ -610,7 +610,7 @@ dependencies = [ [[package]] name = "doppel-cli" -version = "0.1.0" +version = "0.2.0" dependencies = [ "anyhow", "clap", @@ -631,7 +631,7 @@ dependencies = [ [[package]] name = "doppel-core" -version = "0.1.0" +version = "0.2.0" dependencies = [ "arc-swap", "async-trait", @@ -654,7 +654,7 @@ dependencies = [ [[package]] name = "doppel-proxy" -version = "0.1.0" +version = "0.2.0" dependencies = [ "axum", "doppel-core", @@ -672,7 +672,7 @@ dependencies = [ [[package]] name = "doppel-render" -version = "0.1.0" +version = "0.2.0" dependencies = [ "doppel-core", "minijinja", @@ -681,7 +681,7 @@ dependencies = [ [[package]] name = "doppel-store-postgres" -version = "0.1.0" +version = "0.2.0" dependencies = [ "async-trait", "doppel-core", @@ -696,7 +696,7 @@ dependencies = [ [[package]] name = "doppel-telemetry" -version = "0.1.0" +version = "0.2.0" dependencies = [ "doppel-core", "sentry", diff --git a/Cargo.toml b/Cargo.toml index 7101b38..36e5845 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = ["crates/*"] resolver = "3" [workspace.package] -version = "0.1.0" +version = "0.2.0" edition = "2024" rust-version = "1.94" license = "Apache-2.0" diff --git a/crates/doppel-proxy/src/mock.rs b/crates/doppel-proxy/src/mock.rs index b793637..5b20d88 100644 --- a/crates/doppel-proxy/src/mock.rs +++ b/crates/doppel-proxy/src/mock.rs @@ -19,11 +19,16 @@ use doppel_render::Variables; /// declaration order distinguishes the two. Do not "fix" this by anchoring /// the pattern here -- doing so would silently change what an existing, /// unedited configuration means. +/// +/// The path is matched with any run of leading slashes collapsed to one, so a +/// request for `//api/v1/index/` is matched as `/api/v1/index/`. See +/// [`for_matching`]. pub fn match_mock<'a>( proxy: &'a CompiledProxy, method: &Method, path: &str, ) -> Option<(&'a CompiledMock, Variables)> { + let path = for_matching(path); proxy.mocks.iter().find_map(|mock| { // `as_str` on both sides, and no case folding: a mock can only // declare a method the type knows, and an incoming method that is @@ -45,6 +50,36 @@ pub fn match_mock<'a>( }) } +/// The request path as mock patterns see it: any run of leading slashes +/// collapsed to a single one. +/// +/// A request line of `GET //api/v1/index/` is legal HTTP, and clients produce +/// it by accident all the time -- a base URL ending in `/` joined to a path +/// beginning with `/` is the usual way. Nothing in the path's meaning changed, +/// but a pattern written `^/api/v1/index/$` no longer matched it, and the +/// request fell through to the upstream with no sign of why. Matching one slash +/// where the client sent several removes a class of "the mock just does not +/// fire" that has no diagnosis short of reading the request line. +/// +/// Only *leading* slashes. An empty segment in the middle of a path (`/a//b`) +/// is a segment, and collapsing it would be a claim about what the upstream +/// considers the same resource -- which is the upstream's to make, not this +/// function's. A borrow is returned when there is nothing to collapse, so the +/// common path allocates nothing. +fn for_matching(path: &str) -> &str { + match path.strip_prefix('/') { + // `trim_start_matches` on the remainder rather than on `path`, so the + // one slash the path is entitled to survives. + Some(rest) => { + let trimmed = rest.trim_start_matches('/'); + // Byte arithmetic on a `str`: every byte removed was `/`, which is + // ASCII, so the split cannot land inside a character. + &path[path.len() - trimmed.len() - 1..] + } + None => path, + } +} + /// Binds the mock's declared header variables. A header the request does not /// carry binds nothing -- referencing it in a template is an undefined /// variable, per section 5 of the design. @@ -185,6 +220,54 @@ mod tests { assert_eq!(matched.name, "m1"); } + /// An anchored pattern is the case that made this worth fixing: an + /// unanchored `/api/v1/index/` already matched `//api/v1/index/`, because + /// the doubled slash sits outside the substring it looks for. `^`-anchored, + /// it did not, and nothing said why. + #[test] + fn repeated_leading_slashes_match_an_anchored_pattern_written_with_one() { + let p = proxy(vec![mock("m1", "GET", "^/api/v1/index/$")]); + let (matched, _vars) = match_mock(&p, &Method::GET, "//api/v1/index/").unwrap(); + assert_eq!(matched.name, "m1"); + } + + #[test] + fn any_number_of_leading_slashes_collapses_to_one() { + let p = proxy(vec![mock("m1", "GET", "^/widgets/$")]); + for path in ["/widgets/", "//widgets/", "///widgets/", "/////widgets/"] { + assert!( + match_mock(&p, &Method::GET, path).is_some(), + "`{path}` should have matched" + ); + } + } + + /// Only the leading run. An empty segment in the middle of a path is a + /// segment, and whether `/a//b` and `/a/b` name the same resource is the + /// upstream's business -- collapsing it here would answer that question on + /// the upstream's behalf, for mocks only, and disagree with what gets + /// forwarded. + #[test] + fn an_empty_segment_inside_the_path_is_left_alone() { + let p = proxy(vec![mock("m1", "GET", "^/a/b/$")]); + assert!(match_mock(&p, &Method::GET, "/a//b/").is_none()); + } + + #[test] + fn for_matching_leaves_a_path_that_needs_nothing_untouched() { + // Also covers the degenerate paths, where an off-by-one in the slice + // arithmetic would panic or eat the last slash rather than merely + // return the wrong string. + assert_eq!(for_matching("/widgets/"), "/widgets/"); + assert_eq!(for_matching("/"), "/"); + assert_eq!(for_matching("//"), "/"); + assert_eq!(for_matching("///"), "/"); + assert_eq!(for_matching(""), ""); + // A request target that is not origin-form reaches this function + // unchanged; there is no leading slash to collapse. + assert_eq!(for_matching("widgets/"), "widgets/"); + } + /// Fixture names are adversarial on purpose: `zeta` is declared first and /// `alpha` second, so a regression that sorted mocks (alphabetically, or /// by any other accidental order) rather than preserving declaration diff --git a/crates/doppel-proxy/src/server.rs b/crates/doppel-proxy/src/server.rs index 494e9b2..872e91d 100644 --- a/crates/doppel-proxy/src/server.rs +++ b/crates/doppel-proxy/src/server.rs @@ -50,8 +50,11 @@ pub async fn serve( .await } -/// Pipeline order is fixed by the spec: resolve, loss, latency, forward. Phase 2 -/// inserts mock matching between latency and forwarding. +/// Pipeline order: resolve, then either a mock and the mock's own faults, or +/// the proxy's faults and a forward. A mock is decided before any fault, so +/// `replace` is the share of matching requests a mock answers rather than the +/// share of those that survived a loss roll, and the two sets of faults never +/// both apply to one request. See the comment on the mock branch below. async fn handle( State(state): State, ConnectInfo(peer): ConnectInfo, @@ -113,65 +116,95 @@ async fn handle( } }; - let faults = decide( - proxy.loss.as_ref(), - proxy.latency.as_ref(), - state.sampler.as_ref(), - ); - - if let Some(status) = faults.loss_status { - let response = with_request_id( - Response::builder() - .status(status) - .body(axum::body::Body::empty()) - .expect("status came from a validated config"), - &request_id, - ); - let elapsed = started.elapsed(); - metrics::record_loss(&proxy.name); - metrics::record_proxy(&proxy.name, method.as_str(), status, elapsed); - tracing::info!( - request_id, - proxy = proxy.name, - method = %method, - path, - status, - duration_ms = elapsed.as_millis(), - upstream_contacted = false, - loss_injected = true, - latency_injected_ms = 0u128, - "request dropped" - ); - return response; - } - - let latency_ms = match faults.latency { - Some(delay) => { - metrics::record_latency_injected(&proxy.name); - tokio::time::sleep(delay).await; - delay.as_millis() - } - None => 0, - }; - - // Mock matching sits here, between latency and forwarding, because the - // spec fixes that order: faults are a property of the proxy and apply - // before an endpoint is chosen, while a mock replaces the endpoint. + // Mock matching comes before fault injection, so `replace` means what it + // says: the share of matching requests a mock answers. Deciding the faults + // first made it the share of whatever survived the loss roll instead -- + // `loss: 0.5` quietly halved every `replace` in the proxy, and no + // configuration could express "answer half of these from a mock" while any + // loss was set. + // + // Proxy-level `loss` and `latency` therefore do not touch a request a mock + // answered. They describe the real backend, and a mock replaces the real + // backend -- which is the distinction the documentation already drew + // ("loss and latency make the real backend worse; `replace` decides how + // much of it is still involved at all"). + // + // A mock that wants faults declares its own, in its `proxy` block, and + // those are applied here -- after it has won the `replace` roll, so they + // bear only on requests this mock would actually have answered. They do not + // fall back to the proxy's, the way `replace` does: inheriting the proxy's + // `loss` would drop mocked requests again and put back exactly the coupling + // the ordering above exists to remove. `replace` can sensibly have a + // proxy-wide default because it describes how much of the proxy a mock + // stands in for; `loss` and `latency` describe the upstream, which a mock + // is not. // // The `replace` roll is what makes a matched mock optional -- a proxy can - // serve a mock some of the time and the real backend the rest -- so a - // mock that matches but loses the roll falls through to `forward` below - // with its request untouched. That is why `serve_mock`, which consumes - // the request, is only reached inside the winning branch. + // serve a mock some of the time and the real backend the rest -- so a mock + // that matches but loses the roll falls through to the faults and then to + // `forward` below, with its request untouched. That is why `serve_mock`, + // which consumes the request, is only reached inside the winning branch. if let Some((mock, vars)) = crate::mock::match_mock(proxy, &method, &path) { let replace = mock.replace.unwrap_or(proxy.replace); if fires(replace, state.sampler.as_ref()) { + // The same `decide` the forwarding path uses, so "what a fault roll + // means" -- including loss short-circuiting latency, so a dropped + // request is not delayed first -- has one definition. + // + // `latency` falls back to the proxy's, `loss` does not. The + // configured latency describes how slow this proxy is to answer, + // whatever answers, so it applies to a mocked request too and a + // mock's own value overrides rather than adds to it. Loss is the + // one that must not be inherited: a mock picking up the proxy's + // would be dropped by it, which is the coupling between `loss` and + // `replace` the ordering above exists to remove. + let faults = decide( + mock.loss.as_ref(), + mock.latency.as_ref().or(proxy.latency.as_ref()), + state.sampler.as_ref(), + ); + + if let Some(status) = faults.loss_status { + let response = with_request_id( + Response::builder() + .status(status) + .body(axum::body::Body::empty()) + .expect("status came from a validated config"), + &request_id, + ); + let elapsed = started.elapsed(); + // Loss, but not a mock hit: the mock decided this response + // rather than rendering one, and `mock_hits_total` counts + // mocks that answered. The mock is named in the log line + // instead, which is where "whose loss fired" belongs. + metrics::record_loss(&proxy.name); + metrics::record_proxy(&proxy.name, method.as_str(), status, elapsed); + tracing::info!( + request_id, + proxy = proxy.name, + mock = mock.name, + method = %method, + path, + status, + duration_ms = elapsed.as_millis(), + upstream_contacted = false, + loss_injected = true, + latency_injected_ms = 0u128, + "request dropped" + ); + return response; + } + + // Rendered first, then padded: the delay is a target for the whole + // response, so whatever producing it cost comes out of the wait. + let rendering = std::time::Instant::now(); let outcome = serve_mock(&runtime.config.templates.dir, proxy, mock, vars, request).await; let (response, error_code) = match outcome { Ok(response) => (response, None), Err(err) => (error_response(&err), Some(err.code.as_str())), }; + let latency_ms = pad_to_target(faults.latency, rendering.elapsed(), &proxy.name).await; let response = with_request_id(response, &request_id); let elapsed = started.elapsed(); metrics::record_mock_hit(&proxy.name, &mock.name); @@ -205,7 +238,52 @@ async fn handle( } } - match forward( + let faults = decide( + proxy.loss.as_ref(), + proxy.latency.as_ref(), + state.sampler.as_ref(), + ); + + if let Some(status) = faults.loss_status { + let response = with_request_id( + Response::builder() + .status(status) + .body(axum::body::Body::empty()) + .expect("status came from a validated config"), + &request_id, + ); + let elapsed = started.elapsed(); + metrics::record_loss(&proxy.name); + metrics::record_proxy(&proxy.name, method.as_str(), status, elapsed); + tracing::info!( + request_id, + proxy = proxy.name, + method = %method, + path, + status, + duration_ms = elapsed.as_millis(), + upstream_contacted = false, + loss_injected = true, + latency_injected_ms = 0u128, + "request dropped" + ); + return response; + } + + // Forwarded first, then padded. The upstream is a real server and takes + // real time, and the configured latency is what the client should + // experience in total -- so the wait is the remainder, not an addition. A + // 500ms latency in front of an upstream that answers in 120ms sleeps 380ms. + // Sleeping first would have produced 620ms and made the configured number + // unreachable by construction. + // + // Timed around the call rather than read from `UpstreamOutcome`, so the + // failure arm -- which produces no outcome -- is padded by the same rule as + // the success arm. A refused connection returning in 1ms would otherwise + // ignore the latency entirely, which is exactly the case someone + // configuring latency wants to see slow. + let attempt = std::time::Instant::now(); + let forwarded = forward( &runtime.client, proxy, request, @@ -213,8 +291,10 @@ async fn handle( &runtime.resolve_headers, &request_id, ) - .await - { + .await; + let latency_ms = pad_to_target(faults.latency, attempt.elapsed(), &proxy.name).await; + + match forwarded { Ok((response, outcome)) => { let elapsed = started.elapsed(); metrics::record_upstream( @@ -309,6 +389,38 @@ async fn handle( } } +/// Sleeps whatever is left of `target` once `spent` has already gone by, and +/// reports how long that wait was in milliseconds. +/// +/// `target` is the delay the latency roll produced, or `None` when it did not +/// fire -- in which case nothing is waited on and nothing is counted. +/// +/// The delay is a target for the response as a whole, not an addition to it, so +/// producing the response is paid for out of the wait. `saturating_sub` is the +/// whole of the "or nothing" case: an upstream slower than the target leaves no +/// remainder and the request is passed straight through. Doppel does not make a +/// slow backend look fast, and a latency setting is a floor, not a budget. +/// +/// The counter increments whenever the roll fired, including when the remainder +/// came out at zero: the fault applied to the request, and what varied was how +/// much of it the upstream had already delivered. `latency_injected_ms` in the +/// log line is the wait actually taken, so the two answer different questions +/// on purpose -- how often latency was in play, and how much of it this request +/// felt. +async fn pad_to_target( + target: Option, + spent: std::time::Duration, + proxy_name: &str, +) -> u128 { + let Some(target) = target else { + return 0; + }; + metrics::record_latency_injected(proxy_name); + let remainder = target.saturating_sub(spent); + tokio::time::sleep(remainder).await; + remainder.as_millis() +} + /// Reuse an incoming `X-Request-ID` so one request can be followed across /// services; generate one otherwise. A header that is not valid ASCII is /// replaced rather than propagated, since it cannot be logged faithfully. @@ -701,6 +813,77 @@ proxies: assert_eq!(response.status(), StatusCode::BAD_GATEWAY); } + /// The arithmetic of the padding, on its own, where no scheduler is + /// involved: the wait is the remainder of the target, and an upstream that + /// already spent more than the target leaves nothing to wait for. + /// + /// Tested here rather than only through the pipeline because the interesting + /// cases are exact numbers, and asserting exact numbers on elapsed wall + /// clock is a claim about the scheduler rather than about this code. + #[tokio::test] + async fn the_latency_padding_is_the_remainder_of_the_target() { + let target = Some(Duration::from_millis(200)); + + // Nothing spent yet: the whole target is waited on. + assert_eq!(pad_to_target(target, Duration::ZERO, "p").await, 200); + // Half spent upstream: half remains. + assert_eq!( + pad_to_target(target, Duration::from_millis(120), "p").await, + 80 + ); + // Spent exactly the target: nothing remains. + assert_eq!( + pad_to_target(target, Duration::from_millis(200), "p").await, + 0 + ); + // Slower than the target: still nothing, and no underflow. Doppel does + // not make a slow backend look fast, so the setting is a floor. + assert_eq!( + pad_to_target(target, Duration::from_millis(900), "p").await, + 0 + ); + // The roll did not fire: no wait at all. + assert_eq!(pad_to_target(None, Duration::ZERO, "p").await, 0); + } + + /// And the same rule through the pipeline, against a real upstream that + /// takes real time. The upstream sleeps 300ms and the target is 200ms, so + /// the remainder is nothing and the total should stay near the upstream's + /// own figure -- not 500ms, which is what adding the delay would give. + /// + /// The upper bound is generous on purpose: it has 200ms of headroom over the + /// upstream's own 300ms, so it fails on the 500ms of an addition and not on + /// a slow machine. + #[tokio::test] + async fn a_slow_upstream_absorbs_the_configured_latency_rather_than_adding_to_it() { + let app = axum::Router::new().fallback(axum::routing::any(|| async { + tokio::time::sleep(Duration::from_millis(300)).await; + "ok" + })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + let text = config_pointing_at( + &format!("http://{addr}/"), + " latency:\n percentage: 1.0\n min: 0.2\n max: 0.2", + ); + let started = Instant::now(); + let response = send(state(&text, vec![0.0, 0.0]), get("/anything")).await; + let elapsed = started.elapsed(); + assert_eq!(response.status(), StatusCode::OK); + assert!( + elapsed >= Duration::from_millis(300), + "the upstream's own 300ms cannot be shortened, took {elapsed:?}" + ); + assert!( + elapsed < Duration::from_millis(500), + "the 200ms latency must be absorbed by the upstream's 300ms, not added: {elapsed:?}" + ); + } + #[tokio::test] async fn unresolvable_request_returns_the_error_envelope() { let text = config_with("").replace( @@ -1026,6 +1209,28 @@ proxies: assert_upstream_fields_absent(&events[0]); } + #[tokio::test] + async fn a_mocks_own_loss_branch_has_the_full_schema_with_upstream_contacted_false() { + let text = config_with( + " mocks:\n - name: m1\n request:\n method: GET\n \ + url: /widgets/\n response:\n status: 200\n body: 'hello'\n \ + proxy:\n loss:\n percentage: 1.0\n status: 503", + ); + // Two draws: the `replace` roll, then the mock's own loss roll. + let (response, events) = run_captured(state(&text, vec![0.0, 0.0]), get("/widgets/")).await; + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(events.len(), 1, "{events:?}"); + assert_full_schema(&events[0]); + assert_upstream_contacted(&events[0], false); + assert_upstream_fields_absent(&events[0]); + assert_eq!( + events[0].recorded_fields.get("mock").map(String::as_str), + Some("\"m1\""), + "the dropped line must name whose loss fired: {:?}", + events[0] + ); + } + #[tokio::test] async fn forward_error_branch_has_the_full_schema_with_upstream_contacted_true() { // Connection to loopback port 1 is refused: a genuine upstream @@ -1318,6 +1523,275 @@ proxies: assert_eq!(response.status(), StatusCode::BAD_GATEWAY); } + /// `replace` is the share of matching requests a mock answers, and + /// nothing about `loss` changes that number. Before the pipeline put + /// the mock first, a proxy dropping everything answered no mock at all, + /// and `loss: 0.5` silently halved every `replace` in the proxy. + /// + /// One sampler draw is supplied: the `replace` roll. If loss were still + /// decided first it would take that draw, drop the request, and this + /// test would see 503. + #[tokio::test] + async fn a_matched_mock_answers_a_request_loss_would_have_dropped() { + let extra = r#" loss: + percentage: 1.0 + status: 503 + mocks: + - name: m1 + request: + method: GET + url: /widgets/ + response: + status: 200 + body: 'hello' +"#; + let response = send(state(&config_with(extra), vec![0.0]), get("/widgets/")).await; + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(body_string(response).await, "hello"); + } + + /// The other half of the ordering: `loss` is not disabled by the + /// presence of a mock, only deferred behind it. A mock that loses its + /// `replace` roll leaves the request on the way to the real backend, + /// where the proxy's faults do apply. + /// + /// `replace: 0` never fires and draws nothing (see `fault::fires`), so + /// the single draw here is the loss roll. + #[tokio::test] + async fn loss_still_drops_a_request_whose_mock_lost_the_replace_roll() { + let extra = r#" replace: 0.0 + loss: + percentage: 1.0 + status: 503 + mocks: + - name: m1 + request: + method: GET + url: /widgets/ + response: + status: 200 + body: 'hello' +"#; + let response = send(state(&config_with(extra), vec![0.0]), get("/widgets/")).await; + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); + } + + /// The proxy's `latency` describes how slow this proxy is to answer, + /// whatever answers, so a mocked response waits for it too -- the mock + /// does not have to repeat it. + /// + /// Three draws: `replace`, the latency roll, and the interpolation + /// between `min` and `max`. The mock declares no `loss`, and `decide` + /// draws nothing for an absent one. + #[tokio::test] + async fn the_proxys_latency_applies_to_a_served_mock_too() { + let extra = r#" latency: + percentage: 1.0 + min: 0.2 + max: 0.2 + mocks: + - name: m1 + request: + method: GET + url: /widgets/ + response: + status: 200 + body: 'hello' +"#; + let started = Instant::now(); + let response = send( + state(&config_with(extra), vec![0.0, 0.0, 0.0]), + get("/widgets/"), + ) + .await; + let elapsed = started.elapsed(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(body_string(response).await, "hello"); + assert!( + elapsed >= Duration::from_millis(200), + "expected the proxy's 200ms latency to apply to the mock, took {elapsed:?}" + ); + } + + /// A mock's own `latency` overrides the proxy's rather than adding to + /// it. The proxy asks for two seconds and the mock for 200ms; the + /// response must arrive on the mock's schedule, so the assertion is an + /// upper bound well below the proxy's figure. + #[tokio::test] + async fn a_mocks_own_latency_overrides_the_proxys_rather_than_adding_to_it() { + let extra = r#" latency: + percentage: 1.0 + min: 2.0 + max: 2.0 + mocks: + - name: m1 + request: + method: GET + url: /widgets/ + response: + status: 200 + body: 'hello' + proxy: + latency: + percentage: 1.0 + min: 0.2 + max: 0.2 +"#; + let started = Instant::now(); + let response = send( + state(&config_with(extra), vec![0.0, 0.0, 0.0]), + get("/widgets/"), + ) + .await; + let elapsed = started.elapsed(); + assert_eq!(response.status(), StatusCode::OK); + assert!( + elapsed >= Duration::from_millis(200), + "the mock's own 200ms should still be waited on, took {elapsed:?}" + ); + assert!( + elapsed < Duration::from_millis(1500), + "the proxy's 2s must not be added to the mock's 200ms, took {elapsed:?}" + ); + } + + /// A request line of `GET //widgets/` is legal, and clients produce it + /// by accident whenever a base URL ending in `/` is joined to a path + /// beginning with one. The mock must still answer -- the dead upstream + /// is the proof it did, since reaching it would give 502. + /// + /// The pattern is anchored deliberately. An unanchored `/widgets/` + /// matches `//widgets/` on its own, because the doubled slash falls + /// outside the substring it looks for, so this test would pass with the + /// normalisation removed and pin nothing. Verified by removing it: with + /// `url: /widgets/` here the test still passed. + #[tokio::test] + async fn repeated_leading_slashes_still_reach_the_mock() { + let extra = r#" mocks: + - name: m1 + request: + method: GET + url: ^/widgets/$ + response: + status: 200 + body: 'hello' +"#; + let response = send(state(&config_with(extra), vec![0.0]), get("//widgets/")).await; + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(body_string(response).await, "hello"); + } + + /// A mock's own `loss` drops requests the mock would have answered. + /// 503 rather than the dead upstream's 502 is what says the mock's loss + /// fired and not that the request fell through to forwarding. + /// + /// Two draws: the `replace` roll, then the mock's loss roll. + #[tokio::test] + async fn a_mocks_own_loss_drops_a_request_it_would_have_answered() { + let extra = r#" mocks: + - name: m1 + request: + method: GET + url: /widgets/ + response: + status: 200 + body: 'hello' + proxy: + loss: + percentage: 1.0 + status: 503 +"#; + let response = send(state(&config_with(extra), vec![0.0, 0.0]), get("/widgets/")).await; + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); + } + + /// And it is scoped to the mock: a path the mock does not match is + /// forwarded, reaching the dead upstream for 502 rather than picking up + /// the mock's 503. No draws at all -- this proxy declares no faults of + /// its own, so an empty sampler proves none were rolled. + #[tokio::test] + async fn a_mocks_own_loss_does_not_touch_a_path_it_does_not_match() { + let extra = r#" mocks: + - name: m1 + request: + method: GET + url: /widgets/ + response: + status: 200 + body: 'hello' + proxy: + loss: + percentage: 1.0 + status: 503 +"#; + let response = send(state(&config_with(extra), vec![]), get("/other/")).await; + assert_eq!(response.status(), StatusCode::BAD_GATEWAY); + } + + /// A mock's own `latency` delays the mock's response. Asserted as a + /// lower bound on elapsed time: an upper bound would be a claim about + /// the scheduler. + /// + /// Three draws: `replace`, the latency roll, and the interpolation + /// between `min` and `max`. + #[tokio::test] + async fn a_mocks_own_latency_delays_its_response() { + let extra = r#" mocks: + - name: m1 + request: + method: GET + url: /widgets/ + response: + status: 200 + body: 'hello' + proxy: + latency: + percentage: 1.0 + min: 0.2 + max: 0.2 +"#; + let started = Instant::now(); + let response = send( + state(&config_with(extra), vec![0.0, 0.0, 0.0]), + get("/widgets/"), + ) + .await; + let elapsed = started.elapsed(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(body_string(response).await, "hello"); + assert!( + elapsed >= Duration::from_millis(200), + "expected the mock's own 200ms latency to be waited on, took {elapsed:?}" + ); + } + + /// `latency` is inherited from the proxy and `loss` is not, and this + /// pins the half that is easy to break by making the two symmetrical. + /// Inheriting `loss` would drop mocked requests again and restore the + /// coupling between `loss` and `replace` that the pipeline order exists + /// to remove. + /// + /// One draw, the `replace` roll. A fallback to the proxy's loss would + /// take a second and answer 503. + #[tokio::test] + async fn a_mock_does_not_inherit_the_proxys_loss_though_it_inherits_latency() { + let extra = r#" loss: + percentage: 1.0 + status: 503 + mocks: + - name: m1 + request: + method: GET + url: /widgets/ + response: + status: 200 + body: 'hello' +"#; + let response = send(state(&config_with(extra), vec![0.0]), get("/widgets/")).await; + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(body_string(response).await, "hello"); + } + #[tokio::test] async fn a_body_extracting_mock_renders_from_the_body() { let extra = r#" mocks: diff --git a/docs/development/index.md b/docs/development/index.md index b182eba..719dbb8 100644 --- a/docs/development/index.md +++ b/docs/development/index.md @@ -90,6 +90,38 @@ licence. `--strict` turns a broken internal link into a build failure. +### How it is published + +The site is versioned with [`mike`](https://github.com/jimporter/mike), which +keeps one built copy per release on the `gh-pages` branch alongside the +`versions.json` that fills the switcher in the header. GitHub Pages serves that +branch; nothing here uses the Pages deployment API. + +`.github/workflows/docs.yml` decides what is published from the ref it ran on: + +| Ref | Published as | +|---|---| +| a push to `main` | the `dev` alias | +| a final tag `v1.2.3` | `1.2.3`, and the `latest` alias moves to it | +| a pre-release tag `v1.2.3-rc.1` | nothing | + +The site root redirects to `latest`, so the bare URL always lands on the newest +release rather than on unreleased documentation. + +A pre-release publishes nothing on purpose. Release candidates exist to exercise +the release pipeline, their documentation is in-progress documentation that +`dev` already carries, and publishing it would only add entries to the switcher +that someone has to delete by hand afterwards. + +Nothing needs doing by hand for a normal release. To rebuild one version -- after +fixing a typo in already-released documentation, say -- check that tag out and +run mike against it: + +```bash +uv run --with-requirements docs/requirements.txt mike deploy --push 1.2.3 +uv run --with-requirements docs/requirements.txt mike list +``` + ## Dependencies Prefer the standard library and the existing set. Every direct dependency must diff --git a/docs/overview/concepts.md b/docs/overview/concepts.md index dc1e90f..aa995ca 100644 --- a/docs/overview/concepts.md +++ b/docs/overview/concepts.md @@ -72,21 +72,44 @@ The order is fixed: client --> doppel --> upstream | | 1. resolve which proxy handles this request - | 2. maybe drop it (loss) - | 3. maybe delay it (latency) - | 4. maybe answer it here (a matching mock, subject to `replace`) - | 5. otherwise forward it + | 2. does a mock match, and does `replace` fire? + | + | yes -> the mock's loss; drop and stop if it fires + | render the mock + | no -> the proxy's loss; drop and stop if it fires + | forward it + | + | 3. wait out whatever is left of the latency ``` -Faults come before mock matching because they belong to the proxy rather than -to a route: a backend that is slow is slow for endpoints you have mocked and -endpoints you have not. A mock replaces the endpoint, so it comes after. - -Step 4 is conditional twice over. A mock has to match, and then `replace` -- +Step 2 is conditional twice over. A mock has to match, and then `replace` -- itself a fraction -- has to fire. `replace: 0.5` sends half of the matching requests to the real upstream and answers the other half locally, which is how a backend is replaced incrementally rather than all at once. +Mock matching comes before the faults, and that ordering is what makes +`replace` mean what it says. Were `loss` decided first, `replace: 0.5` under +`loss: 0.5` would answer a quarter of matching requests from the mock rather +than half, and no configuration could ask for half while any loss was set. So a +request a mock answers is never dropped by the proxy's `loss`; only by the +mock's own, which it does not inherit. + +`latency` is the other way round: it says how slow this proxy is to answer, and +that holds whatever answers, so a mocked response is delayed like any other. A +mock may override the figure but does not add to it. + +Step 3 comes last because the delay is a target for the whole response, not an +addition to it: the time the upstream really took is subtracted, and what +remains is waited out. A 500ms latency in front of a backend answering in 120ms +sleeps 380ms. An upstream slower than the target leaves nothing to wait for -- +Doppel does not make a slow backend look fast. + +A request the loss roll drops stops at step 2 and is never delayed: refusing a +request and then holding the connection open for 200ms would be the worst of +both. + +See [Injecting faults](../usage/faults.md). + ## Two things that are not what they sound like **`percentage` is a fraction.** The field name is older than the decision to diff --git a/docs/requirements.txt b/docs/requirements.txt index 2d43818..18fc419 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -14,3 +14,10 @@ # evaluate. mkdocs>=1.6,<2 mkdocs-material>=9.7,<10 + +# Versioned publication: `mike` keeps one built site per release on the +# `gh-pages` branch and writes the `versions.json` the header switcher reads. +# Only the Docs workflow calls it; `mkdocs build` and `mkdocs serve` do not need +# it, and it is listed here so the workflow has one requirements file to install +# rather than a second list of pins that can drift from this one. +mike>=2.1,<3 diff --git a/docs/usage/faults.md b/docs/usage/faults.md index 2a8d780..ce02490 100644 --- a/docs/usage/faults.md +++ b/docs/usage/faults.md @@ -35,8 +35,34 @@ for i in $(seq 1 20); do done | sort -n | tail -5 ``` -Roughly nine of twenty should sit near the base latency and the rest between -50 and 200 milliseconds above it. +Roughly nine of twenty should sit near the base latency and the rest between 50 +and 200 milliseconds. + +### The delay is a target, not an addition + +The drawn delay is what the whole response should take, and the time the real +upstream already spent comes out of it. A `min: 0.5, max: 0.5` in front of a +backend answering in 120ms waits 380ms, so the client sees about 500ms -- not +620ms. + +This is what makes the configured number mean something: adding to an upstream +whose own latency varies gives a figure nobody chose, and the setting you wrote +would be unreachable by construction. + +!!! note "A floor, not a budget" + An upstream slower than the delay leaves no remainder, and the request is + passed straight through. Doppel never makes a slow backend look fast, so a + 500ms setting in front of a backend taking 900ms produces 900ms and waits + for nothing. + + `latency_injected_ms` in the log line is the wait actually taken, so it + reads `0` in that case even though the roll fired. `duration_ms` is the + total. The `doppel_latency_injected_total` counter, by contrast, counts + every request whose roll fired -- whether or not there was anything left to + wait for. + +A request answered by a mock is delayed on the same rule; see +[Faults on one endpoint only](#faults-on-one-endpoint-only). ## Dropping a share of requests @@ -101,7 +127,32 @@ endpoint, put the fault on a **mock's** `proxy` block instead: ``` The mock's `proxy` block accepts the same three settings and is held to the -same bounds. It applies only to requests that matched this mock. +same bounds. They apply to requests the mock actually answers -- after it has +matched and won its `replace` roll. + +What each one does when the mock leaves it out: + +| Setting | A mock that does not declare it | A mock that does | +|---|---|---| +| `replace` | uses the proxy's | uses its own | +| `latency` | uses the proxy's | uses its own **instead** of the proxy's, never on top | +| `loss` | has none at all | uses its own | + +`latency` is inherited because it describes how slow this proxy is to answer, +and that is true whatever answers -- so a mocked response is delayed like any +other, and the example above makes `/checkout/` slower than the rest rather than +being the only thing that is slow. Overriding replaces the proxy's figure; the +two are not added, or a mock could only ever be slower than its proxy. + +`loss` is the one exception. A mock inheriting it would be dropped by the +proxy's loss, which is exactly the coupling between `loss` and `replace` that +[the ordering](#loss-does-not-eat-into-replace) exists to remove. So a mock that +should be flaky has to say so itself. + +!!! note "A dropped request is not delayed first" + Within either set, `loss` is decided before `latency` and short-circuits it. + A request the mock's own loss drops does not wait for the mock's latency + first, and a request the proxy's loss drops does not wait for the proxy's. ## Replacing a backend gradually @@ -132,6 +183,37 @@ the proxy (as above) or per mock inside its `proxy` block. is still involved at all. A `replace: 0` mock is dead configuration, not a disabled fault. +### `loss` does not eat into `replace` + +A mock is decided before either fault, so `replace` is the share of *matching* +requests the mock answers, whatever `loss` is set to: + +```yaml + loss: + percentage: 0.5 + status: 503 + replace: 0.5 + mocks: + - name: new-pricing + request: + method: GET + url: /pricing/ + response: + status: 200 + json: '{"price": 100}' +``` + +Half of `GET /pricing/` requests get the mock -- not a quarter. The other half +go on to the loss roll, so about a quarter are dropped with `503` and about a +quarter reach the real service. Requests to any other path are unaffected by +`replace` and take the loss roll as usual. + +The mock's half is not touched by the proxy's `loss` -- that is the whole point +of deciding the mock first. It *is* delayed by the proxy's `latency`, which +applies to every answer this proxy gives. See +[Faults on one endpoint only](#faults-on-one-endpoint-only) for the table of +what a mock inherits and what it does not. + ## The bounds, and why they exist | Field | Range | Refused beyond it because | diff --git a/docs/usage/mocks.md b/docs/usage/mocks.md index 1f4873e..c53b2a0 100644 --- a/docs/usage/mocks.md +++ b/docs/usage/mocks.md @@ -28,6 +28,23 @@ what already-written configurations mean, and a change that alters behaviour without the configuration changing is the worst kind -- there is no error and nothing to review. +### Repeated leading slashes + +A run of slashes at the start of the path is collapsed to one before matching, +so `GET //api/v1/index/` is matched as `/api/v1/index/`. + +Clients produce the doubled form constantly -- a base URL ending in `/` joined +to a path beginning with `/` is the usual way -- and it is legal HTTP, so +nothing rejects it. An unanchored pattern happened to match it anyway, because +the extra slash falls outside the substring being looked for. An anchored one +(`^/api/v1/index/$`) did not, and the only symptom was the mock silently not +firing. + +Only the leading run. `/a//b/` keeps its empty middle segment and does not +match a pattern written `/a/b/`: whether those name the same resource is the +upstream's business, and answering it here would also disagree with the path +that gets forwarded. + ## Variables Four sources, all optional. diff --git a/mkdocs.yml b/mkdocs.yml index a0f544c..38e91f0 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -66,6 +66,14 @@ nav: extra_css: - stylesheets/theme.css +# The version switcher in the header is populated from mike's `versions.json` on +# the deployed `gh-pages` branch, not from anything in this file -- so it is +# empty until the Docs workflow has published at least one version. See +# .github/workflows/docs.yml. +extra: + version: + provider: mike + markdown_extensions: - admonition - pymdownx.details