-
Notifications
You must be signed in to change notification settings - Fork 12
Add GPT runtime diagnostics overlay #974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ChristianPavilonis
wants to merge
3
commits into
main
Choose a base branch
from
feature/gpt-runtime-diagnostics-overlay
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
233 changes: 233 additions & 0 deletions
233
crates/trusted-server-core/src/integrations/gpt_diagnostics.rs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,233 @@ | ||
| //! GPT runtime diagnostics integration. | ||
| //! | ||
| //! This integration only makes the browser diagnostics module available and | ||
| //! injects its early tab-activation bootstrap. GPT observation and presentation | ||
| //! remain entirely client-side and do not alter ad serving behavior. | ||
|
|
||
| use std::sync::Arc; | ||
|
|
||
| use error_stack::Report; | ||
| use serde::Deserialize; | ||
| use validator::Validate; | ||
|
|
||
| use crate::error::TrustedServerError; | ||
| use crate::settings::{IntegrationConfig, Settings}; | ||
|
|
||
| use super::{IntegrationHeadInjector, IntegrationHtmlContext, IntegrationRegistration}; | ||
|
|
||
| const GPT_DIAGNOSTICS_INTEGRATION_ID: &str = "gpt_diagnostics"; | ||
| const GPT_DIAGNOSTICS_BOOTSTRAP_JS: &str = include_str!("gpt_diagnostics_bootstrap.js"); | ||
|
|
||
| /// Configuration for the GPT runtime diagnostics integration. | ||
| #[derive(Debug, Clone, Deserialize, Validate)] | ||
| #[serde(deny_unknown_fields)] | ||
| pub struct GptDiagnosticsConfig { | ||
| /// Whether the GPT diagnostics browser module is available. | ||
| #[serde(default)] | ||
| pub enabled: bool, | ||
| } | ||
|
|
||
| impl IntegrationConfig for GptDiagnosticsConfig { | ||
| fn is_enabled(&self) -> bool { | ||
| self.enabled | ||
| } | ||
| } | ||
|
|
||
| struct GptDiagnosticsIntegration; | ||
|
|
||
| /// Register GPT diagnostics when explicitly enabled. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns an error when the integration configuration cannot be parsed or | ||
| /// fails validation. | ||
| pub fn register( | ||
| settings: &Settings, | ||
| ) -> Result<Option<IntegrationRegistration>, Report<TrustedServerError>> { | ||
| let Some(_config) = | ||
| settings.integration_config::<GptDiagnosticsConfig>(GPT_DIAGNOSTICS_INTEGRATION_ID)? | ||
| else { | ||
| return Ok(None); | ||
| }; | ||
|
|
||
| let integration = Arc::new(GptDiagnosticsIntegration); | ||
| Ok(Some( | ||
| IntegrationRegistration::builder(GPT_DIAGNOSTICS_INTEGRATION_ID) | ||
| .with_head_injector(integration) | ||
| .build(), | ||
| )) | ||
| } | ||
|
|
||
| impl IntegrationHeadInjector for GptDiagnosticsIntegration { | ||
| fn integration_id(&self) -> &'static str { | ||
| GPT_DIAGNOSTICS_INTEGRATION_ID | ||
| } | ||
|
|
||
| fn head_inserts(&self, _ctx: &IntegrationHtmlContext<'_>) -> Vec<String> { | ||
| vec![format!("<script>{GPT_DIAGNOSTICS_BOOTSTRAP_JS}</script>")] | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use serde_json::json; | ||
|
|
||
| use super::*; | ||
| use crate::integrations::{IntegrationDocumentState, IntegrationRegistry}; | ||
| use crate::test_support::tests::create_test_settings; | ||
|
|
||
| #[test] | ||
| fn register_returns_none_without_config() { | ||
| let settings = create_test_settings(); | ||
|
|
||
| let registration = register(&settings).expect("should evaluate diagnostics config"); | ||
|
|
||
| assert!( | ||
| registration.is_none(), | ||
| "should not register diagnostics without explicit config" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn register_returns_none_when_disabled() { | ||
| let mut settings = create_test_settings(); | ||
| settings | ||
| .integrations | ||
| .insert_config(GPT_DIAGNOSTICS_INTEGRATION_ID, &json!({ "enabled": false })) | ||
| .expect("should insert diagnostics config"); | ||
|
|
||
| let registration = register(&settings).expect("should parse diagnostics config"); | ||
|
|
||
| assert!( | ||
| registration.is_none(), | ||
| "should not register disabled diagnostics" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn register_adds_only_immediate_js_and_head_bootstrap() { | ||
| let mut settings = create_test_settings(); | ||
| settings | ||
| .integrations | ||
| .insert_config(GPT_DIAGNOSTICS_INTEGRATION_ID, &json!({ "enabled": true })) | ||
| .expect("should insert diagnostics config"); | ||
|
|
||
| let registration = register(&settings) | ||
| .expect("should parse diagnostics config") | ||
| .expect("should register enabled diagnostics"); | ||
|
|
||
| assert_eq!(registration.integration_id, GPT_DIAGNOSTICS_INTEGRATION_ID); | ||
| assert!( | ||
| !registration.js_deferred, | ||
| "should load diagnostics immediately" | ||
| ); | ||
| assert!(!registration.js_disabled, "should include diagnostics JS"); | ||
| assert!( | ||
| registration.proxies.is_empty(), | ||
| "should register no proxy routes" | ||
| ); | ||
| assert!( | ||
| registration.attribute_rewriters.is_empty(), | ||
| "should register no attribute rewriters" | ||
| ); | ||
| assert!( | ||
| registration.script_rewriters.is_empty(), | ||
| "should register no script rewriters" | ||
| ); | ||
| assert!( | ||
| registration.html_post_processors.is_empty(), | ||
| "should register no HTML post-processors" | ||
| ); | ||
| assert!( | ||
| registration.request_filters.is_empty(), | ||
| "should register no request filters" | ||
| ); | ||
| assert_eq!( | ||
| registration.head_injectors.len(), | ||
| 1, | ||
| "should register one activation bootstrap" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn enabled_registry_includes_diagnostics_in_immediate_modules() { | ||
| let mut settings = create_test_settings(); | ||
| settings | ||
| .integrations | ||
| .insert_config(GPT_DIAGNOSTICS_INTEGRATION_ID, &json!({ "enabled": true })) | ||
| .expect("should insert diagnostics config"); | ||
|
|
||
| let registry = IntegrationRegistry::new(&settings).expect("should build registry"); | ||
|
|
||
| assert!( | ||
| registry | ||
| .js_module_ids_immediate() | ||
| .contains(&GPT_DIAGNOSTICS_INTEGRATION_ID), | ||
| "should include diagnostics in the immediate JS bundle" | ||
| ); | ||
| assert!( | ||
| !registry | ||
| .js_module_ids_deferred() | ||
| .contains(&GPT_DIAGNOSTICS_INTEGRATION_ID), | ||
| "should not defer diagnostics listener installation" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn head_bootstrap_only_exposes_private_activation_logic() { | ||
| let integration = GptDiagnosticsIntegration; | ||
| let document_state = IntegrationDocumentState::default(); | ||
| let context = IntegrationHtmlContext { | ||
| request_host: "edge.example.com", | ||
| request_scheme: "https", | ||
| origin_host: "origin.example.com", | ||
| document_state: &document_state, | ||
| }; | ||
|
|
||
| let inserts = integration.head_inserts(&context); | ||
|
|
||
| assert_eq!(inserts.len(), 1, "should emit one bootstrap script"); | ||
| assert!( | ||
| inserts[0].contains("ts_console"), | ||
| "should read the activation query parameter" | ||
| ); | ||
| assert!( | ||
| inserts[0].contains("sessionStorage"), | ||
| "should persist activation within the tab" | ||
| ); | ||
| assert!( | ||
| inserts[0].contains("history.replaceState"), | ||
| "should remove recognized activation directives" | ||
| ); | ||
| assert!( | ||
| inserts[0].contains("__tsjs_gpt_diagnostics_active"), | ||
| "should expose only the private document activation flag" | ||
| ); | ||
| assert!( | ||
| !inserts[0].contains("gptDiagnostics ="), | ||
| "bootstrap should not expose the public diagnostics API" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn config_rejects_unknown_fields() { | ||
| let mut settings = create_test_settings(); | ||
| settings | ||
| .integrations | ||
| .insert_config( | ||
| GPT_DIAGNOSTICS_INTEGRATION_ID, | ||
| &json!({ "enabled": true, "typo": true }), | ||
| ) | ||
| .expect("should insert diagnostics config"); | ||
|
|
||
| let error = settings | ||
| .integration_config::<GptDiagnosticsConfig>(GPT_DIAGNOSTICS_INTEGRATION_ID) | ||
| .expect_err("should reject unknown diagnostics config fields"); | ||
| let error_text = format!("{error:?}"); | ||
|
|
||
| assert!( | ||
| error_text.contains("typo") || error_text.contains("unknown field"), | ||
| "error should mention the unknown field: {error:?}" | ||
| ); | ||
| } | ||
| } | ||
59 changes: 59 additions & 0 deletions
59
crates/trusted-server-core/src/integrations/gpt_diagnostics_bootstrap.js
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| // Early activation bootstrap for the GPT diagnostics integration. | ||
| // | ||
| // This script intentionally owns only tab-local activation and one-time URL | ||
| // cleanup. The TypeScript integration reads the document flag below and owns | ||
| // all GPT observation, storage, API, and presentation behavior. | ||
| (function () { | ||
| if (typeof window === "undefined") return; | ||
|
|
||
| var queryName = "ts_console"; | ||
| var storageKey = "tsjs:gptDiagnostics:active"; | ||
| var activeFlag = "__tsjs_gpt_diagnostics_active"; | ||
| var active = false; | ||
| var directiveRecognized = false; | ||
| var url; | ||
|
|
||
| try { | ||
| url = new URL(window.location.href); | ||
| var value = url.searchParams.get(queryName); | ||
|
|
||
| if (value === "1" || value === "true") { | ||
| active = true; | ||
| directiveRecognized = true; | ||
| } else if (value === "0" || value === "false") { | ||
| active = false; | ||
| directiveRecognized = true; | ||
| } | ||
| } catch (_) { | ||
| url = undefined; | ||
| } | ||
|
|
||
| if (directiveRecognized) { | ||
| try { | ||
| window.sessionStorage.setItem(storageKey, active ? "1" : "0"); | ||
| } catch (_) { | ||
| // The recognized directive still applies to this document. | ||
| } | ||
|
|
||
| if (url) { | ||
| url.searchParams.delete(queryName); | ||
| try { | ||
| window.history.replaceState( | ||
| window.history.state, | ||
| "", | ||
| url.pathname + url.search + url.hash, | ||
| ); | ||
| } catch (_) { | ||
| // URL cleanup is optional and must not block diagnostics activation. | ||
| } | ||
| } | ||
| } else { | ||
| try { | ||
| active = window.sessionStorage.getItem(storageKey) === "1"; | ||
| } catch (_) { | ||
| active = false; | ||
| } | ||
| } | ||
|
|
||
| window[activeFlag] = active; | ||
| })(); |
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 thinking — Worth weighing: enabling diagnostics ships 30 KB of tester-only code to every visitor.
Built from a clean worktree of
79d0832,tsjs-gpt_diagnostics.jsis 30,343 bytes minified — the largest module in the bundle and 3×tsjs-core.js(10,031 B). With no.with_deferred_js(),js_module_ids_immediate()includes it in the synchronous first-party bundle injected at<head>start, so once an operator setsenabled = trueevery visitor downloads and parses it even though every tab stays inactive until?ts_console=1.enabled_registry_includes_diagnostics_in_immediate_modulesshows this is deliberate, and the reasoning holds: deferring would delay listener installation past the firstslotRequested/slotRenderEndedand lose the initial impression's cycle, which is the one operators most want. So this is not a suggestion to defer.The tension comes from
sessionStorage-only activation leaving the server no signal, which forces an all-or-nothing payload decision. A server-set session cookie on the activation directive would resolve both at once — the module is concatenated only for an activated tester, and for that tester it still loads immediately with listeners installed exactly as early as today. That is what #961 did with__Host-ts-console, and this test could keep asserting immediate loading, just scoped to activated sessions.If shipping to all visitors is an accepted cost for now, it may be worth recording the rationale in
gpt-diagnostics.mdbeside the deployment configuration so operators know what enabling it costs their page weight.