feat(autocapture): route Element Path through the element-selector engine#1842
feat(autocapture): route Element Path through the element-selector engine#1842jxiwang wants to merge 11 commits into
Conversation
…uting Extracted from #1832. - Move canonical legacyCssPath walker into @amplitude/element-selector - Add generateSelector top-level helper for null-engine call sites - Route engine.generate through kill-switch and safety-net fallbacks - Publish package (remove private flag) - Bump size-limit cap to absorb consolidated walker
…gine Integrate @amplitude/element-selector into autocapture's DataExtractor, the single point where every selector string ([Amplitude] Element Path and the viewport Element Exposed paths) is produced. - DataExtractor owns a SelectorEngine and getElementPath now calls engine.generate instead of the local cssPath walker. Ships dormant: the default config (enabled: false) routes through element-selector's byte-identical legacyCssPath, so selector output is unchanged until an org opts in. - updateSelectorConfig applies an ElementSelectorRemoteConfig payload to the engine. - subscribeToElementSelectorConfig wires the engine to remote config; both the autocapture and frustration plugins subscribe so all selector-bearing events (Element Clicked, Element Changed, Dead/Rage/Error Click, Viewport Content Updated) stay consistent when the engine is enabled. This is the engine integration only; selector-config hashing is tracked separately.
…or-datextractor-integration-7d25 # Conflicts: # .size-limit.js # packages/element-selector/src/engine.ts # packages/element-selector/src/generate-selector.ts # packages/element-selector/src/legacy-css-path.ts # packages/element-selector/test/generate-selector.test.ts # packages/element-selector/test/legacy-css-path.test.ts
size-limit report 📦
|
Add a Playwright e2e spec + test page that click real elements and assert the emitted [Amplitude] Element Path. Runs in a real browser, exercising getElementPath -> engine.generate -> (dormant) legacy cssPath end to end — including real CSS.escape and real sibling/layout structure that jsdom unit tests cannot cover. Guards that the local cssPath -> engine swap did not change production selectors: - stable id anchors as 'button#cta-button' - id-less element disambiguates positionally as 'div#container > section > button:nth-child(2)'
Add a manual test-server playground under test-server/autocapture/ that renders, for each clicked element, the selector with the engine disabled (legacy cssPath) vs enabled (new strategy chain), highlighting differences. Covers stable ids, autogenerated-looking ids, positional disambiguation, unstable/library state classes, and the explicit tracking attribute. Opened via 'pnpm dev' for manual QA of selector quality on a realistic DOM (not run in CI).
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5b2e3c4. Configure here.
Pin the enabled-engine unit test to button#test-button instead of only checking length > 0, so the test guards against strategy-chain regressions.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Split engines desync on startup
- Fixed by making the SelectorEngine a module-level singleton shared across all DataExtractor instances, ensuring both plugins see the same configuration immediately when remote config is delivered.
Or push these changes by commenting:
@cursor push 9ec48be674
Preview (9ec48be674)
diff --git a/packages/plugin-autocapture-browser/src/data-extractor.ts b/packages/plugin-autocapture-browser/src/data-extractor.ts
--- a/packages/plugin-autocapture-browser/src/data-extractor.ts
+++ b/packages/plugin-autocapture-browser/src/data-extractor.ts
@@ -32,6 +32,23 @@
type ElementSelectorLogger,
} from '@amplitude/element-selector';
+/**
+ * Module-level shared selector engine singleton. Both autocapture-plugin and
+ * frustration-plugin create separate DataExtractor instances, and each
+ * subscribes to remote config independently. By sharing a single engine across
+ * all extractors, whichever subscription fires first updates the engine for
+ * everyone, eliminating the window where one plugin could see updated config
+ * while the other still uses defaults.
+ */
+let sharedSelectorEngine: SelectorEngine | undefined;
+
+function getSharedSelectorEngine(): SelectorEngine {
+ if (!sharedSelectorEngine) {
+ sharedSelectorEngine = createSelectorEngine(resolveSelectorConfig());
+ }
+ return sharedSelectorEngine;
+}
+
export class DataExtractor {
private readonly additionalMaskTextPatterns: RegExp[];
diagnosticsClient?: IDiagnosticsClient;
@@ -44,12 +61,16 @@
* engine routes through the byte-identical legacy walker, so behavior is
* unchanged until remote config flips an org onto the new algorithm via
* {@link updateSelectorConfig}.
+ *
+ * The engine is shared across all DataExtractor instances to ensure
+ * consistent selector output when both autocapture-plugin and
+ * frustration-plugin are active.
*/
private readonly selectorEngine: SelectorEngine;
constructor(options: ElementInteractionsOptions, context?: { diagnosticsClient: IDiagnosticsClient }) {
this.diagnosticsClient = context?.diagnosticsClient;
- this.selectorEngine = createSelectorEngine(resolveSelectorConfig());
+ this.selectorEngine = getSharedSelectorEngine();
const rawPatterns = options.maskTextRegex ?? [];You can send follow-ups to the cloud agent here.
Both autocapture and frustration plugins create separate DataExtractor instances that subscribe to remote config independently. Use a module-level singleton SelectorEngine so whichever subscription fires first updates config for all plugins, preventing mismatched Element Path values on the same click.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 200c5d1. Configure here.
…ment Path Add debug logging when element-selector remote config is subscribed, delivered, and applied, plus per-click Element Path logs when the engine is enabled. Enhance the manual test page to surface the emitted event path.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Shared engine leaks config
- Added reference counting and teardown method to reset the shared selector engine when all DataExtractor instances are torn down, preventing config leakage across plugin lifecycle boundaries.
Or push these changes by commenting:
@cursor push 20a040a1ce
Preview (20a040a1ce)
diff --git a/packages/plugin-autocapture-browser/src/autocapture-plugin.ts b/packages/plugin-autocapture-browser/src/autocapture-plugin.ts
--- a/packages/plugin-autocapture-browser/src/autocapture-plugin.ts
+++ b/packages/plugin-autocapture-browser/src/autocapture-plugin.ts
@@ -473,6 +473,7 @@
for (const subscription of subscriptions) {
subscription.unsubscribe();
}
+ dataExtractor.teardown();
};
return {
diff --git a/packages/plugin-autocapture-browser/src/data-extractor.ts b/packages/plugin-autocapture-browser/src/data-extractor.ts
--- a/packages/plugin-autocapture-browser/src/data-extractor.ts
+++ b/packages/plugin-autocapture-browser/src/data-extractor.ts
@@ -39,9 +39,13 @@
* all extractors, whichever subscription fires first updates the engine for
* everyone, eliminating the window where one plugin could see updated config
* while the other still uses defaults.
+ *
+ * Reference counting ensures the engine is reset when all DataExtractor
+ * instances are torn down, preventing config leakage across plugin lifecycles.
*/
let sharedSelectorEngine: SelectorEngine | undefined;
let sharedSelectorLogger: ElementSelectorLogger | undefined;
+let activeExtractorCount = 0;
function getSharedSelectorEngine(): SelectorEngine {
if (!sharedSelectorEngine) {
@@ -50,6 +54,14 @@
return sharedSelectorEngine;
}
+function releaseSharedSelectorEngine(): void {
+ activeExtractorCount = Math.max(0, activeExtractorCount - 1);
+ if (activeExtractorCount === 0) {
+ sharedSelectorEngine = undefined;
+ sharedSelectorLogger = undefined;
+ }
+}
+
export class DataExtractor {
private readonly additionalMaskTextPatterns: RegExp[];
diagnosticsClient?: IDiagnosticsClient;
@@ -71,6 +83,7 @@
constructor(options: ElementInteractionsOptions, context?: { diagnosticsClient: IDiagnosticsClient }) {
this.diagnosticsClient = context?.diagnosticsClient;
+ activeExtractorCount++;
this.selectorEngine = getSharedSelectorEngine();
const rawPatterns = options.maskTextRegex ?? [];
@@ -94,6 +107,15 @@
}
/**
+ * Release resources held by this extractor. When all extractors are torn
+ * down, the shared selector engine is reset to prevent config leakage
+ * across plugin lifecycle boundaries.
+ */
+ teardown = (): void => {
+ releaseSharedSelectorEngine();
+ };
+
+ /**
* Wrapper method to replace sensitive strings using the helper function
* @param text - The text to search for sensitive data
* @returns The text with sensitive data replaced by masked text
diff --git a/packages/plugin-autocapture-browser/src/frustration-plugin.ts b/packages/plugin-autocapture-browser/src/frustration-plugin.ts
--- a/packages/plugin-autocapture-browser/src/frustration-plugin.ts
+++ b/packages/plugin-autocapture-browser/src/frustration-plugin.ts
@@ -318,6 +318,7 @@
for (const subscription of subscriptions) {
subscription.unsubscribe();
}
+ dataExtractor.teardown();
};
return {You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 8640942. Configure here.
The element-selector payload lives under configs.analyticsSDK.browserSDK.autocapture.elementSelector, not configs.analyticsSDK.elementSelector.
- Add debug/error mocks to frustration-plugin test logger - Simplify shared-engine update path and debug branches - Cover enabled-engine logging paths in data-extractor tests


Summary
Integrates
@amplitude/element-selectorinto the autocapture plugin at the single seam where selectors are produced:DataExtractor.getElementPath(). This is the engine integration only — no config hashing (tracked separately).getElementPath→cssPathis the one chokepoint that generates every selector string in the package:[Amplitude] Element Path(viagetEventProperties→ Element Clicked, action-click, Element Changed, Dead Click, Rage Click, Error Click) and the viewport[Amplitude] Element Exposedpaths (viatrack-exposure.ts). Swapping it here changes selector output for all of them at once, without touching the individual trackers.Changes
DataExtractorowns aSelectorEngine.getElementPathnow callsengine.generate(element)instead of the localcssPath. It ships dormant: with the default config (enabled: false) the engine routes through element-selector's legacy walker, which is byte-identical to the existinglibs/element-path.ts#cssPath(same Chromium-DevTools algorithm;escapeCssIdentifierdelegates to nativeCSS.escape). So selector output is unchanged until an org opts in via remote config.DataExtractor.updateSelectorConfig(remote, logger)applies anElementSelectorRemoteConfigpayload to the engine (resolveSelectorConfig→engine.updateConfig).subscribeToElementSelectorConfig(config, dataExtractor)wires the engine to remote config (keyconfigs.analyticsSDK.elementSelector, mirroring the existingpageActionssubscription). Both the autocapture and frustration plugins subscribe — so when an org enables the engine, every selector-bearing event (including Rage/Dead/Error Click from the frustration plugin) stays consistent.Behavior / rollout
enabled: truein remote config →getElementPathemits the new strategy-based selectors everywhere; flip back → reverts to legacycssPath. One-config-fetch toggle.libs/element-path.tsis left in place for now; it can later be deleted in favor of re-exporting element-selector's legacy walker (per the consolidation note in that file).Open items
configs.analyticsSDK.elementSelector) is assumed from thepageActionsprecedent — confirm against the backend contract.Verification
pnpm --filter @amplitude/plugin-autocapture-browser test— 409 passed, 100% coverage maintained (added unit tests for the helper,updateSelectorConfig, and the frustration subscription/teardown).e2e/element-path.spec.ts+test-server/element-selector-test.htmlclick real elements and assert the emitted[Amplitude] Element Pathin a real browser (exercisesgetElementPath→engine.generate→ dormant legacycssPath, incl. realCSS.escape/DOM). Assertsbutton#cta-buttonfor a stable id anddiv#container > section > button:nth-child(2)for a positional, id-less element. Passing on Chromium.pnpm build,pnpm lint(0 errors),pnpm docs:check— pass.size-limit: analytics-browser and unified bundles within budget (engine adds ~2.2 kB gz).main(which now contains theelement-selectorpackage); lockfile diff reduced to the single workspace-link entry.Checklist