Repository navigation
feat(telemetry): add anonymous opt-out daily telemetry ping (#1126) - #1300
Merged
Merged
Conversation
Adds the panel-side half of the telemetry contract for v2.0.0: - New Sbpp\Telemetry\Telemetry class with schema-1 payload + atomic daily-tick scheduling (no cron; register_shutdown_function + fastcgi_finish_request hand-off so user requests never wait on the network call). - Vendored web/includes/Telemetry/schema-1.lock.json mirroring the cf-analytics canonical schema. Two parity tests gate both directions (extractor coverage + README field-list drift). - New telemetry.enabled / .last_ping / .instance_id / .endpoint settings rows in install/includes/sql/data.sql + paired updater migration 807.php. - Features-tab toggle (Admin -> Settings -> Features -> Telemetry) with help-icon disclosure copy and audit-log entry on enable / disable transitions. Opt-out clears instance_id so re-enable mints a fresh one the Worker can't link to the previous state. - README ## Privacy & telemetry section (with the <!-- TELEMETRY-FIELDS-START / END --> markers the parity test consumes), ARCHITECTURE.md Telemetry subsystem section + Where- to-find-what rows, UPGRADING.md Telemetry section, AGENTS.md Cross-repo JSON contracts convention, CHANGELOG.md Privacy heading, docker/README.md README mount note. - make sync-telemetry-schema target for manual schema syncs. The Cloudflare Worker that receives these pings lives in sbpp/cf-analytics; that repo's separate. Closes #1126. Co-authored-by: Cursor <cursoragent@cursor.com>
The actual filesystem path under web/includes/ is `Telemetry/` (uppercase, matching the `Sbpp\Telemetry\` namespace + the PSR-4 mapping). The docs and a handful of docblocks copied the issue body's lowercase `web/includes/telemetry/...` shape, which doesn't resolve on case-sensitive filesystems (Linux — i.e., production hosting). Anyone copy-pasting these paths hits "no such file or directory". Code paths are unaffected — `Schema1::LOCK_FILE` uses `__DIR__ . '/schema-1.lock.json'` which resolves correctly at runtime, and `TelemetryReadmeParityTest` uses `realpath()`. This is a docs-only fix. Co-authored-by: Cursor <cursoragent@cursor.com>
3 tasks done
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
Implements the panel-side half of #1126 — anonymous opt-out telemetry pinged daily to a Cloudflare Worker so maintainers can see what versions, environments, and feature toggles are actually in real-world use.
telemetry.enabledto0AND clearstelemetry.instance_idso a re-enable mints a fresh per-install ID the Worker can't link to the previous one. Enable / disable transitions are audit-logged once; pings themselves are never logged.register_shutdown_function+fastcgi_finish_requestso the user's TCP socket closes BEFORE the cURL POST. Atomiclast_pingreservation (UPDATE … WHERE CAST(value AS UNSIGNED) <= :threshold+rowCount === 1) prevents thundering-herd from concurrent requests; slot is reserved at the START of the attempt, so a flapping endpoint costs one ping/day, not one ping/request.schema,instance_id,panel.*,env.*,scale.*,features.*) match the canonicalsbpp/cf-analyticsschema/1.lock.json. Two parity tests gate the contract in BOTH directions:TelemetrySchemaParityTest— extractor coverage vs. lock-file leaf set.TelemetryReadmeParityTest— README's## Privacy & telemetryfield list (between<!-- TELEMETRY-FIELDS-START -->/<!-- TELEMETRY-FIELDS-END -->markers) vs. the same.TelemetryCollectTest::testCollectedPayloadContainsNoSeededPii— the test seeds canary admin names, SteamIDs, ban reasons, server hostnames, mute reasons, then asserts NONE of those literal strings appear in the JSON-serialised payload. Regression guard against the issue's "anonymous by design, not anonymous-if-you-trust-us" rule.Sbpp\Telemetrynamespace underweb/includes/Telemetry/.final class Telemetry+final class Schema1, PHPStan level 5 + dba clean, native types throughout.make sync-telemetry-schemaMakefile target for manual schema syncs from the cf-analytics companion repo. No scheduled auto-PR workflow — the parity tests gate the result and a maintainer invokes the make target when picking up cf-analytics changes.Closes #1126.
Soft-unblocks the milestone:
Files
New:
web/includes/Telemetry/Telemetry.php—final class Telemetry(tickIfDue,collect,send, plus the private cooldown / reservation / flush helpers).web/includes/Telemetry/Schema1.php—final class Schema1(payloadFieldNames(): list<string>over the lock file). Single source of truth for the parity tests.web/includes/Telemetry/schema-1.lock.json— Draft-7 JSON Schema, vendored byte-for-byte from cf-analytics.web/updater/data/807.php— idempotentINSERT IGNOREfor the four telemetry settings rows.web/tests/integration/TelemetryCollectTest.php— payload shape + counts + zero-PII regression guard.web/tests/integration/TelemetryOptOutTest.php— opt-out short-circuits, slot reserved even when endpoint unreachable, no-op inside cooldown.web/tests/integration/TelemetrySchemaParityTest.php— extractor ↔ lock file parity (both directions).web/tests/integration/TelemetryReadmeParityTest.php— README ↔ lock file parity.Makefile—sync-telemetry-schematarget.UPGRADING.md— Telemetry section (file is new for this PR; Author UPGRADING.md for 1.x to 2.0 #1115's territory but no version landed yet).Modified:
web/init.php—register_shutdown_function([Telemetry::class, 'tickIfDue'])at the tail.web/install/includes/sql/data.sql— fourtelemetry.*rows.web/updater/store.json— register807.php.web/pages/admin.settings.php— Features-tab POST handler: enable/disable transition log, opt-out clears instance_id, View DTO carries the new flag.web/themes/default/page_admin_settings_features.tpl— Privacy card with the toggle + help paragraph.web/includes/View/AdminFeaturesView.php—telemetry_enabledproperty.README.md—## Privacy & telemetrysection.ARCHITECTURE.md— Telemetry subsystem section + Directory layout.AGENTS.md— Cross-repo JSON contracts convention + Where-to-find-what rows.CHANGELOG.md—### Privacyheading under 2.0.0.docker-compose.yml+docker/README.md— bind-mountREADME.mdinto the container so the parity test can reach it fromweb/../README.mdlocally (CI gets it for free viaactions/checkout@v4).Resolved ambiguities (judgement calls)
The reviewer should look closely at these — each is a one-specific-way pick on something the issue body left open:
web/updater/data/804.php; that script +805.php+806.phpalready exist onmain. Used the next free integer (807.php) and registered it as"807": "807.php"instore.json. Numbers are historical / sequence not semantic, per the AGENTS.md guidance.scale.bans_activeSQL. Issue body left this open ("decide and document"). PickedWHERE (ends > UNIX_TIMESTAMP() OR length = 0) AND RemoveType IS NULL. Mirrorspage.banlist.php's active-ban definition (a permanent ban withlength = 0is active even thoughends = 0; a removed ban is excluded regardless ofends). Documented in the schema, the README, and the test seeds rows on either side of every boundary.scale.comms_activeSQL. Same shape asbans_activeagainst:prefix_comms. The schema is identical (ends,length,RemoveType).features.smtp_configuredpredicate. The issue suggestedConfig::get('config.mailtype') !== 'phpmail' && !empty(Config::get('config.smtphost'))butconfig.mailtypeis not indata.sql. Switched totrim((string) Config::get('smtp.host')) !== ''—:prefix_settings.smtp.hostIS the panel-controlled SMTP indicator the Settings → Main form drives, so a non-empty value is the load-bearing "SMTP is wired up" signal. Host value itself never leaves the panel.features.geoip_presentdetection. The panel resolvesMMDB_PATH(=web/data/GeoLite2-Country.mmdb) at bootstrap;system-functions.php'scountry()opens that exact file. Useddefined('MMDB_PATH') && @is_file(MMDB_PATH) && @is_readable(MMDB_PATH). Nogeoip2extension to probe — the panel is filesystem-only.panel.themeenum. Schema enum is["default", "custom"]exactly as the issue body specified. The runtime check enumeratesweb/themes/<name>/theme.conf.phpand intersects with the activeSB_THEME; only the literal string'default'(the shipped theme) is allowed through, every fork reports'custom'. The actual fork directory name is never reported.page_admin_settings_features.tplthat:README.md's## Privacy & telemetrysection for the field-by-field list and the SQL behind eachscale.*count.web/themes/default/page_admin_settings_features.tplunder thePrivacycard.Log::add(LogType::Message, 'Telemetry', 'Telemetry ' . ($verb) . ' by ' . $user)—LogType::Message(notWarning/Error) because opting out / in is a routine state change, not a problem. Topic isTelemetryso the audit log filters cleanly. Pings themselves are never logged.instance_idlifecycle. Lazy:Telemetry::collect()mintsbin2hex(random_bytes(16))on the first call after the row is empty / wrong-shape, persists it to:prefix_settings, and memoizes within the request so a secondcollect()call (ortickIfDue's subsequentcollect) returns the same ID. The opt-out path (Settings → Features) wipes the row to''so the nextcollect()mints fresh.flushResponseToClient()short-circuits onPHP_SAPI === 'cli'/'phpdbg'— closing PHPUnit's output buffers viaob_end_flush()would break the test reporter (PHPUnit 11 marks tests Risky for "test code or tested code closed output buffers other than its own"). Production paths (Apache mod_php, FPM) are unaffected../webto/var/www/html/web, so a naïveweb/../README.mdlookup fails locally. Added a paired bind mount indocker-compose.yml(./README.md:/var/www/html/README.md:ro) so the test reaches the file atweb/../README.mdwhether it's running under CI or locally. CI gets the README viaactions/checkout@v4automatically.cf-analyticsrepo URL. The issue references it assbpp/cf-analyticswithout a more specific URL. Usedhttps://raw.githubusercontent.com/sbpp/cf-analytics/main/schema/1.lock.jsonfor themake sync-telemetry-schematarget and the\$idin the schema lock file. Self-hosters can repoint to a different fork by editing the Makefile.Test plan
TelemetryCollectTest,TelemetryOptOutTest,TelemetrySchemaParityTest,TelemetryReadmeParityTest). 403 tests, 1765 assertions total.telemetry.instance_id, transitions audit-log.mainwithout any of these changes. Settings-related tests (admin-settings-token-lifetimes, the Features section a11y scans,smoke /admin/settings) all pass with my changes. The flaky failures are pre-existing dev-env infrastructure issues (DB reset races, animation timings) and CI runsworkers: 1to avoid them.Companion
The Cloudflare Worker that receives these pings lives in
sbpp/cf-analytics. That repo is separate by design — Worker code, deployment, and the canonical schema source live there. This PR'sweb/includes/Telemetry/schema-1.lock.jsonis vendored byte-for-byte from cf-analytics'sschema/1.lock.jsonand synced manually viamake sync-telemetry-schema.