Skip to content

feat(autocapture): fetch page actions from remote config #1168

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
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/analytics-core/src/types/element-interactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export type Filter = {
export type LabeledEvent = {
id: string;
definition: {
event_type: 'click' | 'change'; // [Amplitude] Element Clicked | [Amplitude] Element Changed
event_type: '[Amplitude] Element Clicked' | '[Amplitude] Element Changed';
filters: Filter[];
}[];
};
Expand Down
1 change: 1 addition & 0 deletions packages/plugin-autocapture-browser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
},
"dependencies": {
"@amplitude/analytics-core": "^2.16.0",
"@amplitude/analytics-remote-config": "^0.6.3",
"rxjs": "^7.8.1",
"tslib": "^2.4.1"
},
Expand Down
53 changes: 48 additions & 5 deletions packages/plugin-autocapture-browser/src/autocapture-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
DEFAULT_ACTION_CLICK_ALLOWLIST,
DEFAULT_DATA_ATTRIBUTE_PREFIX,
} from '@amplitude/analytics-core';
import { createRemoteConfigFetch } from '@amplitude/analytics-remote-config';
import * as constants from './constants';
import { fromEvent, map, Observable, Subscription, share } from 'rxjs';
import {
Expand All @@ -28,6 +29,7 @@ import { createMutationObservable, createClickObservable } from './observables';

import {
createLabeledEventToTriggerMap,
generateEvaluateTriggers,
groupLabeledEventIdsByEventType,
matchEventToLabeledEvents,
matchLabeledEventsToTriggers,
Expand Down Expand Up @@ -152,14 +154,14 @@ export const autocapturePlugin = (options: ElementInteractionsOptions = {}): Bro
};

// Group labeled events by event type (eg. click, change)
const groupedLabeledEvents = groupLabeledEventIdsByEventType(Object.values(options.pageActions?.labeledEvents ?? {}));
let groupedLabeledEvents = groupLabeledEventIdsByEventType(Object.values(options.pageActions?.labeledEvents ?? {}));

const labeledEventToTriggerMap = createLabeledEventToTriggerMap(options.pageActions?.triggers ?? []);
let labeledEventToTriggerMap = createLabeledEventToTriggerMap(options.pageActions?.triggers ?? []);

// Evaluate triggers for the given event by running the actions associated with the matching triggers
const evaluateTriggers = <T extends ElementBasedEvent>(
event: ElementBasedTimestampedEvent<T>,
): ElementBasedTimestampedEvent<T> => {
let evaluateTriggers = (
event: ElementBasedTimestampedEvent<ElementBasedEvent>,
): ElementBasedTimestampedEvent<ElementBasedEvent> => {
// If there is no pageActions, return the event as is
const { pageActions } = options;
if (!pageActions) {
Expand All @@ -180,12 +182,53 @@ export const autocapturePlugin = (options: ElementInteractionsOptions = {}): Bro
return event;
};

// Function to recalculate internal variables when remote config is updated
const recomputePageActionsData = (remotePageActions: ElementInteractionsOptions['pageActions']) => {
if (remotePageActions) {
// Merge remote config with local options
options.pageActions = {
...options.pageActions,
...remotePageActions,
};

// Recalculate internal variables
groupedLabeledEvents = groupLabeledEventIdsByEventType(Object.values(options.pageActions.labeledEvents ?? {}));
labeledEventToTriggerMap = createLabeledEventToTriggerMap(options.pageActions.triggers ?? []);

// Update evaluateTriggers function
evaluateTriggers = generateEvaluateTriggers(groupedLabeledEvents, labeledEventToTriggerMap, options);
}
};

const setup: BrowserEnrichmentPlugin['setup'] = async (config, amplitude) => {
/* istanbul ignore if */
if (typeof document === 'undefined') {
return;
}

// Fetch remote config for pageActions in a non-blocking manner
if (config.fetchRemoteConfig) {
createRemoteConfigFetch({
localConfig: config,
configKeys: ['analyticsSDK.pageActions'],
})
.then(async (remoteConfigFetch) => {
try {
const remotePageActions = await remoteConfigFetch.getRemoteConfig('analyticsSDK', 'pageActions');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the remote config endpoint allow us to return analyticsSDK: { browserSDK, pageActions} instead of calling these 2 separately?

recomputePageActionsData(remotePageActions as ElementInteractionsOptions['pageActions']);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to confirm, this means that:

  1. if the fetch fails, it will fallback to the hardcoded configuration?
  2. up until the fetch is complete, it's going to use the hardcoded config?

} catch (error) {
// Log error but don't fail the setup
/* istanbul ignore next */
config?.loggerProvider?.error(`Failed to fetch remote config: ${String(error)}`);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nitpick: this try -> catch block isn't necessary. If the error gets thrown from inside of 'then' it will be caught by 'catch'.

})
.catch((error) => {
// Log error but don't fail the setup
/* istanbul ignore next */
config?.loggerProvider?.error(`Failed to create remote config fetch: ${String(error)}`);
});
}

// Create should track event functions the different allowlists
const shouldTrackEvent = createShouldTrackEvent(
options,
Expand Down
6 changes: 3 additions & 3 deletions packages/plugin-autocapture-browser/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,9 @@ export type ElementBasedTimestampedEvent<T> = BaseTimestampedEvent<T> & {
targetElementProperties: Record<string, any>;
};

export type evaluateTriggersFn = <T extends ElementBasedEvent>(
event: ElementBasedTimestampedEvent<T>,
) => ElementBasedTimestampedEvent<T>;
export type evaluateTriggersFn = (
event: ElementBasedTimestampedEvent<ElementBasedEvent>,
) => ElementBasedTimestampedEvent<ElementBasedEvent>;

// Union type for all possible TimestampedEvents
export type TimestampedEvent<T> = BaseTimestampedEvent<T> | ElementBasedTimestampedEvent<T>;
Expand Down
4 changes: 2 additions & 2 deletions packages/plugin-autocapture-browser/src/hierarchy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { isNonSensitiveElement, JSONValue } from './helpers';
import { Hierarchy, HierarchyNode } from './typings/autocapture';
import { isNonSensitiveElement, type JSONValue } from './helpers';
import type { Hierarchy, HierarchyNode } from './typings/autocapture';

const BLOCKED_ATTRIBUTES = [
// Already captured elsewhere in the hierarchy object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const matchEventToFilter = (event: ElementBasedTimestampedEvent<ElementBa
if (filter.subprop_key === '[Amplitude] Element Text') {
// TODO: add support for the other operators
return (
filter.subprop_op === 'exact' &&
filter.subprop_op === 'is' &&
filter.subprop_value.includes(event.targetElementProperties['[Amplitude] Element Text'] as string)
);
} else if (filter.subprop_key === '[Amplitude] Element Hierarchy') {
Expand Down
65 changes: 56 additions & 9 deletions packages/plugin-autocapture-browser/src/pageActions/triggers.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,39 @@
import { Trigger } from '@amplitude/analytics-core/lib/esm/types/element-interactions';
// Return which labeled events, if any, the element matches
import type { LabeledEvent } from '@amplitude/analytics-core/lib/esm/types/element-interactions';
import type {
ElementInteractionsOptions,
LabeledEvent,
} from '@amplitude/analytics-core/lib/esm/types/element-interactions';
import { ElementBasedTimestampedEvent, ElementBasedEvent } from 'src/helpers';
import { matchEventToFilter } from './matchEventToFilter';
import { executeActions } from './actions';

const eventTypeToBrowserEventMap = {
'[Amplitude] Element Clicked': 'click',
'[Amplitude] Element Changed': 'change',
} as const;
// groups labeled events by event type
// skips any labeled events with malformed definitions or unexpected event_type
export const groupLabeledEventIdsByEventType = (labeledEvents?: LabeledEvent[] | null) => {
const groupedLabeledEvents = {
click: new Set<string>(),
change: new Set<string>(),
};
export const groupLabeledEventIdsByEventType = (labeledEvents: LabeledEvent[] | null | undefined) => {
// Initialize all event types with empty sets
const groupedLabeledEvents = Object.values(eventTypeToBrowserEventMap).reduce((acc, browserEvent) => {
acc[browserEvent] = new Set<string>();
return acc;
}, {} as Record<string, Set<string>>);

// If there are no labeled events, return the initialized groupedLabeledEvents
if (!labeledEvents) {
return groupedLabeledEvents;
}

// Group labeled events by event type
for (const le of labeledEvents) {
try {
for (const def of le.definition) {
groupedLabeledEvents[def.event_type]?.add(le.id);
const browserEvent = eventTypeToBrowserEventMap[def.event_type];
if (browserEvent) {
groupedLabeledEvents[browserEvent].add(le.id);
}
}
} catch (e) {
// Skip this labeled event if there is an error
Expand Down Expand Up @@ -64,7 +79,10 @@ export const matchEventToLabeledEvents = (
) => {
return labeledEvents.filter((le) => {
return le.definition.some((def) => {
return def.event_type === event.type && def.filters.every((filter) => matchEventToFilter(event, filter));
return (
eventTypeToBrowserEventMap[def.event_type] === event.type &&
def.filters.every((filter) => matchEventToFilter(event, filter))
);
});
});
};
Expand All @@ -74,8 +92,37 @@ export const matchLabeledEventsToTriggers = (labeledEvents: LabeledEvent[], leTo
for (const le of labeledEvents) {
const triggers = leToTriggerMap.get(le.id);
if (triggers) {
triggers.forEach((trigger) => matchingTriggers.add(trigger));
for (const trigger of triggers) {
matchingTriggers.add(trigger);
}
}
}
return Array.from(matchingTriggers);
};

export const generateEvaluateTriggers = (
groupedLabeledEvents: ReturnType<typeof groupLabeledEventIdsByEventType>,
labeledEventToTriggerMap: ReturnType<typeof createLabeledEventToTriggerMap>,
options: ElementInteractionsOptions,
) => {
return (event: ElementBasedTimestampedEvent<ElementBasedEvent>) => {
// If there is no pageActions, return the event as is
const { pageActions } = options;
if (!pageActions) {
return event;
}

// Find matching labeled events
const matchingLabeledEvents = matchEventToLabeledEvents(
event,
Array.from(groupedLabeledEvents[event.type]).map((id) => pageActions.labeledEvents[id]),
);
// Find matching conditions
const matchingTriggers = matchLabeledEventsToTriggers(matchingLabeledEvents, labeledEventToTriggerMap);
for (const trigger of matchingTriggers) {
executeActions(trigger.actions, event);
}

return event;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this function be moved into a helper function? It looks like it does the same thing as evaluateTriggers from autocapture-plugin.ts

};
};
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ describe('page actions', () => {
id: '123',
definition: [
{
event_type: 'click',
event_type: '[Amplitude] Element Clicked',
filters: [
{
subprop_key: '[Amplitude] Element Text',
subprop_op: 'exact',
subprop_op: 'is',
subprop_value: ['Add to Cart'],
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe('action clicks:', () => {
const loggerProvider: Partial<ILogger> = {
log: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const config: Partial<BrowserConfig> = {
defaultTracking: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('autoTrackingPlugin', () => {
const loggerProvider: Partial<ILogger> = {
log: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const config: Partial<BrowserConfig> = {
defaultTracking: false,
Expand All @@ -86,6 +87,7 @@ describe('autoTrackingPlugin', () => {
const loggerProvider: Partial<ILogger> = {
log: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const config: Partial<BrowserConfig> = {
defaultTracking: false,
Expand Down Expand Up @@ -599,6 +601,7 @@ describe('autoTrackingPlugin', () => {
const loggerProvider: Partial<ILogger> = {
log: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const config: Partial<BrowserConfig> = {
defaultTracking: false,
Expand Down Expand Up @@ -641,6 +644,7 @@ describe('autoTrackingPlugin', () => {
const loggerProvider: Partial<ILogger> = {
log: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const config: Partial<BrowserConfig> = {
defaultTracking: false,
Expand Down Expand Up @@ -897,6 +901,7 @@ describe('autoTrackingPlugin', () => {
const loggerProvider: Partial<ILogger> = {
log: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const config: Partial<BrowserConfig> = {
defaultTracking: false,
Expand All @@ -922,6 +927,7 @@ describe('autoTrackingPlugin', () => {
const loggerProvider: Partial<ILogger> = {
log: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const config: Partial<BrowserConfig> = {
defaultTracking: false,
Expand All @@ -942,6 +948,7 @@ describe('autoTrackingPlugin', () => {
const loggerProvider: Partial<ILogger> = {
log: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};
const config: Partial<BrowserConfig> = {
defaultTracking: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,31 +52,31 @@ describe('matchEventToFilter', () => {
testContainer.appendChild(dummyElement);
});

test('should return true if subprop_op is "exact" and text matches', () => {
test('should return true if subprop_op is "is" and text matches', () => {
const event = createEventForTesting('Hello World', dummyElement);
const filter: Filter = {
subprop_key: textFilterKey,
subprop_op: 'exact',
subprop_op: 'is',
subprop_value: ['Hello World'],
};
expect(matchEventToFilter(event, filter)).toBe(true);
});

test('should return true if subprop_op is "exact" and text is one of the values', () => {
test('should return true if subprop_op is "is" and text is one of the values', () => {
const event = createEventForTesting('Click Here', dummyElement);
const filter: Filter = {
subprop_key: textFilterKey,
subprop_op: 'exact',
subprop_op: 'is',
subprop_value: ['Submit', 'Click Here', 'View More'],
};
expect(matchEventToFilter(event, filter)).toBe(true);
});

test('should return false if subprop_op is "exact" and text does not match', () => {
test('should return false if subprop_op is "is" and text does not match', () => {
const event = createEventForTesting('Goodbye World', dummyElement);
const filter: Filter = {
subprop_key: textFilterKey,
subprop_op: 'exact',
subprop_op: 'is',
subprop_value: ['Hello World'],
};
expect(matchEventToFilter(event, filter)).toBe(false);
Expand All @@ -86,14 +86,14 @@ describe('matchEventToFilter', () => {
const event = createEventForTesting(undefined, dummyElement);
const filter: Filter = {
subprop_key: textFilterKey,
subprop_op: 'exact',
subprop_op: 'is',
subprop_value: ['Hello World'],
};
expect(matchEventToFilter(event, filter)).toBe(false);
});

// TODO: add tests for other operators
test('should return false if subprop_op is not "exact" (due to other operators not implemented)', () => {
test('should return false if subprop_op is not "is" (due to other operators not implemented)', () => {
const event = createEventForTesting('Hello World', dummyElement);
const filter: Filter = {
subprop_key: textFilterKey,
Expand All @@ -103,11 +103,11 @@ describe('matchEventToFilter', () => {
expect(matchEventToFilter(event, filter)).toBe(false);
});

test('should return false if subprop_value is an empty array for "exact" text match', () => {
test('should return false if subprop_value is an empty array for "is" text match', () => {
const event = createEventForTesting('Hello World', dummyElement);
const filter: Filter = {
subprop_key: textFilterKey,
subprop_op: 'exact',
subprop_op: 'is',
subprop_value: [],
};
expect(matchEventToFilter(event, filter)).toBe(false);
Expand Down Expand Up @@ -175,7 +175,7 @@ describe('matchEventToFilter', () => {
const event = createEventForTesting('Any text', button);
const filter: Filter = {
subprop_key: hierarchyFilterKey,
subprop_op: 'exact', // Any other operator
subprop_op: 'is', // Any other operator
subprop_value: ['div > .my-button'],
};
expect(matchEventToFilter(event, filter)).toBe(false);
Expand Down Expand Up @@ -230,7 +230,7 @@ describe('matchEventToFilter', () => {
const event = createEventForTesting('Some Text', dummyElement);
const filter: Filter = {
subprop_key: '[Amplitude] Unknown Key' as EventSubpropKey, // Intentionally unknown
subprop_op: 'exact',
subprop_op: 'is',
subprop_value: ['Some Text'],
};
expect(matchEventToFilter(event, filter)).toBe(false);
Expand Down
Loading
Loading