Add direct Tinybird auction telemetry#818
Open
ChristianPavilonis wants to merge 5 commits into
Open
Conversation
aram356
approved these changes
Jul 1, 2026
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Adds disabled-by-default, best-effort Tinybird auction telemetry from Fastly Compute: a privacy-preserving core row model plus sink trait, a Fastly fire-and-forget Events API sink, and Tinybird datasources/pipes/fixtures. The terminal-event accounting is careful (verified exactly one terminal event per auction across every dispatch-failed / skipped / abandoned / completed path, including EOF-without-</body>), the change is best-effort and isolated from customer traffic, and it ships with real privacy-regression tests. Approving — the comments below are non-blocking quality / perf / docs notes. One asks the author to confirm an intended change to the /auction response metadata.
Non-blocking
🤔 thinking · ⛏ nitpick · 🌱 seedling
/auctionext now lists timed-out providers — see inline atcrates/trusted-server-core/src/auction/orchestrator.rs:653between_bytes_timeoutreuses the first-byte constant — see inline atcrates/trusted-server-adapter-fastly/src/tinybird.rs:209- Append token re-fetched from the Secret Store on every emit — see inline at
crates/trusted-server-adapter-fastly/src/tinybird.rs:97
Cross-cutting / body-level findings
- ♻️ Telemetry rows are built even when telemetry is disabled (the default path). At every call site in
endpoints.rsandpublisher.rs,build_auction_events(...)runs unconditionally and the resulting batch is handed toemit_auction_events_best_effort. Only the network send is gated by the sink type — so withtinybird.enabled = false(today's default for every publisher), each server-side auction still allocates the full row set (summary + one row per provider + one row per bid, each cloningpublisher_domain/page_path/country/ …) purely forNoopAuctionTelemetrySinkto drop it. Consider gating the build behind a cheap enablement check — e.g. addAuctionTelemetrySink::is_enabled()(Noop →false) and skipbuild_auction_eventswhen it returns false, or haveemit_*_best_efforttake aFnOnce() -> AuctionEventBatchand build lazily after the enabled check. Apply manually — spans many call sites across two files, so it can't be a single-file suggestion. - 🌱 Access-log telemetry is config + schema only, with no emitter.
settings.rsaddsaccess_enabled/access_sample_rate/access_dataset/access_token_secretwith validation, andtinybird/datasources/access_logs_raw.datasourceexists, but no Rust path emits access telemetry (grep findsaccess_*only in settings + a test fixture). Settingaccess_enabled = truepassesprepare_runtimevalidation yet emits nothing. The plan doc frames this as an intended future phase, so it's forward-looking by design — but as shipped it can mislead an operator who enables it. Recommend a short "not yet wired" doc/comment on the access fields, or deferring the access-only config to the PR that implements the emitter.
CI Status
integration tests: PASSintegration tests (EdgeZero entry point): PASSbrowser integration tests: PASSprepare integration artifacts: PASScargo fmt/clippy/cargo test,vitest, JS/docs format: not run remotely on this PR (it targets the feature branchserver-side-ad-templates-impl); the PR body attests they pass locally. No JS files changed, so the unchecked JS boxes are N/A. Worth ensuring these gates run before the feature branch merges towardmain.
bffa375 to
3b9a0c1
Compare
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
Changes
crates/trusted-server-core/src/auction/telemetry.rscrates/trusted-server-core/src/auction/endpoints.rsPOST /auction; bid selection is unchanged. Timed-out providers are intentionally surfaced in response diagnostics (ext.orchestrator) instead of being dropped silently.crates/trusted-server-core/src/publisher.rs/__ts/page-bidspaths.crates/trusted-server-core/src/auction/orchestrator.rscrates/trusted-server-core/src/platform/types.rsAuctionTelemetrySinkplumbing toRuntimeServices.crates/trusted-server-core/src/settings.rs,fastly.tomlcrates/trusted-server-adapter-fastly/src/tinybird.rssend_async, and drops pending responses.crates/trusted-server-adapter-fastly/src/app.rs,src/main.rs,src/platform.rstinybird/**docs/superpowers/**Closes
Closes #817
Test plan
cargo fmt --all -- --checkcargo test-fastlycargo clippy-fastlycargo check-fastlycargo check-axum && cargo check-cloudflare && cargo check-spincd crates/trusted-server-js/lib && npx vitest run(no JS changed)cd crates/trusted-server-js/lib && npm run format(no JS changed)cd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute servecd tinybird && TB_CLI_TELEMETRY_OPTOUT=1 uvx --from tinybird-cli tb checkpython3/yamlgit diff --checkNote:
tb test run -vwas attempted, but the available Tinybird CLI exited opaquely withError: **before executing scenarios;tb checkvalidates the datafiles and YAML test contracts parse successfully.Checklist
unwrap()in production code — useexpect("should ...")tracingmacros (notprintln!)