Add direct Tinybird auction telemetry#818
Conversation
…nto auction-telemetry-direct-tinybird
aram356
left a comment
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.
| } | ||
| } | ||
|
|
||
| for (provider_name, start_time, _) in backend_to_provider.into_values() { |
There was a problem hiding this comment.
🤔 thinking — This new block surfaces providers still in-flight at the deadline as provider_timeout_responses, which land in OrchestrationResult.provider_responses. That vector is consumed by formats.rs (convert_to_openrtb_response) for ext.orchestrator.providers (the count) and provider_details[], so the public /auction response now lists timed-out providers where it previously dropped them silently. Winning-bid selection is unaffected (select_winning_bids skips non-Success), so this reads as a diagnostic improvement — but it contradicts the PR description's "without changing response behavior" note for the endpoints. Can you confirm it's intended and update the description? (The same push was added to the streaming collect_dispatched_auction path.)
| host_header_override: None, | ||
| certificate_check: true, | ||
| first_byte_timeout: TINYBIRD_FIRST_BYTE_TIMEOUT, | ||
| between_bytes_timeout: TINYBIRD_FIRST_BYTE_TIMEOUT, |
There was a problem hiding this comment.
⛏ nitpick — between_bytes_timeout reuses TINYBIRD_FIRST_BYTE_TIMEOUT. It works (both are 2s), but reads as accidental. A dedicated TINYBIRD_BETWEEN_BYTES_TIMEOUT const would document that the equal value is intentional rather than a copy-paste.
| batch.to_ndjson(self.target.max_body_bytes) | ||
| } | ||
|
|
||
| fn load_append_token( |
There was a problem hiding this comment.
🌱 seedling — On the enabled path, load_append_token does a synchronous Secret Store host call on every emit (once per auction) before the fire-and-forget send. Append tokens rarely rotate; caching the token on the sink (or memoizing it) would remove a per-request host call from the enabled hot path. Future optimization — not needed for this PR.
Summary
Changes
crates/trusted-server-core/src/auction/telemetry.rscrates/trusted-server-core/src/auction/endpoints.rsPOST /auctionwithout changing response behavior.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,trusted-server.toml,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 test --workspacecargo clippy --workspace --all-targets --all-features -- -D warningscargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd 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!)