Repository navigation
November release candidate (DO NOT MERGE) - #1490
Draft
ChristianPavilonis wants to merge 229 commits into
Draft
ChristianPavilonis wants to merge 229 commits into
ChristianPavilonis wants to merge 229 commits into
Conversation
* Add request phase timing design spec (Server-Timing subtimings + access telemetry) * Address review round 1: freeze point, template-cache naming, snapshot semantics, KV scope, geo carry, route template, sink confirmation, sampling and query model, config rollback * Address review round 2: auction-wait placement modes, conservative private-only header emission, non-null sorting key with service identity, coarse publisher route template, telemetry snapshot and outage behavior, tinybird flag decoupling, adapter phase semantics * Add request phase timing implementation plan * Address engineer review: KV timing decorator, try_lock sampling, route metadata extension, adapter-derived env, typed template-cache state, adapter-owned emission context, per-mode delivery semantics, Axum outer wrapper
…ite-back in middleware Three final-review fixes for access telemetry correctness: - Normalize the HTTP method to an allowlist (GET/HEAD/POST/PUT/DELETE/ PATCH/OPTIONS, else "other") inside access_event_row, so a client- controlled extension-method token can never inflate the LowCardinality method column, regardless of which adapter builds the row. - Guard emit_access_telemetry_after_send against snapshots carrying a degraded sample_rate of 0.0 (captured on the app-state-build-failure fallback path), which could otherwise be sampled in by freshly reloaded settings and corrupt the sum(1.0/sample_rate) volume estimator. - Mirror the geo lookup write-back from apply_entry_point_finalize_headers into FinalizeResponseMiddleware::handle, so a middleware-finalized response that resolved geo via fallback carries the resolved GeoLookupState for the access-telemetry snapshot instead of showing country "unknown".
std::time::Instant::now() panics on wasm32-unknown-unknown, so every publisher request on the Cloudflare adapter trapped when the timing collector was constructed, and the two auction-wait sites would trap once an auction dispatched. web_time re-exports std's Instant on every other target, so Fastly, Axum, and Spin behavior is unchanged. The publisher.rs sites are qualified locally because that module's std Instant import still serves the pre-existing template-cache sites, which are out of scope here.
The character allowlist alone does not bound identity: [a-z0-9_-] is exactly the alphabet UUIDs, hex ids, reset tokens, and article slugs are built from, and truncating to 32 characters still leaves a globally unique prefix. A first segment now rejects whole to /other/* when it exceeds 32 characters or carries more than 7 ASCII digits, alongside the existing charset rejection. Year archives and hyphenated section names still pass. Extends the adversarial tests to the publisher-fallback path with UUID, hex-id, token, and slug shapes, and fixes the stale event_date reference in the row-builder doc.
- Gate building the access snapshot on tinybird.enabled and access_enabled, threaded through SendContext: a disabled deployment (the default) no longer pays env reads and String allocations on the pre-send path. DeliveryOutcome.snapshot becomes Option and the emitter treats None as nothing to send. - Classify asset-fallback responses as route_class asset with the operator-configured route prefix as the template, instead of landing in the other/unknown bucket alongside 404s. - Pin Phase::index() to PHASE_COUNT with a uniqueness-and-bounds test so a future variant fails the suite instead of panicking at runtime. - Drop the tautological sampled-out emission test; the 0.0-rate behavior is covered by sampled_in_boundary_rates_are_unconditional. - Clarify that the local dev config env var name genuinely triples trusted_server_config (prefix, store, key) rather than reading as a find/replace mistake.
Adds section 18 to the request phase timing spec: three first-call-wins T0 offsets (auction dispatched, resolved, committed) on RequestTimings, emitted as additive nullable columns on access_logs_raw with auction_id as the join key to the per-bidder auction dataset. Answers the overlap-proof questions the two existing clocks cannot: when the auction started relative to request entry, when the final bid landed, and when targeting was committed toward GAM.
Implements spec section 18: three first-call-wins marks on RequestTimings (dispatched at the DispatchAuctionOutcome::Dispatched arm, resolved after collect at both sites, committed after write_bids_to_state at both sites), carried through TimingSnapshot into four additive access_logs_raw columns: auction_dispatched_ms, auction_resolved_ms, auction_committed_ms, and auction_id as the join key to the per-bidder auction dataset. Null offsets mean no auction ran; a failed dispatch records nothing. FORWARD_QUERY fills the new columns with typed defaults for pre-existing rows. No header emission, no config surface, no adapter changes: the values ride the existing snapshot and the tinybird.access_enabled gate.
The Cloudflare integration harness writes wrangler.integration.generated.toml at test time; it was swept into the previous commit by accident. Ignore it so local CI=1 runs cannot commit it again.
The first path segment is only a section name when the path has depth: under a /%postname%/ permalink structure every article is a single-segment path, so keeping those segments verbatim put full article slugs into the 30-day dataset, against spec section 9. Depth is now required for a named template; single-segment paths, root landing pages included, bucket to /other/*. Route slicing keeps route_class and multi-segment templates like /news/*.
The bucket-quantized sampler truncated rates below one in a million to a zero threshold (silently emitting nothing) and quantized other low rates downward while rows still carried the configured rate, biasing the sum(1.0 / sample_rate) volume estimator. Its no-rand premise was also wrong: rand::thread_rng() is WASI-backed on this target and the EC generation path already relies on it. The sampler is now a direct uniform-roll comparison, and the roll gates on the rate stored in the snapshot itself, so emission probability and the row's sample_rate column cannot diverge; the divergence guard and its tests are removed. Also per review: the settings-reload fallback in the post-send path could never emit (no snapshot exists when settings were absent) and is removed; the dead_code allow on DeliveryOutcome narrows to the one collected-but-unemitted field; and the post-send ordering test is narrowed to the leg it actually proves, that request_elapsed is stamped when send returns.
On origin failure with a dispatched auction, the origin span guard stayed alive through the emit_abandoned_auction await, so ts-origin and origin_ms absorbed Tinybird emission time. The span now closes when the send resolves, before either branch, with an error-path regression test.
- Pin HEADER_PHASES against Phase::header_name() in the phase-index test, closing the second hand-synced list. - Add RouteClass::Asset to the snake_case rendering test; rename the lowercasing test to say what it does. - Give the Axum adapter a named, fully configured construction path (TrustedServerApp::dev_server_service) so server_timing_enabled is never silently discarded; the tuple API is private now. - Document that Server-Timing is client-visible when enabled, in the configuration guide's observability section. - Replace stale event_date references in the spec, plan, and dashboard guidance with the toDate(event_ts) sorting-key expression, and state the single-segment rejection rule in spec section 9.
…ming # Conflicts: # crates/trusted-server-adapter-fastly/src/app.rs # crates/trusted-server-adapter-fastly/src/main.rs # crates/trusted-server-adapter-fastly/src/middleware.rs # crates/trusted-server-core/src/publisher.rs # crates/trusted-server-core/src/settings.rs # docs/guide/configuration.md # trusted-server.example.toml
The access emitter carried the configured body limit without enforcing it, allowing oversized rows to bypass the intended transport safeguard.
…iter A removal whose row writes all failed was lost, and a positive capture on a request that was not the writer never reached KV beyond the one UID browser ingest picked. Both now go to pending:<ec_id> (one-day TTL, CAS append, at most 16 items). Every returning writer reads that key, merges the items above the row's new pg watermark before its own mutation, and stores the newest generation. A positive capture keeps the revision it read, so it never overwrites a record that changed since, and removed UIDs stay blocked by their ps.iv and ps.ov hashes. removal_deferred counts what went to the key; removal_lost now counts only what could not reach it. Auctions never read it. Spec 15 questions 2 and 4 record the decision. The cost is one more KV read per writer navigation. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
A visitor who stayed in a single-page app waited for a full navigation before browser ingest or provider refreshes reached KV, because /_ts/page-bids never wrote. A /_ts/page-bids call from a real browser with Sec-Fetch-Site same-origin is now the writer, at most once per ec.identity.spa_writer_interval_sec (default 300, 0 = off) since the row's new wa field, the time of its last writer flush. It writes only on a row its auction read, so it adds no row read, and runs browser ingest and after-send providers only, never hold. Spec 15 question 1 records the decision. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
A visitor who left after one page never got a provider ID, because no provider ran until a returning visitor's navigation. With a hold-mode provider that does not read the EC ID configured, the Fastly publisher path now mints the EC without writing. A navigation that runs an auction dispatches those providers on the row the create will write, skipping any that read the EC ID, then creates the row with their answers before the preload and the auction, so the create stays the page load's one write and no ID reaches a bidder before its row exists. A collision fails that create rather than minting an ID the auction may already hold. Finalize creates a row its navigation never reached. Without such a provider the create runs before routing, as before. Spec 15 question 6 records the decision. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
ConsentPolicy::strict() required Purposes 1 and 4 and no vendor, so a resolver that forgot with_tcf_vendor sent visitor data on purpose consent alone, and the fixed purposes matched no vendor's GVL entry. strict() now requires Purpose 1 only. ProviderDescriptorBuilder::build rejects a resolver whose policy declares no purposes with with_tcf_purposes, or names no vendor without the documented without_tcf_vendor relaxation. Capture-only providers need neither, and generic pull keeps its parity policy. The global-revoke marker now fingerprints all ten TCF purposes so it lifts on any purpose change. Test fixtures keep today's Purposes 1 and 4 through test_support::resolver_policy. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
… exception The decisions log said decisions were made with "the user" and left the consent-withdrawal exception unconfirmed. Both now name the spec author, who confirmed the exception after the round 14 review. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…currency # Conflicts: # AGENTS.md
Move the yq install and concurrency guard steps into format-rust so a required check enforces them, and pin the yq download by SHA-256. The guard now accepts expression timeouts, requires every matrix dimension in a matrix job's group, and drops an unreachable reusable-job skip. Add tests for the previously uncovered checks.
Resolve changelog merge conflict while preserving SDK-matching documentation and all base-branch changes.
Resolve merge conflicts while preserving SDK URL ownership matching and base-branch Lockr routing; update corpus expectations for absolute first-party URLs.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This branch has not been deployed
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
Starts fresh from
main, not from the October RC. Merges the five remaining open PRs agreed for this RC, one at a time, preserving each audited head in the branch history.Release scope
Base main:
51730bdaf841.1f945068ab609654deb7ac4a88cc505daecc08eb99c53afdd79392a49d3aThe source descriptions for #1428 and #1426 lag their audited heads. In particular, #1428 is no longer docs-only. The table describes the actual included revisions.
#1261 replaces the closed #1074, #1076, #1121 and #1154 timing/diagnostics stack. Closed #1260 is not merged separately; the current #1428 implementation supplies the identity work in this RC. Source PRs retain their own review and merge processes.
Integration decisions
511ce06ff3df59996641af2ea64cbc54cdac7ea5, from EdgeZero #389. This candidate contains released main's lifecycle and pre-dispatch ingress APIs plus shared timing. Cargo resolves all eight EdgeZero packages to this single Git identity. Replace the temporary revisions with an approved release tag and revalidate before merging this RC.Validation
Local checks pass with Rust 1.98.1, Node 24.20.0, Viceroy 0.21.0, Spin 4.1.0, Wrangler 4.151.3 and wasm-bindgen 0.2.126.
The URL-host allowlist is unchanged. Narrow, explained per-line exceptions retain shared legacy test hosts, interpolated reserved-domain fixture URLs, a negative URL parser case, the default Tinybird endpoint assertion and a one-off cited source link. That URL-host follow-up changed comments only; native core and Fastly adapter tests, Fastly Clippy, formatting and docs checks pass again afterward. Other passing checks are reused with unchanged relevant runtime inputs.
All five audited source heads and the base main commit are ancestors of this RC. The working tree is clean. The table above records pre-publication local checks, not production acceptance.
CI follow-up
All 30 GitHub checks pass at
595dffea0, including CodeQL, Rust and JavaScript tests, browser integration, all trace-runtime acceptance tests, worker deadlines, HTML EOF, formatting and docs.Release and rollout gates
__Host-ts-eids-sclient mirror is default-off. Provider consent policies, writer/stager behavior, pending removals, new-visitor hold behavior and provider-specific acceptance still need operator review.