fix(web): make shutdown() a real uninstall so hosts can scope capture - #3
Merged
Conversation
installWebVitalsTracker returned a no-op disposer while registering web-vitals observers that have no unsubscribe API, so vitals kept reporting after Scout.shutdown() and every re-initialize() stacked another live callback on a possibly dead instance. Register once per document and route to whichever instance is installed. installStartupTracker read PerformanceNavigationTiming, which describes the document rather than the installation, and fired whenever readyState was already complete — so re-initializing on the same page reported a second cold start with identical timings. Together these let a host mount and unmount the SDK on its own lifecycle instead of leaking capture across the rest of the page.
installRouteTracker captured history.pushState.bind(history) and restored that bound wrapper rather than the original function, so an install/uninstall cycle left the page altered and each subsequent cycle stacked another bind layer — unbounded for a host that mounts the SDK on every visit. Adds the first tests for Scout.initialize/Scout.shutdown, which had none: shutdown must hand back every patched page global, and repeated cycles must neither stack patches nor wedge re-initialization. That is the contract a Grafana app plugin or micro-frontend relies on to keep its telemetry scoped to itself.
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
1 participant
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.
Makes
Scout.shutdown()a genuine uninstall, so a host can scope RUM capture to its own lifetime instead of the page's.Why
The SDK installs itself by patching page-globals —
history.pushState/replaceState,fetch,XMLHttpRequest.prototype. That is fine when the SDK is initialized once per page load, which is what every existing consumer does. It breaks down for a host that mounts and unmounts: a Grafana app plugin, a micro-frontend, anything embedded in a larger SPA.Concretely: logX is a Grafana app plugin. Once a user opened it, the SDK kept reporting every dashboard and Explore page they visited afterwards, all tagged
service.name = logx.shutdown()exists and looked like the answer, but three defects meant calling it did not actually give the page back.What was wrong
installWebVitalsTrackerreturned a no-op disposer.onCLS/onFCP/onINP/onLCP/onTTFBregisterPerformanceObservers thatweb-vitalsv5 gives no way to unsubscribe, so vitals kept reporting aftershutdown(), and every re-initialize()stacked another live callback holding a torn-down instance. Now registered once per document and routed through a module-level slot pointing at whichever instance is currently installed.installStartupTrackerre-emitted acoldapp_startupspan on every install. It readsperformance.getEntriesByType('navigation'), which describes the document, and fires immediately wheneverreadyState === 'complete'— true for every install after the first. So re-entering the host reported a page load that never happened, carrying byte-identical timings, quietly skewing startup percentiles. Latched per document, and only on a real emission so a document with no navigation entry yet stays eligible.installRouteTrackernever restoredhistory. It capturedhistory.pushState.bind(history)and restored that bound wrapper rather than the original function. Navigation still worked, so nothing looked broken — but the page was left altered after dispose, and each subsequent install/uninstall cycle stacked anotherbindlayer. Unbounded, for a host that mounts on every visit. Now captures the unbound originals and calls through withapply.Tests
Scout.initializeandScout.shutdownhad no test coverage at all, which is why none of the above surfaced.src/web/lifecycle.test.tssnapshots the page globals, installs, and asserts shutdown restores every one of them by identity — plus that repeated cycles neither stack patches nor wedge re-initialization.That test is what found the
historybug; it was not on the list when this branch started.Also adds
startup.test.tsandweb-vitals.test.ts, neither of which existed. 210 → 225 tests,make cigreen.Compatibility
No API change and no behaviour change for a consumer that initializes once per page load, which is all of them today. Callers that never call
shutdown()are unaffected.