Skip to content

Add direct Tinybird auction telemetry#818

Open
ChristianPavilonis wants to merge 5 commits into
server-side-ad-templates-implfrom
auction-telemetry-direct-tinybird
Open

Add direct Tinybird auction telemetry#818
ChristianPavilonis wants to merge 5 commits into
server-side-ad-templates-implfrom
auction-telemetry-direct-tinybird

Conversation

@ChristianPavilonis

Copy link
Copy Markdown
Collaborator

Summary

  • Adds direct, best-effort Tinybird Events API ingestion for auction telemetry from Fastly Compute.
  • Introduces privacy-preserving core auction telemetry rows plus a Fastly fire-and-forget sink using Secret Store APPEND tokens.
  • Adds Tinybird datasources, rollups, endpoint pipes, fixtures, and YAML test contracts for Grafana-facing telemetry.

Changes

File Change
crates/trusted-server-core/src/auction/telemetry.rs Adds auction observation context, row model, NDJSON batching, privacy-preserving page path normalization, terminal outcomes, no-op sink, and best-effort emit helper.
crates/trusted-server-core/src/auction/endpoints.rs Emits skipped/completed/execution-failed telemetry for POST /auction without changing response behavior.
crates/trusted-server-core/src/publisher.rs Emits completed/skipped/dispatch-failed/abandoned telemetry for initial-navigation SSAT and SPA /__ts/page-bids paths.
crates/trusted-server-core/src/auction/orchestrator.rs Tracks split dispatch outcomes, retained launch failures, timeout rows, and explicit abandonment metadata.
crates/trusted-server-core/src/platform/types.rs Adds AuctionTelemetrySink plumbing to RuntimeServices.
crates/trusted-server-core/src/settings.rs, trusted-server.toml, fastly.toml Adds disabled-by-default Tinybird config, validation, and fake local Secret Store placeholders.
crates/trusted-server-adapter-fastly/src/tinybird.rs Adds Fastly Tinybird sink that builds NDJSON Events API POSTs, calls send_async, and drops pending responses.
crates/trusted-server-adapter-fastly/src/app.rs, src/main.rs, src/platform.rs Wires the configured telemetry sink through EdgeZero and legacy runtime services.
tinybird/** Adds raw/rollup datasources, materialized and endpoint pipes, fixture NDJSON, and YAML endpoint test contracts.
docs/superpowers/** Adds revised spec and implementation plan for direct Tinybird ingestion.

Closes

Closes #817

Test plan

  • cargo test --workspace
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo fmt --all -- --check
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run
  • JS format: cd crates/trusted-server-js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve
  • Other: cd tinybird && TB_CLI_TELEMETRY_OPTOUT=1 uvx --from tinybird-cli tb check
  • Other: YAML test contract parse check with python3/yaml
  • Other: git diff --check

Note: tb test run -v was attempted, but the available Tinybird CLI exited opaquely with Error: ** before executing scenarios; tb check validates the datafiles and YAML test contracts parse successfully.

Checklist

  • Changes follow CLAUDE.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses tracing macros (not println!)
  • New code has tests
  • No secrets or credentials committed

@aram356 aram356 marked this pull request as draft June 26, 2026 15:36
@ChristianPavilonis ChristianPavilonis linked an issue Jun 29, 2026 that may be closed by this pull request
@ChristianPavilonis ChristianPavilonis marked this pull request as ready for review June 29, 2026 15:20
@aram356 aram356 requested review from aram356 and prk-Jr and removed request for aram356 June 29, 2026 15:48

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • /auction ext now lists timed-out providers — see inline at crates/trusted-server-core/src/auction/orchestrator.rs:653
  • between_bytes_timeout reuses the first-byte constant — see inline at crates/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.rs and publisher.rs, build_auction_events(...) runs unconditionally and the resulting batch is handed to emit_auction_events_best_effort. Only the network send is gated by the sink type — so with tinybird.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 cloning publisher_domain / page_path / country / …) purely for NoopAuctionTelemetrySink to drop it. Consider gating the build behind a cheap enablement check — e.g. add AuctionTelemetrySink::is_enabled() (Noop → false) and skip build_auction_events when it returns false, or have emit_*_best_effort take a FnOnce() -> AuctionEventBatch and 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.rs adds access_enabled / access_sample_rate / access_dataset / access_token_secret with validation, and tinybird/datasources/access_logs_raw.datasource exists, but no Rust path emits access telemetry (grep finds access_* only in settings + a test fixture). Setting access_enabled = true passes prepare_runtime validation 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: PASS
  • integration tests (EdgeZero entry point): PASS
  • browser integration tests: PASS
  • prepare integration artifacts: PASS
  • cargo fmt / clippy / cargo test, vitest, JS/docs format: not run remotely on this PR (it targets the feature branch server-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 toward main.

}
}

for (provider_name, start_time, _) in backend_to_provider.into_values() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 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,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpickbetween_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(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌱 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement direct Tinybird auction telemetry ingestion

2 participants