From ecb5f18107f4fafc9528d6d485a7b0e657bdf949 Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Thu, 20 Aug 2026 14:12:39 +0530 Subject: [PATCH 01/18] feat(telemetry): report which host event and which params are triggered SCAL-333657 trigger() uploaded `visual-sdk-trigger-` with no properties, so the only answerable question was whether a host event fired at all, across ~90 separate Mixpanel event names. Which parameters customers actually use, which embed component triggered, and how the trigger resolved were all invisible. Adds a `visual-sdk-host-event` upload, fired once when a trigger settles or bails out, with `hostEvent` as a property so one report can rank host events and their parameters. The existing per-event upload keeps its name for existing dashboards and now carries the same properties. Payload values never leave the browser: a value is reported as its `typeof` (`name:string`), booleans included. SDK enum members are the one exception (`operator:EQ`) - a fixed token from our own contract - and only when the key is a known enum-valued parameter and the value matches that enum exactly. Key names are reported only when they read as code identifiers, since a payload can be keyed by a customer column name, and error messages are never uploaded - only a status and our own EmbedErrorCodes. Two further fixes fall out of this: - A trigger the embedded app never answered was indistinguishable from a successful one, because processTrigger resolves, rather than rejects, with Error(TRIGGER_TIMED_OUT). It now reports status 'timed-out'. - The pre-init event queue in mixpanel-service was unbounded, and with `disableSDKTracking` set initMixpanel never runs, so the queue grew for the lifetime of the page. Capped at 100. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/host-event-telemetry.spec.ts | 157 +++++++++ .../hostEventClient/host-event-client.ts | 25 +- src/embed/ts-embed.ts | 99 ++++-- src/mixpanel-service.spec.ts | 15 + src/mixpanel-service.ts | 16 +- src/utils/hostEventTelemetry.spec.ts | 241 +++++++++++++ src/utils/hostEventTelemetry.ts | 331 ++++++++++++++++++ 7 files changed, 853 insertions(+), 31 deletions(-) create mode 100644 src/embed/host-event-telemetry.spec.ts create mode 100644 src/utils/hostEventTelemetry.spec.ts create mode 100644 src/utils/hostEventTelemetry.ts diff --git a/src/embed/host-event-telemetry.spec.ts b/src/embed/host-event-telemetry.spec.ts new file mode 100644 index 000000000..20283982f --- /dev/null +++ b/src/embed/host-event-telemetry.spec.ts @@ -0,0 +1,157 @@ +import { + init, AuthType, LiveboardEmbed, HostEvent, EmbedErrorCodes, RuntimeFilterOp, +} from '../index'; +import { getDocumentBody, getRootEl } from '../test/test-utils'; +import { ERROR_MESSAGE } from '../errors'; +import { logger } from '../utils/logger'; +import * as authInstance from '../auth'; +import * as mixpanelInstance from '../mixpanel-service'; +import { MIXPANEL_EVENT } from '../mixpanel-service'; +import * as processTriggerInstance from '../utils/processTrigger'; + +/** + * Returns the properties of the single `visual-sdk-host-event` upload. + * @param mock The spy on `uploadMixpanelEvent` + */ +const getHostEventProps = (mock: jest.SpyInstance) => { + const calls = mock.mock.calls.filter( + ([eventId]) => eventId === MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT, + ); + expect(calls).toHaveLength(1); + return calls[0][1] as Record; +}; + +/** + * Renders a Liveboard embed, so that `trigger` runs its normal path. + */ +const renderLiveboard = async () => { + init({ + thoughtSpotHost: 'https://tshost', + authType: AuthType.None, + }); + const embed = new LiveboardEmbed(getRootEl(), { + frameParams: { width: '100%', height: '100%' }, + liveboardId: '4c8a1b2e-0000-0000-0000-000000000001', + }); + await embed.render(); + return embed; +}; + +describe('Host event telemetry', () => { + let mockUploadMixpanelEvent: jest.SpyInstance; + let mockProcessTrigger: jest.SpyInstance; + + beforeEach(() => { + document.body.innerHTML = getDocumentBody(); + jest.spyOn(authInstance, 'postLoginService').mockImplementation( + () => Promise.resolve(true as any), + ); + mockUploadMixpanelEvent = jest.spyOn(mixpanelInstance, 'uploadMixpanelEvent'); + mockProcessTrigger = jest + .spyOn(processTriggerInstance, 'processTrigger') + .mockResolvedValue({ session: 'ok' }); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('reports the host event, its parameters and a successful outcome', async () => { + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + + // The per-event upload is kept for the existing dashboards, and now + // carries the same properties. + expect(mockUploadMixpanelEvent).toHaveBeenCalledWith( + `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${HostEvent.DownloadAsCsv}`, + expect.objectContaining({ + hostEvent: HostEvent.DownloadAsCsv, + paramKeys: ['vizId'], + }), + ); + + expect(getHostEventProps(mockUploadMixpanelEvent)).toEqual( + expect.objectContaining({ + hostEvent: HostEvent.DownloadAsCsv, + embedComponentType: 'LiveboardEmbed', + contextType: 'none', + hasPayload: true, + paramCount: 1, + paramKeys: ['vizId'], + paramShape: ['vizId:string'], + status: 'success', + route: 'legacy', + durationMs: expect.any(Number), + }), + ); + }); + + test('reports parameter names and enum members, never customer values', async () => { + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.UpdateRuntimeFilters, [ + { columnName: 'Region', operator: RuntimeFilterOp.EQ, values: ['west'] }, + ]); + + const serialized = JSON.stringify(mockUploadMixpanelEvent.mock.calls); + ['Region', 'west'].forEach((value) => expect(serialized).not.toContain(value)); + + const props = getHostEventProps(mockUploadMixpanelEvent); + expect(props.paramKeys).toEqual(['columnName', 'operator', 'values']); + expect(props.paramShape).toEqual( + expect.arrayContaining([ + 'payload[].columnName:string', + 'payload[].operator:EQ', + 'payload[].values:array(1)', + ]), + ); + }); + + test('reports a trigger that the embedded app never answered', async () => { + // processTrigger resolves, rather than rejects, when it times out. + mockProcessTrigger.mockResolvedValue(new Error(ERROR_MESSAGE.TRIGGER_TIMED_OUT)); + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + + expect(getHostEventProps(mockUploadMixpanelEvent).status).toBe('timed-out'); + }); + + test('reports a failed trigger without its error message', async () => { + mockProcessTrigger.mockRejectedValue(new Error('Answer 4c8a1b2e not found')); + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await expect(embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' })).rejects.toThrow(); + + const props = getHostEventProps(mockUploadMixpanelEvent); + expect(props.status).toBe('error'); + expect(JSON.stringify(props)).not.toContain('4c8a1b2e'); + }); + + test('reports a trigger called before render', async () => { + jest.spyOn(logger, 'error').mockImplementation(() => undefined); + init({ + thoughtSpotHost: 'https://tshost', + authType: AuthType.None, + }); + const embed = new LiveboardEmbed(getRootEl(), { + frameParams: { width: '100%', height: '100%' }, + liveboardId: '4c8a1b2e-0000-0000-0000-000000000001', + }); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + + expect(getHostEventProps(mockUploadMixpanelEvent)).toEqual( + expect.objectContaining({ + status: 'render-not-called', + errorCode: EmbedErrorCodes.RENDER_NOT_CALLED, + }), + ); + }); +}); diff --git a/src/embed/hostEventClient/host-event-client.ts b/src/embed/hostEventClient/host-event-client.ts index 46e0f8cb0..98f389df2 100644 --- a/src/embed/hostEventClient/host-event-client.ts +++ b/src/embed/hostEventClient/host-event-client.ts @@ -1,4 +1,5 @@ import { ContextType, HostEvent } from '../../types'; +import { HostEventRoute } from '../../utils/hostEventTelemetry'; import { processTrigger as processTriggerService } from '../../utils/processTrigger'; import { getEmbedConfig } from '../embedConfig'; import { @@ -278,6 +279,10 @@ export class HostEventClient { * @param hostEvent - The host event to trigger * @param payload - Optional payload for the event * @param context - Optional context (e.g. vizId) for scoped operations + * @param onRoute - Optional telemetry hook, called with the dispatch branch + * that served the event. A custom handler can itself fall back to the legacy + * channel, so `custom-handler` reports which branch ran, not which channel + * ultimately carried the message. */ public async triggerHostEvent< HostEventT extends HostEvent, @@ -287,6 +292,7 @@ export class HostEventClient { hostEvent: HostEventT, payload?: TriggerPayload, context?: ContextT, + onRoute?: (route: HostEventRoute) => void, ): Promise> { const customHandler = this.customHandlers[hostEvent]; const passthroughEvent = PASSTHROUGH_MAP[hostEvent]; @@ -294,15 +300,22 @@ export class HostEventClient { // If embedded app supports passthrough but not this event, use legacy channel const keys = passthroughEvent ? await this.getAvailableUIPassthroughKeys(context as ContextType) : []; if (passthroughEvent && keys.length > 0 && !keys.includes(passthroughEvent)) { + onRoute?.('legacy'); return this.hostEventFallback(hostEvent, payload, context) as any; } // Custom handler (setters) > getter passthrough > legacy fallback - return (customHandler - ? customHandler(payload, context as ContextType) - : passthroughEvent - ? this.getDataWithPassthroughFallback(passthroughEvent, hostEvent, payload, context as ContextType) - : this.hostEventFallback(hostEvent, payload, context) - ) as any; + if (customHandler) { + onRoute?.('custom-handler'); + return customHandler(payload, context as ContextType) as any; + } + if (passthroughEvent) { + onRoute?.('ui-passthrough'); + return this.getDataWithPassthroughFallback( + passthroughEvent, hostEvent, payload, context as ContextType, + ) as any; + } + onRoute?.('legacy'); + return this.hostEventFallback(hostEvent, payload, context) as any; } } diff --git a/src/embed/ts-embed.ts b/src/embed/ts-embed.ts index 10ed5d535..864b9d33a 100644 --- a/src/embed/ts-embed.ts +++ b/src/embed/ts-embed.ts @@ -71,6 +71,11 @@ import { BaseViewConfig, } from '../types'; import { uploadMixpanelEvent, MIXPANEL_EVENT } from '../mixpanel-service'; +import { + getHostEventTelemetryProps, + HostEventRoute, + HostEventStatus, +} from '../utils/hostEventTelemetry'; import { processEventData, processAuthFailure } from '../utils/processData'; import { version } from '../utils/sdk-version'; import { @@ -1679,9 +1684,31 @@ export class TsEmbed { data: TriggerPayload = {} as any, context?: ContextT, ): Promise> { - uploadMixpanelEvent(`${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`); + const telemetryProps = getHostEventTelemetryProps({ + hostEvent: messageType, + payload: data, + context, + embedComponentType: this.viewConfig?.embedComponentType, + }); + const triggerStartedAt = Date.now(); + let route: HostEventRoute; + // Emitted once, when the trigger settles or bails out, so a single + // Mixpanel report can answer which host events are used, with which + // parameters, and how they resolve. + const reportHostEvent = (status: HostEventStatus, errorCode?: EmbedErrorCodes) => { + uploadMixpanelEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT, { + ...telemetryProps, + status, + durationMs: Date.now() - triggerStartedAt, + ...(route ? { route } : {}), + ...(errorCode ? { errorCode } : {}), + }); + }; + + uploadMixpanelEvent(`${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`, telemetryProps); if (!this.isRendered) { + reportHostEvent('render-not-called', EmbedErrorCodes.RENDER_NOT_CALLED); this.handleError({ errorType: ErrorDetailsTypes.VALIDATION_ERROR, message: ERROR_MESSAGE.RENDER_BEFORE_EVENTS_REQUIRED, @@ -1692,6 +1719,7 @@ export class TsEmbed { } if (!messageType) { + reportHostEvent('host-event-undefined', EmbedErrorCodes.HOST_EVENT_TYPE_UNDEFINED); this.handleError({ errorType: ErrorDetailsTypes.VALIDATION_ERROR, message: ERROR_MESSAGE.HOST_EVENT_TYPE_UNDEFINED, @@ -1707,34 +1735,57 @@ export class TsEmbed { logger.debug( `Cannot trigger ${messageType} - iframe not available (likely due to auth failure)`, ); + reportHostEvent('no-iframe'); return null; } // send an empty object, this is needed for liveboard default handlers - return this.hostEventClient.triggerHostEvent(messageType, data, context).catch( - ( - err: Error & { - isValidationError?: boolean; - embedErrorDetails?: { - errorType: ErrorDetailsTypes; - message: string; - code: EmbedErrorCodes; - error: string; - }; + return this.hostEventClient + .triggerHostEvent(messageType, data, context, (dispatchRoute) => { + route = dispatchRoute; + }) + .then((response) => { + // processTrigger resolves — it does not reject — with an Error + // when the embedded app never answers, so a timed-out trigger + // is otherwise invisible. + const settled = response as unknown; + reportHostEvent( + settled instanceof Error + && settled.message === ERROR_MESSAGE.TRIGGER_TIMED_OUT + ? 'timed-out' + : 'success', + ); + return response; + }) + .catch( + ( + err: Error & { + isValidationError?: boolean; + embedErrorDetails?: { + errorType: ErrorDetailsTypes; + message: string; + code: EmbedErrorCodes; + error: string; + }; + }, + ): Promise => { + if (err?.isValidationError) { + const errorDetails = err.embedErrorDetails ?? { + errorType: ErrorDetailsTypes.VALIDATION_ERROR, + message: err.message || ERROR_MESSAGE.UPDATEFILTERS_INVALID_PAYLOAD, + code: EmbedErrorCodes.UPDATEFILTERS_INVALID_PAYLOAD, + error: err.message, + }; + this.handleError(errorDetails); + reportHostEvent('error', errorDetails.code); + } else { + // The error message can hold customer data, so only the + // fact of the failure is reported. + reportHostEvent('error'); + } + throw err; }, - ): Promise => { - if (err?.isValidationError) { - const errorDetails = err.embedErrorDetails ?? { - errorType: ErrorDetailsTypes.VALIDATION_ERROR, - message: err.message || ERROR_MESSAGE.UPDATEFILTERS_INVALID_PAYLOAD, - code: EmbedErrorCodes.UPDATEFILTERS_INVALID_PAYLOAD, - error: err.message, - }; - this.handleError(errorDetails); - } - throw err; - }, - ); + ); } /** diff --git a/src/mixpanel-service.spec.ts b/src/mixpanel-service.spec.ts index 9fe7c229d..5f8c35ef0 100644 --- a/src/mixpanel-service.spec.ts +++ b/src/mixpanel-service.spec.ts @@ -3,6 +3,7 @@ import { initMixpanel, uploadMixpanelEvent, MIXPANEL_EVENT, + MAX_QUEUED_EVENTS, testResetMixpanel, } from './mixpanel-service'; import { AuthType } from './types'; @@ -83,6 +84,20 @@ describe('Unit test for mixpanel', () => { expect(mixpanel.track).toHaveBeenCalledTimes(2); }); + test('caps the pre-init queue, so tracking left uninitialized cannot grow it', () => { + testResetMixpanel(); + for (let i = 0; i < MAX_QUEUED_EVENTS + 50; i += 1) { + uploadMixpanelEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT, { index: i }); + } + const sessionInfo = { + mixpanelToken: 'abc123', + userGUID: '12345', + isPublicUser: false, + } as SessionInfo; + initMixpanel(sessionInfo); + expect(mixpanel.track).toHaveBeenCalledTimes(MAX_QUEUED_EVENTS); + }); + test('init mixpanel with no mixpanel token', () => { jest.spyOn(logger, 'error').mockImplementation(() => {}); initMixpanel({ test: 'dummy' } as any); diff --git a/src/mixpanel-service.ts b/src/mixpanel-service.ts index 09da666b5..61dd97b1d 100644 --- a/src/mixpanel-service.ts +++ b/src/mixpanel-service.ts @@ -22,6 +22,10 @@ export const MIXPANEL_EVENT = { VISUAL_SDK_RENDER_COMPLETE: 'visual-sdk-render-complete', VISUAL_SDK_RENDER_FAILED: 'visual-sdk-render-failed', VISUAL_SDK_TRIGGER: 'visual-sdk-trigger', + // Emitted once per host event trigger, when it settles. Carries the host + // event name as a property, so one report can rank host events and their + // parameters instead of needing one report per `visual-sdk-trigger-*` name. + VISUAL_SDK_HOST_EVENT: 'visual-sdk-host-event', VISUAL_SDK_ON: 'visual-sdk-on', VISUAL_SDK_IFRAME_LOAD_PERFORMANCE: 'visual-sdk-iframe-load-performance', VISUAL_SDK_EMBED_CREATE: 'visual-sdk-embed-create', @@ -35,6 +39,14 @@ export const MIXPANEL_EVENT = { let isMixpanelInitialized = false; let eventQueue: { eventId: string; eventProps: any }[] = []; +/** + * Upper bound on events held before mixpanel is initialized. A host + * application can turn tracking off entirely with `disableSDKTracking`, in + * which case `initMixpanel` is never called and this queue would otherwise + * grow for the lifetime of the page. + */ +export const MAX_QUEUED_EVENTS = 100; + /** * Pushes the event with its Property key-value map to mixpanel. * @param eventId @@ -42,7 +54,9 @@ let eventQueue: { eventId: string; eventProps: any }[] = []; */ export function uploadMixpanelEvent(eventId: string, eventProps = {}): void { if (!isMixpanelInitialized) { - eventQueue.push({ eventId, eventProps }); + if (eventQueue.length < MAX_QUEUED_EVENTS) { + eventQueue.push({ eventId, eventProps }); + } return; } mixpanelInstance.track(eventId, eventProps); diff --git a/src/utils/hostEventTelemetry.spec.ts b/src/utils/hostEventTelemetry.spec.ts new file mode 100644 index 000000000..82094cff3 --- /dev/null +++ b/src/utils/hostEventTelemetry.spec.ts @@ -0,0 +1,241 @@ +import { + describeHostEventPayload, + getHostEventTelemetryProps, + MAX_SHAPE_PATHS, + REDACTED_KEY, +} from './hostEventTelemetry'; +import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; +import { ApplicabilityLevel } from '../embed/hostEventClient/contracts'; +import { version } from './sdk-version'; + +describe('describeHostEventPayload', () => { + test('reports no payload for undefined and null', () => { + [undefined, null].forEach((payload) => { + expect(describeHostEventPayload(payload)).toEqual({ + hasPayload: false, + payloadType: 'none', + paramCount: 0, + paramKeys: [], + paramShape: [], + shapeTruncated: false, + }); + }); + }); + + test('reports an empty object as a payload with no parameters', () => { + const shape = describeHostEventPayload({}); + expect(shape.hasPayload).toBe(false); + expect(shape.payloadType).toBe('object'); + expect(shape.paramCount).toBe(0); + expect(shape.paramKeys).toEqual([]); + }); + + test('reports which parameters of an object payload are used', () => { + const shape = describeHostEventPayload({ + newVizName: 'Quarterly revenue', + liveboardId: '4c8a1b2e-0000-0000-0000-000000000001', + vizId: 'd0a1', + }); + expect(shape.paramCount).toBe(3); + expect(shape.paramKeys).toEqual(['liveboardId', 'newVizName', 'vizId']); + expect(shape.paramShape).toEqual([ + 'liveboardId:string', + 'newVizName:string', + 'vizId:string', + ]); + }); + + test('reports a boolean by its type, not its value', () => { + const shape = describeHostEventPayload({ runRuntimeFilters: true, isPublic: false }); + expect(shape.paramShape).toEqual(['isPublic:boolean', 'runRuntimeFilters:boolean']); + }); + + test('reports array length and the shape of the first element', () => { + const shape = describeHostEventPayload({ + runtimeFilters: [ + { columnName: 'Region', operator: RuntimeFilterOp.EQ, values: ['west', 'east'] }, + { columnName: 'Revenue', operator: RuntimeFilterOp.GT, values: [100] }, + ], + }); + expect(shape.paramKeys).toEqual(['runtimeFilters']); + expect(shape.paramShape).toEqual([ + 'runtimeFilters:array(2)', + 'runtimeFilters[]:object(3)', + 'runtimeFilters[].columnName:string', + // `operator` is an SDK enum, so the member is reported. + 'runtimeFilters[].operator:EQ', + 'runtimeFilters[].values:array(2)', + ]); + }); + + test('reports the member of an enum parameter, by either spelling', () => { + expect( + describeHostEventPayload({ + filters: [{ column: 'Region', oper: RuntimeFilterOp.IN, values: ['west'] }], + }).paramShape, + ).toContain('filters[].oper:IN'); + + expect( + describeHostEventPayload({ + filter: { + column: 'Region', + operator: RuntimeFilterOp.BW, + applicability: { level: ApplicabilityLevel.Tab, targetId: 'tab-1' }, + }, + }).paramShape, + ).toEqual( + expect.arrayContaining(['filter.operator:BW', 'filter.applicability.level:TAB']), + ); + }); + + test('falls back to the type when an enum parameter holds something else', () => { + const shape = describeHostEventPayload({ + filters: [{ column: 'Region', oper: 'Total Sales > 500', values: ['west'] }], + }); + expect(shape.paramShape).toContain('filters[].oper:string'); + expect(JSON.stringify(shape)).not.toContain('Total Sales'); + }); + + test('does not treat a customer value as an enum just because a sibling key does', () => { + // `values` is never an enum parameter, so an + // operator-shaped value in it stays a type. + const shape = describeHostEventPayload({ + oper: RuntimeFilterOp.EQ, + values: ['EQ'], + }); + expect(shape.paramShape).toEqual(['oper:EQ', 'values:array(1)', 'values[]:string']); + }); + + test('reports empty containers and nulls without walking into them', () => { + const shape = describeHostEventPayload({ + runtimeFilters: [], + parameters: {}, + vizId: null, + }); + expect(shape.paramShape).toEqual([ + 'parameters:object(0)', + 'runtimeFilters:array(0)', + 'vizId:null', + ]); + expect(shape.shapeTruncated).toBe(false); + }); + + test('treats a top-level array payload as the parameter list', () => { + const shape = describeHostEventPayload([ + { columnName: 'Region', values: ['west'] }, + ]); + expect(shape.payloadType).toBe('array'); + expect(shape.paramCount).toBe(1); + expect(shape.paramKeys).toEqual(['columnName', 'values']); + expect(shape.paramShape[0]).toBe('payload:array(1)'); + }); + + test('reports a primitive payload as its type only', () => { + expect(describeHostEventPayload('answer-guid')).toEqual( + expect.objectContaining({ + hasPayload: true, + payloadType: 'primitive', + paramKeys: [], + paramShape: ['payload:string'], + }), + ); + }); + + test('never reports a payload value', () => { + const secrets = ['Region', 'west', 'super-secret-token', 'Quarterly revenue']; + const shape = describeHostEventPayload({ + name: 'Quarterly revenue', + token: 'super-secret-token', + filters: [{ columnName: 'Region', values: ['west'] }], + }); + const serialized = JSON.stringify(shape); + secrets.forEach((secret) => { + expect(serialized).not.toContain(secret); + }); + }); + + test('redacts key names that could be customer data', () => { + const shape = describeHostEventPayload({ + 'Total Sales': 100, + région: 'west', + [`${'a'.repeat(41)}`]: 1, + vizId: 'd0a1', + }); + expect(shape.paramKeys.filter((key) => key !== 'vizId')).toEqual([ + REDACTED_KEY, + REDACTED_KEY, + REDACTED_KEY, + ]); + expect(shape.paramShape).toContain('vizId:string'); + expect(shape.paramShape).not.toContain('Total Sales:number'); + }); + + test('summarizes below the depth limit instead of walking the whole payload', () => { + const shape = describeHostEventPayload({ + a: { b: { c: { d: { e: 'deep' } } } }, + }); + expect(shape.shapeTruncated).toBe(true); + expect(shape.paramShape).toEqual([ + 'a:object(1)', + 'a.b:object(1)', + 'a.b.c:object(1)', + ]); + }); + + test('caps the number of reported key paths', () => { + const wide: Record = {}; + for (let i = 0; i < MAX_SHAPE_PATHS + 10; i += 1) { + wide[`param${i}`] = i; + } + const shape = describeHostEventPayload(wide); + expect(shape.paramCount).toBe(MAX_SHAPE_PATHS + 10); + expect(shape.paramShape).toHaveLength(MAX_SHAPE_PATHS); + expect(shape.shapeTruncated).toBe(true); + }); + + test('survives a cyclic payload', () => { + const cyclic: any = { vizId: 'd0a1' }; + cyclic.self = cyclic; + expect(() => describeHostEventPayload(cyclic)).not.toThrow(); + expect(describeHostEventPayload(cyclic).paramKeys).toEqual(['self', 'vizId']); + }); + + test('survives a payload with a throwing getter', () => { + const hostile = { + get vizId() { + throw new Error('nope'); + }, + }; + expect(describeHostEventPayload(hostile)).toEqual( + expect.objectContaining({ payloadType: 'unknown' }), + ); + }); +}); + +describe('getHostEventTelemetryProps', () => { + test('reports the host event, context, embed component and SDK version', () => { + expect( + getHostEventTelemetryProps({ + hostEvent: HostEvent.Pin, + payload: { vizId: 'd0a1' }, + context: ContextType.Liveboard, + embedComponentType: 'LiveboardEmbed', + }), + ).toEqual( + expect.objectContaining({ + hostEvent: HostEvent.Pin, + contextType: ContextType.Liveboard, + embedComponentType: 'LiveboardEmbed', + sdkVersion: version, + paramKeys: ['vizId'], + }), + ); + }); + + test('falls back when context and embed component are unknown', () => { + const props = getHostEventTelemetryProps({ hostEvent: HostEvent.Reload }); + expect(props.contextType).toBe('none'); + expect(props.embedComponentType).toBe('unknown'); + expect(props.hasPayload).toBe(false); + }); +}); diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts new file mode 100644 index 000000000..7fe7fe489 --- /dev/null +++ b/src/utils/hostEventTelemetry.ts @@ -0,0 +1,331 @@ +/* + * Telemetry helpers for host events. These build the property bag uploaded + * with the host event Mixpanel events, so we can answer which host events are + * triggered, which parameters of those events are actually used, and how those + * triggers resolve. + * + * Host event payloads carry customer data — GUIDs, filter values, search + * strings and column names. So a value is reported as its `typeof`, not as + * itself: `name:string`, never `name:"Quarterly revenue"`. + * + * The one exception is an SDK enum. `operator:EQ` is a fixed, low-cardinality + * token from our own contract, and knowing *which* operator customers pass is + * the point of the exercise, so enum members are reported by value. A value is + * treated as an enum member only when its key is a known enum-valued parameter + * *and* the value matches one of that enum's members exactly — anything else + * falls back to its type. + * + * Key names are reported too, but only when they read as SDK contract + * identifiers; a payload can be keyed by a customer column name, so anything + * else becomes REDACTED_KEY. + */ + +import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; +import { ApplicabilityLevel } from '../embed/hostEventClient/contracts'; +import { version as sdkVersion } from './sdk-version'; + +/** How deep into a payload the shape walk goes before it summarizes. */ +export const MAX_SHAPE_DEPTH = 3; + +/** Upper bound on the number of key paths reported for one payload. */ +export const MAX_SHAPE_PATHS = 40; + +/** Key names longer than this are reported as {@link REDACTED_KEY}. */ +export const MAX_KEY_LENGTH = 40; + +/** Stands in for a key name that could carry customer data. */ +export const REDACTED_KEY = 'redactedKey'; + +/** + * Path label for a payload that is not a key-value record, so that an array + * payload reads as `payload[].columnName` rather than starting with a colon. + */ +const ROOT_PATH = 'payload'; + +/** + * A key is reported verbatim only when it reads as a plain code identifier, + * the way every key in the host event contracts does. A customer column name + * used as a key ("Total Sales", "région") fails this and gets redacted. + */ +const SAFE_KEY_PATTERN = /^[A-Za-z_$][A-Za-z0-9_$]*$/; + +/** + * Host event parameters that are typed as an SDK enum, and the members that + * enum allows. A value under one of these keys is reported as-is when it is + * one of the listed members — it is a token from our own contract, not + * customer data. Add a key here when a host event gains an enum parameter. + */ +const ENUM_VALUED_PARAMS: Record = { + // `RuntimeFilter.operator`, and the `oper` spelling that + // `HostEvent.UpdateFilters` also accepts. + operator: Object.values(RuntimeFilterOp), + oper: Object.values(RuntimeFilterOp), + // `Applicability.level` on a filter or parameter update. + level: Object.values(ApplicabilityLevel), +}; + +/** + * Whether a value is a member of the enum its key is typed as. + * @param key The key the value sits under + * @param value The string value at that key + */ +function isEnumMember(key: string, value: string): boolean { + return ENUM_VALUED_PARAMS[key]?.includes(value) ?? false; +} + +/** + * Which dispatch branch inside `HostEventClient.triggerHostEvent` served the + * host event. A custom handler may itself fall back to the legacy channel, so + * `custom-handler` means "a setter with custom logic ran", not "UI passthrough + * was used". + */ +export type HostEventRoute = 'custom-handler' | 'ui-passthrough' | 'legacy'; + +/** + * How a host event trigger ended. Everything other than `success` is a case + * the host application cannot currently see in aggregate. + */ +export type HostEventStatus = + | 'success' + | 'error' + | 'timed-out' + | 'render-not-called' + | 'host-event-undefined' + | 'no-iframe'; + +/** + * The shape of a host event payload, with no values in it. + */ +export interface HostEventPayloadShape { + /** Whether the caller passed a payload with anything in it. */ + hasPayload: boolean; + /** Top-level container kind of the payload. */ + payloadType: 'none' | 'object' | 'array' | 'primitive' | 'unknown'; + /** Top-level key count for an object payload, or length for an array. */ + paramCount: number; + /** + * Sorted top-level parameter names. For an array payload these are the + * keys of the first element, which is what identifies, say, which filter + * fields a customer sets on `HostEvent.UpdateFilters`. + */ + paramKeys: string[]; + /** + * Key paths annotated with value type — `runtimeFilters:array(3)`, + * `runtimeFilters[].columnName:string`, `start:true`. Boolean values are + * reported as-is because the value is the usage signal and carries no + * customer data; every other value is reduced to its type. + */ + paramShape: string[]; + /** Whether the walk hit {@link MAX_SHAPE_PATHS} or {@link MAX_SHAPE_DEPTH}. */ + shapeTruncated: boolean; +} + +const EMPTY_SHAPE: HostEventPayloadShape = { + hasPayload: false, + payloadType: 'none', + paramCount: 0, + paramKeys: [], + paramShape: [], + shapeTruncated: false, +}; + +/** + * Returns the key if it reads as a code identifier, and a placeholder if it + * could be customer data. + * @param key A key from a host event payload + */ +function sanitizeKey(key: string): string { + return key.length <= MAX_KEY_LENGTH && SAFE_KEY_PATTERN.test(key) ? key : REDACTED_KEY; +} + +/** + * Describes a leaf value by its type, so the value itself never leaves the + * browser. An SDK enum member is the one exception — see the module comment. + * @param value A leaf value from a host event payload + * @param key The key the value sits under, used to spot enum parameters + */ +function describeLeaf(value: unknown, key?: string): string { + if (value === null) { + return 'null'; + } + if (typeof value === 'string' && key && isEnumMember(key, value)) { + return value; + } + return typeof value; +} + +/** + * Whether a value should be walked into as a key-value record. + * @param value A value from a host event payload + */ +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +interface ShapeAccumulator { + paths: string[]; + truncated: boolean; +} + +/** + * Walks a payload branch, appending `path:type` entries to the accumulator. + * The depth and path caps also bound cyclic payloads. + * @param value The value at this path + * @param path The dotted path to this value + * @param acc Collected paths and the truncation flag + * @param depth Current walk depth + * @param key The raw key this value sits under, if it has one + */ +function walkShape( + value: unknown, + path: string, + acc: ShapeAccumulator, + depth: number, + key?: string, +): void { + if (acc.paths.length >= MAX_SHAPE_PATHS) { + acc.truncated = true; + return; + } + + if (Array.isArray(value)) { + acc.paths.push(`${path}:array(${value.length})`); + if (value.length === 0) { + return; + } + if (depth >= MAX_SHAPE_DEPTH) { + acc.truncated = true; + return; + } + walkShape(value[0], `${path}[]`, acc, depth + 1, key); + return; + } + + if (isRecord(value)) { + const keys = Object.keys(value); + acc.paths.push(`${path}:object(${keys.length})`); + if (keys.length === 0) { + return; + } + if (depth >= MAX_SHAPE_DEPTH) { + acc.truncated = true; + return; + } + keys.sort().forEach((childKey) => { + walkShape( + value[childKey], `${path}.${sanitizeKey(childKey)}`, acc, depth + 1, childKey, + ); + }); + return; + } + + acc.paths.push(`${path}:${describeLeaf(value, key)}`); +} + +/** + * Summarizes a host event payload as shape only, never values. + * @param payload The payload passed to `trigger` + * @example + * ```js + * describeHostEventPayload({ runtimeFilters: [{ columnName: 'Region' }] }); + * // paramKeys: ['runtimeFilters'] + * // paramShape: ['runtimeFilters:array(1)', 'runtimeFilters[]:object(1)', + * // 'runtimeFilters[].columnName:string'] + * ``` + */ +export function describeHostEventPayload(payload: unknown): HostEventPayloadShape { + if (payload === undefined || payload === null) { + return { ...EMPTY_SHAPE }; + } + + try { + const acc: ShapeAccumulator = { paths: [], truncated: false }; + + if (Array.isArray(payload)) { + const firstElement = payload[0]; + walkShape(payload, ROOT_PATH, acc, 0); + return { + hasPayload: payload.length > 0, + payloadType: 'array', + paramCount: payload.length, + paramKeys: isRecord(firstElement) + ? Object.keys(firstElement).map(sanitizeKey).sort() + : [], + paramShape: acc.paths, + shapeTruncated: acc.truncated, + }; + } + + if (isRecord(payload)) { + const keys = Object.keys(payload); + keys.sort().forEach((key) => { + walkShape(payload[key], sanitizeKey(key), acc, 1, key); + }); + return { + hasPayload: keys.length > 0, + payloadType: 'object', + paramCount: keys.length, + paramKeys: keys.map(sanitizeKey), + paramShape: acc.paths, + shapeTruncated: acc.truncated, + }; + } + + return { + ...EMPTY_SHAPE, + hasPayload: true, + payloadType: 'primitive', + paramShape: [`${ROOT_PATH}:${describeLeaf(payload)}`], + }; + } catch (e) { + // A payload with a throwing getter must never break the trigger it is + // describing. + return { ...EMPTY_SHAPE, payloadType: 'unknown' }; + } +} + +/** + * The properties uploaded with a host event Mixpanel event. + */ +export interface HostEventTelemetryProps extends HostEventPayloadShape { + /** The host event that was triggered. */ + hostEvent: string; + /** The context the trigger was scoped to, or `none`. */ + contextType: string; + /** Which embed component triggered it, or `unknown`. */ + embedComponentType: string; + /** Version of the SDK the host application is on. */ + sdkVersion: string; +} + +/** + * Builds the property bag for a host event trigger. + * + * The name of the host event is a *property* here, not only a suffix on the + * Mixpanel event name, so that a single report can rank host events by usage + * instead of one report per event name. + * @param params Trigger details + * @param params.hostEvent The host event being triggered + * @param params.payload The payload passed to `trigger` + * @param params.context The context the trigger is scoped to + * @param params.embedComponentType The embed component that is triggering + */ +export function getHostEventTelemetryProps({ + hostEvent, + payload, + context, + embedComponentType, +}: { + hostEvent: HostEvent; + payload?: unknown; + context?: ContextType; + embedComponentType?: string; +}): HostEventTelemetryProps { + return { + hostEvent: String(hostEvent), + contextType: context ? String(context) : 'none', + embedComponentType: embedComponentType || 'unknown', + sdkVersion, + ...describeHostEventPayload(payload), + }; +} From 73a1139bf33460b8ed16f2419993ec5f5da75abf Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Thu, 20 Aug 2026 17:26:49 +0530 Subject: [PATCH 02/18] docs(telemetry): note that ui-passthrough can fall back internally too Browser verification showed a passthrough getter reporting `route: 'ui-passthrough'` while the message was ultimately carried by the legacy channel, because getDataWithPassthroughFallback falls back when the app returns no usable response. The caveat was documented for custom handlers only; it applies to the getter branch as well. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/hostEventClient/host-event-client.ts | 7 ++++--- src/utils/hostEventTelemetry.ts | 8 +++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/embed/hostEventClient/host-event-client.ts b/src/embed/hostEventClient/host-event-client.ts index 98f389df2..b7779d69b 100644 --- a/src/embed/hostEventClient/host-event-client.ts +++ b/src/embed/hostEventClient/host-event-client.ts @@ -280,9 +280,10 @@ export class HostEventClient { * @param payload - Optional payload for the event * @param context - Optional context (e.g. vizId) for scoped operations * @param onRoute - Optional telemetry hook, called with the dispatch branch - * that served the event. A custom handler can itself fall back to the legacy - * channel, so `custom-handler` reports which branch ran, not which channel - * ultimately carried the message. + * taken here. It reports which branch ran, not which channel ultimately + * carried the message: a custom handler can fall back to the legacy channel + * itself, and `ui-passthrough` falls back too when the app returns no usable + * response. */ public async triggerHostEvent< HostEventT extends HostEvent, diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index 7fe7fe489..50f17a840 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -75,9 +75,11 @@ function isEnumMember(key: string, value: string): boolean { /** * Which dispatch branch inside `HostEventClient.triggerHostEvent` served the - * host event. A custom handler may itself fall back to the legacy channel, so - * `custom-handler` means "a setter with custom logic ran", not "UI passthrough - * was used". + * host event. This is the branch that ran, not the channel that ultimately + * carried the message: `custom-handler` means "a setter with custom logic ran", + * and both it and `ui-passthrough` can fall back to the legacy channel + * internally — a custom handler when the payload lacks the fields it needs, and + * a passthrough getter when the app returns no usable response. */ export type HostEventRoute = 'custom-handler' | 'ui-passthrough' | 'legacy'; From 64dcca4386041ad217c4fc85768be5f1508e476f Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Thu, 20 Aug 2026 21:09:04 +0530 Subject: [PATCH 03/18] fix(telemetry): report a timed-out UI passthrough setter as timed out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 Review catch. For the four host events dispatched through a custom handler — Pin, SaveAnswer, UpdateFilters, DrillDown — a real 30s timeout was reported as a generic `status: 'error'`, which undercounted exactly the signal this work exists to surface. processTrigger resolves, rather than rejects, with Error(TRIGGER_TIMED_OUT). An Error has no `.find`, so handleHostEventWithParam saw a missing response and threw a plain `{ error: 'No answer found' }`, losing the timeout before trigger() could classify it. The `settled instanceof Error` check in the success path never ran, because the promise had rejected. The thrown shape is unchanged, so nothing a host application catches today moves; an `isTimeout` flag rides alongside it and trigger() reports 'timed-out'. The timeout predicate now lives once, in processTrigger, and both call sites share it. Also from review: - The paramShape doc comment still claimed booleans are reported as-is; they have been reduced to `boolean` since the typeof-not-value rule landed. - `route` is declared `HostEventRoute | undefined`, matching the branches that report before dispatch ever happens. - Tests for the 'custom-handler' and 'ui-passthrough' routes, the legacy fallback, and a regression test for the timeout above — verified to fail without the fix. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/host-event-telemetry.spec.ts | 90 ++++++++++++++++++- .../hostEventClient/host-event-client.ts | 15 +++- src/embed/ts-embed.ts | 20 ++--- src/utils/hostEventTelemetry.ts | 6 +- src/utils/processTrigger.ts | 11 +++ 5 files changed, 122 insertions(+), 20 deletions(-) diff --git a/src/embed/host-event-telemetry.spec.ts b/src/embed/host-event-telemetry.spec.ts index 20283982f..0b3ec534d 100644 --- a/src/embed/host-event-telemetry.spec.ts +++ b/src/embed/host-event-telemetry.spec.ts @@ -3,6 +3,7 @@ import { } from '../index'; import { getDocumentBody, getRootEl } from '../test/test-utils'; import { ERROR_MESSAGE } from '../errors'; +import { UIPassthroughEvent } from './hostEventClient/contracts'; import { logger } from '../utils/logger'; import * as authInstance from '../auth'; import * as mixpanelInstance from '../mixpanel-service'; @@ -62,8 +63,8 @@ describe('Host event telemetry', () => { await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); - // The per-event upload is kept for the existing dashboards, and now - // carries the same properties. + // The per-event upload keeps its name so existing Mixpanel reports + // still work, and now carries the same properties. expect(mockUploadMixpanelEvent).toHaveBeenCalledWith( `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${HostEvent.DownloadAsCsv}`, expect.objectContaining({ @@ -133,6 +134,91 @@ describe('Host event telemetry', () => { expect(JSON.stringify(props)).not.toContain('4c8a1b2e'); }); + /** + * Makes the embedded app answer UI passthrough calls, advertising the given + * passthrough keys. Everything else resolves over the legacy channel. + * @param keys The passthrough keys the app claims to support + * @param passthroughResult What a passthrough call other than the key + * lookup resolves with + */ + const mockPassthroughApp = (keys: string[], passthroughResult: any = [{ value: { ok: true } }]) => { + mockProcessTrigger.mockImplementation( + (_iFrame: any, messageType: any, _host: any, data: any) => { + if (messageType !== HostEvent.UIPassthrough) { + return Promise.resolve({ session: 'ok' }); + } + if (data?.type === UIPassthroughEvent.GetAvailableUIPassthroughs) { + return Promise.resolve([{ value: { keys } }]); + } + return Promise.resolve(passthroughResult); + }, + ); + }; + + test('reports the ui-passthrough route for a getter the app supports', async () => { + mockPassthroughApp([UIPassthroughEvent.GetTabs]); + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.GetTabs, {}); + + expect(getHostEventProps(mockUploadMixpanelEvent)).toEqual( + expect.objectContaining({ hostEvent: HostEvent.GetTabs, route: 'ui-passthrough' }), + ); + }); + + test('reports the legacy route when the app lacks the passthrough key', async () => { + mockPassthroughApp(['someUnrelatedPassthrough']); + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.GetTabs, {}); + + expect(getHostEventProps(mockUploadMixpanelEvent)).toEqual( + expect.objectContaining({ route: 'legacy' }), + ); + }); + + test('reports the custom-handler route for a setter with custom logic', async () => { + mockPassthroughApp([UIPassthroughEvent.PinAnswerToLiveboard]); + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.Pin, { + newVizName: 'Quarterly revenue', + liveboardId: '4c8a1b2e-0000-0000-0000-000000000002', + }); + + expect(getHostEventProps(mockUploadMixpanelEvent)).toEqual( + expect.objectContaining({ + route: 'custom-handler', + paramKeys: ['liveboardId', 'newVizName'], + }), + ); + }); + + test('reports a custom-handler trigger that the app never answered as timed out', async () => { + // A UI passthrough setter turns the resolved timeout + // Error into a thrown "no answer", which used to be + // reported as a plain error and hid the timeout for + // Pin, SaveAnswer, UpdateFilters and DrillDown. + mockPassthroughApp( + [UIPassthroughEvent.PinAnswerToLiveboard], + new Error(ERROR_MESSAGE.TRIGGER_TIMED_OUT), + ); + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await expect( + embed.trigger(HostEvent.Pin, { + newVizName: 'Quarterly revenue', + liveboardId: '4c8a1b2e-0000-0000-0000-000000000002', + }), + ).rejects.toBeDefined(); + + expect(getHostEventProps(mockUploadMixpanelEvent).status).toBe('timed-out'); + }); + test('reports a trigger called before render', async () => { jest.spyOn(logger, 'error').mockImplementation(() => undefined); init({ diff --git a/src/embed/hostEventClient/host-event-client.ts b/src/embed/hostEventClient/host-event-client.ts index b7779d69b..9898d9565 100644 --- a/src/embed/hostEventClient/host-event-client.ts +++ b/src/embed/hostEventClient/host-event-client.ts @@ -1,6 +1,9 @@ import { ContextType, HostEvent } from '../../types'; import { HostEventRoute } from '../../utils/hostEventTelemetry'; -import { processTrigger as processTriggerService } from '../../utils/processTrigger'; +import { + isTriggerTimeout, + processTrigger as processTriggerService, +} from '../../utils/processTrigger'; import { getEmbedConfig } from '../embedConfig'; import { isValidUpdateFiltersPayload, @@ -93,12 +96,16 @@ export class HostEventClient { parameters: UIPassthroughRequest, context?: ContextType, ): Promise> { - const response = (await this.triggerUIPassthroughApi(apiName, parameters, context)) - ?.find?.((r) => r.error || r.value); + const raw = await this.triggerUIPassthroughApi(apiName, parameters, context); + const response = raw?.find?.((r) => r.error || r.value); if (!response) { const error = `No answer found${parameters.vizId ? ` for vizId: ${parameters.vizId}` : ''}.`; - throw { error }; + // A timeout arrives here as a missing response, because + // processTrigger resolves with an Error rather than rejecting. The + // thrown shape stays as it was; the flag lets telemetry tell an + // unanswered trigger from a genuine "no answer". + throw isTriggerTimeout(raw) ? { error, isTimeout: true } : { error }; } const errors = response.error diff --git a/src/embed/ts-embed.ts b/src/embed/ts-embed.ts index 864b9d33a..22610ff88 100644 --- a/src/embed/ts-embed.ts +++ b/src/embed/ts-embed.ts @@ -76,6 +76,7 @@ import { HostEventRoute, HostEventStatus, } from '../utils/hostEventTelemetry'; +import { isTriggerTimeout } from '../utils/processTrigger'; import { processEventData, processAuthFailure } from '../utils/processData'; import { version } from '../utils/sdk-version'; import { @@ -1691,7 +1692,7 @@ export class TsEmbed { embedComponentType: this.viewConfig?.embedComponentType, }); const triggerStartedAt = Date.now(); - let route: HostEventRoute; + let route: HostEventRoute | undefined; // Emitted once, when the trigger settles or bails out, so a single // Mixpanel report can answer which host events are used, with which // parameters, and how they resolve. @@ -1745,22 +1746,14 @@ export class TsEmbed { route = dispatchRoute; }) .then((response) => { - // processTrigger resolves — it does not reject — with an Error - // when the embedded app never answers, so a timed-out trigger - // is otherwise invisible. - const settled = response as unknown; - reportHostEvent( - settled instanceof Error - && settled.message === ERROR_MESSAGE.TRIGGER_TIMED_OUT - ? 'timed-out' - : 'success', - ); + reportHostEvent(isTriggerTimeout(response) ? 'timed-out' : 'success'); return response; }) .catch( ( err: Error & { isValidationError?: boolean; + isTimeout?: boolean; embedErrorDetails?: { errorType: ErrorDetailsTypes; message: string; @@ -1778,6 +1771,11 @@ export class TsEmbed { }; this.handleError(errorDetails); reportHostEvent('error', errorDetails.code); + } else if (err?.isTimeout) { + // A UI passthrough setter turns an unanswered trigger + // into a thrown "no answer", so the timeout only + // reaches us as this flag. + reportHostEvent('timed-out'); } else { // The error message can hold customer data, so only the // fact of the failure is reported. diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index 50f17a840..d12b05a23 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -113,9 +113,9 @@ export interface HostEventPayloadShape { paramKeys: string[]; /** * Key paths annotated with value type — `runtimeFilters:array(3)`, - * `runtimeFilters[].columnName:string`, `start:true`. Boolean values are - * reported as-is because the value is the usage signal and carries no - * customer data; every other value is reduced to its type. + * `runtimeFilters[].columnName:string`, `isPublic:boolean`. Every value is + * reduced to its type, except an SDK enum member (see the module comment), + * so no customer value ever appears here. */ paramShape: string[]; /** Whether the walk hit {@link MAX_SHAPE_PATHS} or {@link MAX_SHAPE_DEPTH}. */ diff --git a/src/utils/processTrigger.ts b/src/utils/processTrigger.ts index 761eb6463..3f72db2e6 100644 --- a/src/utils/processTrigger.ts +++ b/src/utils/processTrigger.ts @@ -37,6 +37,17 @@ function postIframeMessage( export const TRIGGER_TIMEOUT = 30000; +/** + * Whether a settled `processTrigger` result is the timeout sentinel. + * + * `processTrigger` resolves — it does not reject — with an Error when the + * embedded app never answers, so without this check a timed-out trigger is + * indistinguishable from a successful one. + * @param value A value a `processTrigger` promise settled with + */ +export const isTriggerTimeout = (value: unknown): boolean => value instanceof Error + && value.message === ERROR_MESSAGE.TRIGGER_TIMED_OUT; + /** * * @param iFrame From c42ddb99275ab2c5889dd8f209218a1cfcea45de Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 12:06:05 +0530 Subject: [PATCH 04/18] refactor(telemetry): one upload per trigger, add embed events, drop comments SCAL-333657 Review feedback, seven points. Fold the data into the existing upload instead of adding a second one. The per-event `visual-sdk-trigger-` name is unchanged, but it now fires once when the trigger settles and carries the parameters, outcome and duration, so there is one upload per trigger rather than two. Report how long a trigger took, and emit `visual-sdk-host-event-no-response` alongside the per-event upload when the embedded app never answers. Track embed events too. `executeCallbacks` is the single point every embed event passes through, so one hook there covers all of them, reporting the event, its payload shape, whether the host application has a handler registered, and the event's own status. Embed payloads are large and full of customer data, so they get the same shape-only treatment as host events, with a tighter path cap. Keep it off the critical path. Nothing is walked or built when a host application has set `disableSDKTracking`, telemetry is reported after the host's own handlers have run, and the payload walk is deferred to requestIdleCallback, so an event on a hot path costs the caller a closure. Drop the comments. The one that remains is a note on the enum map, which is hand-maintained and does not scale: an unlisted enum parameter silently degrades to `string` and nothing fails, so it needs a better answer than a list someone has to remember to update. The module covers both event kinds now, so hostEventTelemetry is eventTelemetry. Co-Authored-By: Claude Opus 5 (1M context) --- ...emetry.spec.ts => event-telemetry.spec.ts} | 248 +++++++++---- .../hostEventClient/host-event-client.ts | 2 +- src/embed/ts-embed.ts | 44 +-- src/mixpanel-service.spec.ts | 2 +- src/mixpanel-service.ts | 13 +- ...lemetry.spec.ts => eventTelemetry.spec.ts} | 124 +++++-- src/utils/eventTelemetry.ts | 278 +++++++++++++++ src/utils/hostEventTelemetry.ts | 333 ------------------ src/utils/processTrigger.ts | 8 - 9 files changed, 580 insertions(+), 472 deletions(-) rename src/embed/{host-event-telemetry.spec.ts => event-telemetry.spec.ts} (51%) rename src/utils/{hostEventTelemetry.spec.ts => eventTelemetry.spec.ts} (66%) create mode 100644 src/utils/eventTelemetry.ts delete mode 100644 src/utils/hostEventTelemetry.ts diff --git a/src/embed/host-event-telemetry.spec.ts b/src/embed/event-telemetry.spec.ts similarity index 51% rename from src/embed/host-event-telemetry.spec.ts rename to src/embed/event-telemetry.spec.ts index 0b3ec534d..dc9d64c67 100644 --- a/src/embed/host-event-telemetry.spec.ts +++ b/src/embed/event-telemetry.spec.ts @@ -1,5 +1,5 @@ import { - init, AuthType, LiveboardEmbed, HostEvent, EmbedErrorCodes, RuntimeFilterOp, + init, AuthType, LiveboardEmbed, HostEvent, EmbedEvent, EmbedErrorCodes, RuntimeFilterOp, } from '../index'; import { getDocumentBody, getRootEl } from '../test/test-utils'; import { ERROR_MESSAGE } from '../errors'; @@ -10,25 +10,17 @@ import * as mixpanelInstance from '../mixpanel-service'; import { MIXPANEL_EVENT } from '../mixpanel-service'; import * as processTriggerInstance from '../utils/processTrigger'; -/** - * Returns the properties of the single `visual-sdk-host-event` upload. - * @param mock The spy on `uploadMixpanelEvent` - */ -const getHostEventProps = (mock: jest.SpyInstance) => { - const calls = mock.mock.calls.filter( - ([eventId]) => eventId === MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT, - ); - expect(calls).toHaveLength(1); - return calls[0][1] as Record; -}; +const flushTelemetry = () => new Promise((resolve) => setTimeout(resolve, 5)); + +const uploadsOf = (mock: jest.SpyInstance, eventId: string) => mock.mock.calls + .filter(([id]) => id === eventId) + .map(([, props]) => props as Record); -/** - * Renders a Liveboard embed, so that `trigger` runs its normal path. - */ -const renderLiveboard = async () => { +const renderLiveboard = async (config: Record = {}) => { init({ thoughtSpotHost: 'https://tshost', authType: AuthType.None, + ...config, }); const embed = new LiveboardEmbed(getRootEl(), { frameParams: { width: '100%', height: '100%' }, @@ -57,23 +49,42 @@ describe('Host event telemetry', () => { jest.restoreAllMocks(); }); - test('reports the host event, its parameters and a successful outcome', async () => { + const triggerProps = async (hostEvent: HostEvent) => { + await flushTelemetry(); + const uploads = uploadsOf( + mockUploadMixpanelEvent, + `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${hostEvent}`, + ); + expect(uploads).toHaveLength(1); + return uploads[0]; + }; + + const mockPassthroughApp = ( + keys: string[], + passthroughResult: any = [{ value: { ok: true } }], + ) => { + mockProcessTrigger.mockImplementation( + (_iFrame: any, messageType: any, _host: any, data: any) => { + if (messageType !== HostEvent.UIPassthrough) { + return Promise.resolve({ session: 'ok' }); + } + if (data?.type === UIPassthroughEvent.GetAvailableUIPassthroughs) { + return Promise.resolve([{ value: { keys } }]); + } + return Promise.resolve(passthroughResult); + }, + ); + }; + + test('enriches the existing per-event upload instead of adding another', async () => { const embed = await renderLiveboard(); + await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + await flushTelemetry(); - // The per-event upload keeps its name so existing Mixpanel reports - // still work, and now carries the same properties. - expect(mockUploadMixpanelEvent).toHaveBeenCalledWith( - `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${HostEvent.DownloadAsCsv}`, - expect.objectContaining({ - hostEvent: HostEvent.DownloadAsCsv, - paramKeys: ['vizId'], - }), - ); - - expect(getHostEventProps(mockUploadMixpanelEvent)).toEqual( + expect(await triggerProps(HostEvent.DownloadAsCsv)).toEqual( expect.objectContaining({ hostEvent: HostEvent.DownloadAsCsv, embedComponentType: 'LiveboardEmbed', @@ -87,20 +98,25 @@ describe('Host event telemetry', () => { durationMs: expect.any(Number), }), ); + expect(mockUploadMixpanelEvent.mock.calls.map(([id]) => id)).toEqual([ + `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${HostEvent.DownloadAsCsv}`, + ]); }); test('reports parameter names and enum members, never customer values', async () => { const embed = await renderLiveboard(); + await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await embed.trigger(HostEvent.UpdateRuntimeFilters, [ { columnName: 'Region', operator: RuntimeFilterOp.EQ, values: ['west'] }, ]); + await flushTelemetry(); const serialized = JSON.stringify(mockUploadMixpanelEvent.mock.calls); ['Region', 'west'].forEach((value) => expect(serialized).not.toContain(value)); - const props = getHostEventProps(mockUploadMixpanelEvent); + const props = await triggerProps(HostEvent.UpdateRuntimeFilters); expect(props.paramKeys).toEqual(['columnName', 'operator', 'values']); expect(props.paramShape).toEqual( expect.arrayContaining([ @@ -111,85 +127,80 @@ describe('Host event telemetry', () => { ); }); - test('reports a trigger that the embedded app never answered', async () => { - // processTrigger resolves, rather than rejects, when it times out. + test('sends a no-response event when the app never answers', async () => { mockProcessTrigger.mockResolvedValue(new Error(ERROR_MESSAGE.TRIGGER_TIMED_OUT)); const embed = await renderLiveboard(); + await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + await flushTelemetry(); - expect(getHostEventProps(mockUploadMixpanelEvent).status).toBe('timed-out'); + expect((await triggerProps(HostEvent.DownloadAsCsv)).status).toBe('timed-out'); + const noResponse = uploadsOf( + mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT_NO_RESPONSE, + ); + expect(noResponse).toHaveLength(1); + expect(noResponse[0]).toEqual( + expect.objectContaining({ + hostEvent: HostEvent.DownloadAsCsv, + status: 'timed-out', + durationMs: expect.any(Number), + }), + ); }); test('reports a failed trigger without its error message', async () => { mockProcessTrigger.mockRejectedValue(new Error('Answer 4c8a1b2e not found')); const embed = await renderLiveboard(); + await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await expect(embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' })).rejects.toThrow(); + await flushTelemetry(); - const props = getHostEventProps(mockUploadMixpanelEvent); + const props = await triggerProps(HostEvent.DownloadAsCsv); expect(props.status).toBe('error'); expect(JSON.stringify(props)).not.toContain('4c8a1b2e'); }); - /** - * Makes the embedded app answer UI passthrough calls, advertising the given - * passthrough keys. Everything else resolves over the legacy channel. - * @param keys The passthrough keys the app claims to support - * @param passthroughResult What a passthrough call other than the key - * lookup resolves with - */ - const mockPassthroughApp = (keys: string[], passthroughResult: any = [{ value: { ok: true } }]) => { - mockProcessTrigger.mockImplementation( - (_iFrame: any, messageType: any, _host: any, data: any) => { - if (messageType !== HostEvent.UIPassthrough) { - return Promise.resolve({ session: 'ok' }); - } - if (data?.type === UIPassthroughEvent.GetAvailableUIPassthroughs) { - return Promise.resolve([{ value: { keys } }]); - } - return Promise.resolve(passthroughResult); - }, - ); - }; - test('reports the ui-passthrough route for a getter the app supports', async () => { mockPassthroughApp([UIPassthroughEvent.GetTabs]); const embed = await renderLiveboard(); + await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await embed.trigger(HostEvent.GetTabs, {}); + await flushTelemetry(); - expect(getHostEventProps(mockUploadMixpanelEvent)).toEqual( - expect.objectContaining({ hostEvent: HostEvent.GetTabs, route: 'ui-passthrough' }), - ); + expect((await triggerProps(HostEvent.GetTabs)).route).toBe('ui-passthrough'); }); test('reports the legacy route when the app lacks the passthrough key', async () => { mockPassthroughApp(['someUnrelatedPassthrough']); const embed = await renderLiveboard(); + await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await embed.trigger(HostEvent.GetTabs, {}); + await flushTelemetry(); - expect(getHostEventProps(mockUploadMixpanelEvent)).toEqual( - expect.objectContaining({ route: 'legacy' }), - ); + expect((await triggerProps(HostEvent.GetTabs)).route).toBe('legacy'); }); test('reports the custom-handler route for a setter with custom logic', async () => { mockPassthroughApp([UIPassthroughEvent.PinAnswerToLiveboard]); const embed = await renderLiveboard(); + await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await embed.trigger(HostEvent.Pin, { newVizName: 'Quarterly revenue', liveboardId: '4c8a1b2e-0000-0000-0000-000000000002', }); + await flushTelemetry(); - expect(getHostEventProps(mockUploadMixpanelEvent)).toEqual( + expect(await triggerProps(HostEvent.Pin)).toEqual( expect.objectContaining({ route: 'custom-handler', paramKeys: ['liveboardId', 'newVizName'], @@ -198,15 +209,12 @@ describe('Host event telemetry', () => { }); test('reports a custom-handler trigger that the app never answered as timed out', async () => { - // A UI passthrough setter turns the resolved timeout - // Error into a thrown "no answer", which used to be - // reported as a plain error and hid the timeout for - // Pin, SaveAnswer, UpdateFilters and DrillDown. mockPassthroughApp( [UIPassthroughEvent.PinAnswerToLiveboard], new Error(ERROR_MESSAGE.TRIGGER_TIMED_OUT), ); const embed = await renderLiveboard(); + await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await expect( @@ -215,8 +223,12 @@ describe('Host event telemetry', () => { liveboardId: '4c8a1b2e-0000-0000-0000-000000000002', }), ).rejects.toBeDefined(); + await flushTelemetry(); - expect(getHostEventProps(mockUploadMixpanelEvent).status).toBe('timed-out'); + expect((await triggerProps(HostEvent.Pin)).status).toBe('timed-out'); + expect( + uploadsOf(mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT_NO_RESPONSE), + ).toHaveLength(1); }); test('reports a trigger called before render', async () => { @@ -229,15 +241,117 @@ describe('Host event telemetry', () => { frameParams: { width: '100%', height: '100%' }, liveboardId: '4c8a1b2e-0000-0000-0000-000000000001', }); + await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + await flushTelemetry(); - expect(getHostEventProps(mockUploadMixpanelEvent)).toEqual( + expect(await triggerProps(HostEvent.DownloadAsCsv)).toEqual( expect.objectContaining({ status: 'render-not-called', errorCode: EmbedErrorCodes.RENDER_NOT_CALLED, }), ); }); + + test('uploads nothing at all when the host application disabled tracking', async () => { + const embed = await renderLiveboard({ disableSDKTracking: true }); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + await flushTelemetry(); + + expect(mockUploadMixpanelEvent).not.toHaveBeenCalled(); + }); +}); + +describe('Embed event telemetry', () => { + let mockUploadMixpanelEvent: jest.SpyInstance; + + beforeEach(() => { + document.body.innerHTML = getDocumentBody(); + jest.spyOn(authInstance, 'postLoginService').mockImplementation( + () => Promise.resolve(true as any), + ); + jest.spyOn(processTriggerInstance, 'processTrigger').mockResolvedValue({ session: 'ok' }); + mockUploadMixpanelEvent = jest.spyOn(mixpanelInstance, 'uploadMixpanelEvent'); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + const embedEventUploads = async () => { + await flushTelemetry(); + return uploadsOf(mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT); + }; + + test('reports an embed event the app sent, with types only', async () => { + const embed = await renderLiveboard(); + const handler = jest.fn(); + embed.on(EmbedEvent.Data, handler); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + (embed as any).executeCallbacks(EmbedEvent.Data, { + status: 'end', + data: { columnNames: ['Region'], rows: [['west', 100]] }, + answerName: 'Quarterly revenue', + }); + + const uploads = await embedEventUploads(); + expect(uploads).toHaveLength(1); + expect(uploads[0]).toEqual( + expect.objectContaining({ + embedEvent: EmbedEvent.Data, + embedComponentType: 'LiveboardEmbed', + eventStatus: 'end', + handlerCount: 1, + }), + ); + expect(uploads[0].paramKeys).toEqual(['answerName', 'data', 'status']); + ['Region', 'west', 'Quarterly revenue'].forEach((value) => { + expect(JSON.stringify(uploads[0])).not.toContain(value); + }); + }); + + test('reports an embed event nobody is listening for', async () => { + const embed = await renderLiveboard(); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + (embed as any).executeCallbacks(EmbedEvent.Error, { status: 'end', error: 'boom' }); + + const uploads = await embedEventUploads(); + expect(uploads).toHaveLength(1); + expect(uploads[0].handlerCount).toBe(0); + expect(uploads[0].embedEvent).toBe(EmbedEvent.Error); + }); + + test('does not block the host application handler', async () => { + const embed = await renderLiveboard(); + const order: string[] = []; + embed.on(EmbedEvent.Data, () => order.push('handler')); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + mockUploadMixpanelEvent.mockImplementation(() => order.push('telemetry')); + + (embed as any).executeCallbacks(EmbedEvent.Data, { status: 'end' }); + + expect(order).toEqual(['handler']); + await flushTelemetry(); + expect(order).toEqual(['handler', 'telemetry']); + }); + + test('uploads nothing at all when the host application disabled tracking', async () => { + const embed = await renderLiveboard({ disableSDKTracking: true }); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + (embed as any).executeCallbacks(EmbedEvent.Data, { status: 'end' }); + + expect(await embedEventUploads()).toHaveLength(0); + }); }); diff --git a/src/embed/hostEventClient/host-event-client.ts b/src/embed/hostEventClient/host-event-client.ts index 9898d9565..c3f9852f3 100644 --- a/src/embed/hostEventClient/host-event-client.ts +++ b/src/embed/hostEventClient/host-event-client.ts @@ -1,5 +1,5 @@ import { ContextType, HostEvent } from '../../types'; -import { HostEventRoute } from '../../utils/hostEventTelemetry'; +import { HostEventRoute } from '../../utils/eventTelemetry'; import { isTriggerTimeout, processTrigger as processTriggerService, diff --git a/src/embed/ts-embed.ts b/src/embed/ts-embed.ts index 22610ff88..aa7b157a6 100644 --- a/src/embed/ts-embed.ts +++ b/src/embed/ts-embed.ts @@ -72,10 +72,12 @@ import { } from '../types'; import { uploadMixpanelEvent, MIXPANEL_EVENT } from '../mixpanel-service'; import { + getEmbedEventTelemetryProps, getHostEventTelemetryProps, HostEventRoute, HostEventStatus, -} from '../utils/hostEventTelemetry'; + reportEvent, +} from '../utils/eventTelemetry'; import { isTriggerTimeout } from '../utils/processTrigger'; import { processEventData, processAuthFailure } from '../utils/processData'; import { version } from '../utils/sdk-version'; @@ -1445,6 +1447,12 @@ export class TsEmbed { callbackObj.callback(data, responder); } }); + reportEvent(MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT, () => getEmbedEventTelemetryProps({ + embedEvent: eventType, + payload: data, + embedComponentType: this.viewConfig?.embedComponentType, + handlerCount: callbacks.length, + })); } /** @@ -1685,29 +1693,26 @@ export class TsEmbed { data: TriggerPayload = {} as any, context?: ContextT, ): Promise> { - const telemetryProps = getHostEventTelemetryProps({ - hostEvent: messageType, - payload: data, - context, - embedComponentType: this.viewConfig?.embedComponentType, - }); const triggerStartedAt = Date.now(); let route: HostEventRoute | undefined; - // Emitted once, when the trigger settles or bails out, so a single - // Mixpanel report can answer which host events are used, with which - // parameters, and how they resolve. const reportHostEvent = (status: HostEventStatus, errorCode?: EmbedErrorCodes) => { - uploadMixpanelEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT, { - ...telemetryProps, + const durationMs = Date.now() - triggerStartedAt; + const buildProps = () => getHostEventTelemetryProps({ + hostEvent: messageType, + payload: data, + context, + embedComponentType: this.viewConfig?.embedComponentType, status, - durationMs: Date.now() - triggerStartedAt, - ...(route ? { route } : {}), - ...(errorCode ? { errorCode } : {}), + durationMs, + route, + errorCode, }); + reportEvent(`${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`, buildProps); + if (status === 'timed-out') { + reportEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT_NO_RESPONSE, buildProps); + } }; - uploadMixpanelEvent(`${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`, telemetryProps); - if (!this.isRendered) { reportHostEvent('render-not-called', EmbedErrorCodes.RENDER_NOT_CALLED); this.handleError({ @@ -1772,13 +1777,8 @@ export class TsEmbed { this.handleError(errorDetails); reportHostEvent('error', errorDetails.code); } else if (err?.isTimeout) { - // A UI passthrough setter turns an unanswered trigger - // into a thrown "no answer", so the timeout only - // reaches us as this flag. reportHostEvent('timed-out'); } else { - // The error message can hold customer data, so only the - // fact of the failure is reported. reportHostEvent('error'); } throw err; diff --git a/src/mixpanel-service.spec.ts b/src/mixpanel-service.spec.ts index 5f8c35ef0..c421b2877 100644 --- a/src/mixpanel-service.spec.ts +++ b/src/mixpanel-service.spec.ts @@ -87,7 +87,7 @@ describe('Unit test for mixpanel', () => { test('caps the pre-init queue, so tracking left uninitialized cannot grow it', () => { testResetMixpanel(); for (let i = 0; i < MAX_QUEUED_EVENTS + 50; i += 1) { - uploadMixpanelEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT, { index: i }); + uploadMixpanelEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT_NO_RESPONSE, { index: i }); } const sessionInfo = { mixpanelToken: 'abc123', diff --git a/src/mixpanel-service.ts b/src/mixpanel-service.ts index 61dd97b1d..61c11d195 100644 --- a/src/mixpanel-service.ts +++ b/src/mixpanel-service.ts @@ -22,10 +22,8 @@ export const MIXPANEL_EVENT = { VISUAL_SDK_RENDER_COMPLETE: 'visual-sdk-render-complete', VISUAL_SDK_RENDER_FAILED: 'visual-sdk-render-failed', VISUAL_SDK_TRIGGER: 'visual-sdk-trigger', - // Emitted once per host event trigger, when it settles. Carries the host - // event name as a property, so one report can rank host events and their - // parameters instead of needing one report per `visual-sdk-trigger-*` name. - VISUAL_SDK_HOST_EVENT: 'visual-sdk-host-event', + VISUAL_SDK_HOST_EVENT_NO_RESPONSE: 'visual-sdk-host-event-no-response', + VISUAL_SDK_EMBED_EVENT: 'visual-sdk-embed-event', VISUAL_SDK_ON: 'visual-sdk-on', VISUAL_SDK_IFRAME_LOAD_PERFORMANCE: 'visual-sdk-iframe-load-performance', VISUAL_SDK_EMBED_CREATE: 'visual-sdk-embed-create', @@ -39,12 +37,6 @@ export const MIXPANEL_EVENT = { let isMixpanelInitialized = false; let eventQueue: { eventId: string; eventProps: any }[] = []; -/** - * Upper bound on events held before mixpanel is initialized. A host - * application can turn tracking off entirely with `disableSDKTracking`, in - * which case `initMixpanel` is never called and this queue would otherwise - * grow for the lifetime of the page. - */ export const MAX_QUEUED_EVENTS = 100; /** @@ -59,6 +51,7 @@ export function uploadMixpanelEvent(eventId: string, eventProps = {}): void { } return; } + mixpanelInstance.track(eventId, eventProps); } diff --git a/src/utils/hostEventTelemetry.spec.ts b/src/utils/eventTelemetry.spec.ts similarity index 66% rename from src/utils/hostEventTelemetry.spec.ts rename to src/utils/eventTelemetry.spec.ts index 82094cff3..48a39fb82 100644 --- a/src/utils/hostEventTelemetry.spec.ts +++ b/src/utils/eventTelemetry.spec.ts @@ -1,17 +1,19 @@ import { - describeHostEventPayload, + describePayload, + getEmbedEventTelemetryProps, getHostEventTelemetryProps, + MAX_EMBED_SHAPE_PATHS, MAX_SHAPE_PATHS, REDACTED_KEY, -} from './hostEventTelemetry'; -import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; +} from './eventTelemetry'; +import { ContextType, EmbedEvent, HostEvent, RuntimeFilterOp } from '../types'; import { ApplicabilityLevel } from '../embed/hostEventClient/contracts'; import { version } from './sdk-version'; -describe('describeHostEventPayload', () => { +describe('describePayload', () => { test('reports no payload for undefined and null', () => { [undefined, null].forEach((payload) => { - expect(describeHostEventPayload(payload)).toEqual({ + expect(describePayload(payload)).toEqual({ hasPayload: false, payloadType: 'none', paramCount: 0, @@ -23,7 +25,7 @@ describe('describeHostEventPayload', () => { }); test('reports an empty object as a payload with no parameters', () => { - const shape = describeHostEventPayload({}); + const shape = describePayload({}); expect(shape.hasPayload).toBe(false); expect(shape.payloadType).toBe('object'); expect(shape.paramCount).toBe(0); @@ -31,7 +33,7 @@ describe('describeHostEventPayload', () => { }); test('reports which parameters of an object payload are used', () => { - const shape = describeHostEventPayload({ + const shape = describePayload({ newVizName: 'Quarterly revenue', liveboardId: '4c8a1b2e-0000-0000-0000-000000000001', vizId: 'd0a1', @@ -46,12 +48,12 @@ describe('describeHostEventPayload', () => { }); test('reports a boolean by its type, not its value', () => { - const shape = describeHostEventPayload({ runRuntimeFilters: true, isPublic: false }); + const shape = describePayload({ runRuntimeFilters: true, isPublic: false }); expect(shape.paramShape).toEqual(['isPublic:boolean', 'runRuntimeFilters:boolean']); }); test('reports array length and the shape of the first element', () => { - const shape = describeHostEventPayload({ + const shape = describePayload({ runtimeFilters: [ { columnName: 'Region', operator: RuntimeFilterOp.EQ, values: ['west', 'east'] }, { columnName: 'Revenue', operator: RuntimeFilterOp.GT, values: [100] }, @@ -62,7 +64,6 @@ describe('describeHostEventPayload', () => { 'runtimeFilters:array(2)', 'runtimeFilters[]:object(3)', 'runtimeFilters[].columnName:string', - // `operator` is an SDK enum, so the member is reported. 'runtimeFilters[].operator:EQ', 'runtimeFilters[].values:array(2)', ]); @@ -70,13 +71,13 @@ describe('describeHostEventPayload', () => { test('reports the member of an enum parameter, by either spelling', () => { expect( - describeHostEventPayload({ + describePayload({ filters: [{ column: 'Region', oper: RuntimeFilterOp.IN, values: ['west'] }], }).paramShape, ).toContain('filters[].oper:IN'); expect( - describeHostEventPayload({ + describePayload({ filter: { column: 'Region', operator: RuntimeFilterOp.BW, @@ -89,7 +90,7 @@ describe('describeHostEventPayload', () => { }); test('falls back to the type when an enum parameter holds something else', () => { - const shape = describeHostEventPayload({ + const shape = describePayload({ filters: [{ column: 'Region', oper: 'Total Sales > 500', values: ['west'] }], }); expect(shape.paramShape).toContain('filters[].oper:string'); @@ -97,9 +98,7 @@ describe('describeHostEventPayload', () => { }); test('does not treat a customer value as an enum just because a sibling key does', () => { - // `values` is never an enum parameter, so an - // operator-shaped value in it stays a type. - const shape = describeHostEventPayload({ + const shape = describePayload({ oper: RuntimeFilterOp.EQ, values: ['EQ'], }); @@ -107,7 +106,7 @@ describe('describeHostEventPayload', () => { }); test('reports empty containers and nulls without walking into them', () => { - const shape = describeHostEventPayload({ + const shape = describePayload({ runtimeFilters: [], parameters: {}, vizId: null, @@ -121,7 +120,7 @@ describe('describeHostEventPayload', () => { }); test('treats a top-level array payload as the parameter list', () => { - const shape = describeHostEventPayload([ + const shape = describePayload([ { columnName: 'Region', values: ['west'] }, ]); expect(shape.payloadType).toBe('array'); @@ -131,7 +130,7 @@ describe('describeHostEventPayload', () => { }); test('reports a primitive payload as its type only', () => { - expect(describeHostEventPayload('answer-guid')).toEqual( + expect(describePayload('answer-guid')).toEqual( expect.objectContaining({ hasPayload: true, payloadType: 'primitive', @@ -143,7 +142,7 @@ describe('describeHostEventPayload', () => { test('never reports a payload value', () => { const secrets = ['Region', 'west', 'super-secret-token', 'Quarterly revenue']; - const shape = describeHostEventPayload({ + const shape = describePayload({ name: 'Quarterly revenue', token: 'super-secret-token', filters: [{ columnName: 'Region', values: ['west'] }], @@ -155,13 +154,13 @@ describe('describeHostEventPayload', () => { }); test('redacts key names that could be customer data', () => { - const shape = describeHostEventPayload({ + const shape = describePayload({ 'Total Sales': 100, région: 'west', [`${'a'.repeat(41)}`]: 1, vizId: 'd0a1', }); - expect(shape.paramKeys.filter((key) => key !== 'vizId')).toEqual([ + expect(shape.paramKeys.filter((key: string) => key !== 'vizId')).toEqual([ REDACTED_KEY, REDACTED_KEY, REDACTED_KEY, @@ -171,7 +170,7 @@ describe('describeHostEventPayload', () => { }); test('summarizes below the depth limit instead of walking the whole payload', () => { - const shape = describeHostEventPayload({ + const shape = describePayload({ a: { b: { c: { d: { e: 'deep' } } } }, }); expect(shape.shapeTruncated).toBe(true); @@ -187,7 +186,7 @@ describe('describeHostEventPayload', () => { for (let i = 0; i < MAX_SHAPE_PATHS + 10; i += 1) { wide[`param${i}`] = i; } - const shape = describeHostEventPayload(wide); + const shape = describePayload(wide); expect(shape.paramCount).toBe(MAX_SHAPE_PATHS + 10); expect(shape.paramShape).toHaveLength(MAX_SHAPE_PATHS); expect(shape.shapeTruncated).toBe(true); @@ -196,8 +195,8 @@ describe('describeHostEventPayload', () => { test('survives a cyclic payload', () => { const cyclic: any = { vizId: 'd0a1' }; cyclic.self = cyclic; - expect(() => describeHostEventPayload(cyclic)).not.toThrow(); - expect(describeHostEventPayload(cyclic).paramKeys).toEqual(['self', 'vizId']); + expect(() => describePayload(cyclic)).not.toThrow(); + expect(describePayload(cyclic).paramKeys).toEqual(['self', 'vizId']); }); test('survives a payload with a throwing getter', () => { @@ -206,20 +205,23 @@ describe('describeHostEventPayload', () => { throw new Error('nope'); }, }; - expect(describeHostEventPayload(hostile)).toEqual( + expect(describePayload(hostile)).toEqual( expect.objectContaining({ payloadType: 'unknown' }), ); }); }); describe('getHostEventTelemetryProps', () => { - test('reports the host event, context, embed component and SDK version', () => { + test('reports the host event, context, embed component, outcome and duration', () => { expect( getHostEventTelemetryProps({ hostEvent: HostEvent.Pin, payload: { vizId: 'd0a1' }, context: ContextType.Liveboard, embedComponentType: 'LiveboardEmbed', + status: 'success', + durationMs: 412, + route: 'custom-handler', }), ).toEqual( expect.objectContaining({ @@ -228,14 +230,76 @@ describe('getHostEventTelemetryProps', () => { embedComponentType: 'LiveboardEmbed', sdkVersion: version, paramKeys: ['vizId'], + status: 'success', + durationMs: 412, + route: 'custom-handler', }), ); }); - test('falls back when context and embed component are unknown', () => { - const props = getHostEventTelemetryProps({ hostEvent: HostEvent.Reload }); + test('omits route and errorCode when there is nothing to report', () => { + const props = getHostEventTelemetryProps({ + hostEvent: HostEvent.Reload, + status: 'no-iframe', + durationMs: 1, + }); expect(props.contextType).toBe('none'); expect(props.embedComponentType).toBe('unknown'); expect(props.hasPayload).toBe(false); + expect('route' in props).toBe(false); + expect('errorCode' in props).toBe(false); + }); +}); + +describe('getEmbedEventTelemetryProps', () => { + test('reports the embed event, its payload shape and whether anyone listens', () => { + const props = getEmbedEventTelemetryProps({ + embedEvent: EmbedEvent.Data, + payload: { + status: 'end', + data: { columnNames: ['Region'], rows: [['west', 100]] }, + }, + embedComponentType: 'LiveboardEmbed', + handlerCount: 2, + }); + expect(props).toEqual( + expect.objectContaining({ + embedEvent: EmbedEvent.Data, + embedComponentType: 'LiveboardEmbed', + eventStatus: 'end', + handlerCount: 2, + sdkVersion: version, + }), + ); + expect(props.paramKeys).toEqual(['data', 'status']); + }); + + test('never reports an embed event payload value', () => { + const props = getEmbedEventTelemetryProps({ + embedEvent: EmbedEvent.Data, + payload: { + data: { columnNames: ['Region'], rows: [['west', 100]] }, + answerName: 'Quarterly revenue', + }, + handlerCount: 0, + }); + const serialized = JSON.stringify(props); + ['Region', 'west', 'Quarterly revenue'].forEach((value) => { + expect(serialized).not.toContain(value); + }); + }); + + test('caps an embed payload harder than a host event payload', () => { + const wide: Record = {}; + for (let i = 0; i < MAX_SHAPE_PATHS; i += 1) { + wide[`field${i}`] = i; + } + const props = getEmbedEventTelemetryProps({ + embedEvent: EmbedEvent.Data, + payload: wide, + handlerCount: 1, + }); + expect(props.paramShape).toHaveLength(MAX_EMBED_SHAPE_PATHS); + expect(props.shapeTruncated).toBe(true); }); }); diff --git a/src/utils/eventTelemetry.ts b/src/utils/eventTelemetry.ts new file mode 100644 index 000000000..8ef4af676 --- /dev/null +++ b/src/utils/eventTelemetry.ts @@ -0,0 +1,278 @@ +import { ContextType, EmbedEvent, HostEvent, RuntimeFilterOp } from '../types'; +import { ApplicabilityLevel } from '../embed/hostEventClient/contracts'; +import { getEmbedConfig } from '../embed/embedConfig'; +import { uploadMixpanelEvent } from '../mixpanel-service'; +import { logger } from './logger'; +import { version as sdkVersion } from './sdk-version'; + +export const MAX_SHAPE_DEPTH = 3; +export const MAX_SHAPE_PATHS = 40; +export const MAX_EMBED_SHAPE_PATHS = 20; +export const MAX_KEY_LENGTH = 40; +export const REDACTED_KEY = 'redactedKey'; +export const IDLE_TIMEOUT = 2000; + +const ROOT_PATH = 'payload'; +const SAFE_KEY_PATTERN = /^[A-Za-z_$][A-Za-z0-9_$]*$/; + +/* + * TODO: this hand-maintained map does not scale. Every host event that gains an + * enum parameter has to be added by hand and nothing fails if it is forgotten, + * so an unlisted enum silently degrades to `string`. A generated map, or a + * marker on the contract types that the members can be read back from, would + * keep it honest. Worth finding a better way. + */ +const ENUM_VALUED_PARAMS: Record = { + operator: Object.values(RuntimeFilterOp), + oper: Object.values(RuntimeFilterOp), + level: Object.values(ApplicabilityLevel), +}; + +export type HostEventRoute = 'custom-handler' | 'ui-passthrough' | 'legacy'; + +export type HostEventStatus = + | 'success' + | 'error' + | 'timed-out' + | 'render-not-called' + | 'host-event-undefined' + | 'no-iframe'; + +export interface PayloadShape { + hasPayload: boolean; + payloadType: 'none' | 'object' | 'array' | 'primitive' | 'unknown'; + paramCount: number; + paramKeys: string[]; + paramShape: string[]; + shapeTruncated: boolean; +} + +export interface HostEventTelemetryProps extends PayloadShape { + hostEvent: string; + contextType: string; + embedComponentType: string; + sdkVersion: string; + status: HostEventStatus; + durationMs: number; + route?: HostEventRoute; + errorCode?: string; +} + +export interface EmbedEventTelemetryProps extends PayloadShape { + embedEvent: string; + embedComponentType: string; + sdkVersion: string; + eventStatus: string; + handlerCount: number; +} + +interface ShapeAccumulator { + paths: string[]; + truncated: boolean; + maxPaths: number; +} + +const EMPTY_SHAPE: PayloadShape = { + hasPayload: false, + payloadType: 'none', + paramCount: 0, + paramKeys: [], + paramShape: [], + shapeTruncated: false, +}; + +const sanitizeKey = (key: string): string => ( + key.length <= MAX_KEY_LENGTH && SAFE_KEY_PATTERN.test(key) ? key : REDACTED_KEY +); + +const isEnumMember = (key: string, value: string): boolean => ( + ENUM_VALUED_PARAMS[key]?.includes(value) ?? false +); + +const describeLeaf = (value: unknown, key?: string): string => { + if (value === null) { + return 'null'; + } + if (typeof value === 'string' && key && isEnumMember(key, value)) { + return value; + } + return typeof value; +}; + +const isRecord = (value: unknown): value is Record => ( + typeof value === 'object' && value !== null && !Array.isArray(value) +); + +const walkShape = ( + value: unknown, + path: string, + acc: ShapeAccumulator, + depth: number, + key?: string, +): void => { + if (acc.paths.length >= acc.maxPaths) { + acc.truncated = true; + return; + } + + if (Array.isArray(value)) { + acc.paths.push(`${path}:array(${value.length})`); + if (value.length === 0) { + return; + } + if (depth >= MAX_SHAPE_DEPTH) { + acc.truncated = true; + return; + } + walkShape(value[0], `${path}[]`, acc, depth + 1, key); + return; + } + + if (isRecord(value)) { + const keys = Object.keys(value); + acc.paths.push(`${path}:object(${keys.length})`); + if (keys.length === 0) { + return; + } + if (depth >= MAX_SHAPE_DEPTH) { + acc.truncated = true; + return; + } + keys.sort().forEach((childKey) => { + walkShape( + value[childKey], `${path}.${sanitizeKey(childKey)}`, acc, depth + 1, childKey, + ); + }); + return; + } + + acc.paths.push(`${path}:${describeLeaf(value, key)}`); +}; + +export const describePayload = ( + payload: unknown, + maxPaths = MAX_SHAPE_PATHS, +): PayloadShape => { + if (payload === undefined || payload === null) { + return { ...EMPTY_SHAPE }; + } + + try { + const acc: ShapeAccumulator = { paths: [], truncated: false, maxPaths }; + + if (Array.isArray(payload)) { + const firstElement = payload[0]; + walkShape(payload, ROOT_PATH, acc, 0); + return { + hasPayload: payload.length > 0, + payloadType: 'array', + paramCount: payload.length, + paramKeys: isRecord(firstElement) + ? Object.keys(firstElement).map(sanitizeKey).sort() + : [], + paramShape: acc.paths, + shapeTruncated: acc.truncated, + }; + } + + if (isRecord(payload)) { + const keys = Object.keys(payload); + keys.sort().forEach((key) => { + walkShape(payload[key], sanitizeKey(key), acc, 1, key); + }); + return { + hasPayload: keys.length > 0, + payloadType: 'object', + paramCount: keys.length, + paramKeys: keys.map(sanitizeKey), + paramShape: acc.paths, + shapeTruncated: acc.truncated, + }; + } + + return { + ...EMPTY_SHAPE, + hasPayload: true, + payloadType: 'primitive', + paramShape: [`${ROOT_PATH}:${describeLeaf(payload)}`], + }; + } catch (e) { + return { ...EMPTY_SHAPE, payloadType: 'unknown' }; + } +}; + +export const isTelemetryEnabled = (): boolean => !getEmbedConfig()?.disableSDKTracking; + +const runWhenIdle = (work: () => void): void => { + const idle = (globalThis as any)?.requestIdleCallback; + if (typeof idle === 'function') { + idle(work, { timeout: IDLE_TIMEOUT }); + return; + } + setTimeout(work, 0); +}; + +export const reportEvent = ( + eventId: string, + buildProps: () => Record, +): void => { + if (!isTelemetryEnabled()) { + return; + } + runWhenIdle(() => { + try { + uploadMixpanelEvent(eventId, buildProps()); + } catch (e) { + logger.debug('Could not report telemetry for', eventId, e); + } + }); +}; + +export const getHostEventTelemetryProps = ({ + hostEvent, + payload, + context, + embedComponentType, + status, + durationMs, + route, + errorCode, +}: { + hostEvent: HostEvent; + payload?: unknown; + context?: ContextType; + embedComponentType?: string; + status: HostEventStatus; + durationMs: number; + route?: HostEventRoute; + errorCode?: string; +}): HostEventTelemetryProps => ({ + hostEvent: String(hostEvent), + contextType: context ? String(context) : 'none', + embedComponentType: embedComponentType || 'unknown', + sdkVersion, + status, + durationMs, + ...(route ? { route } : {}), + ...(errorCode ? { errorCode } : {}), + ...describePayload(payload), +}); + +export const getEmbedEventTelemetryProps = ({ + embedEvent, + payload, + embedComponentType, + handlerCount, +}: { + embedEvent: EmbedEvent; + payload?: any; + embedComponentType?: string; + handlerCount: number; +}): EmbedEventTelemetryProps => ({ + embedEvent: String(embedEvent), + embedComponentType: embedComponentType || 'unknown', + sdkVersion, + eventStatus: payload?.status ? String(payload.status) : 'none', + handlerCount, + ...describePayload(payload, MAX_EMBED_SHAPE_PATHS), +}); diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts deleted file mode 100644 index d12b05a23..000000000 --- a/src/utils/hostEventTelemetry.ts +++ /dev/null @@ -1,333 +0,0 @@ -/* - * Telemetry helpers for host events. These build the property bag uploaded - * with the host event Mixpanel events, so we can answer which host events are - * triggered, which parameters of those events are actually used, and how those - * triggers resolve. - * - * Host event payloads carry customer data — GUIDs, filter values, search - * strings and column names. So a value is reported as its `typeof`, not as - * itself: `name:string`, never `name:"Quarterly revenue"`. - * - * The one exception is an SDK enum. `operator:EQ` is a fixed, low-cardinality - * token from our own contract, and knowing *which* operator customers pass is - * the point of the exercise, so enum members are reported by value. A value is - * treated as an enum member only when its key is a known enum-valued parameter - * *and* the value matches one of that enum's members exactly — anything else - * falls back to its type. - * - * Key names are reported too, but only when they read as SDK contract - * identifiers; a payload can be keyed by a customer column name, so anything - * else becomes REDACTED_KEY. - */ - -import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; -import { ApplicabilityLevel } from '../embed/hostEventClient/contracts'; -import { version as sdkVersion } from './sdk-version'; - -/** How deep into a payload the shape walk goes before it summarizes. */ -export const MAX_SHAPE_DEPTH = 3; - -/** Upper bound on the number of key paths reported for one payload. */ -export const MAX_SHAPE_PATHS = 40; - -/** Key names longer than this are reported as {@link REDACTED_KEY}. */ -export const MAX_KEY_LENGTH = 40; - -/** Stands in for a key name that could carry customer data. */ -export const REDACTED_KEY = 'redactedKey'; - -/** - * Path label for a payload that is not a key-value record, so that an array - * payload reads as `payload[].columnName` rather than starting with a colon. - */ -const ROOT_PATH = 'payload'; - -/** - * A key is reported verbatim only when it reads as a plain code identifier, - * the way every key in the host event contracts does. A customer column name - * used as a key ("Total Sales", "région") fails this and gets redacted. - */ -const SAFE_KEY_PATTERN = /^[A-Za-z_$][A-Za-z0-9_$]*$/; - -/** - * Host event parameters that are typed as an SDK enum, and the members that - * enum allows. A value under one of these keys is reported as-is when it is - * one of the listed members — it is a token from our own contract, not - * customer data. Add a key here when a host event gains an enum parameter. - */ -const ENUM_VALUED_PARAMS: Record = { - // `RuntimeFilter.operator`, and the `oper` spelling that - // `HostEvent.UpdateFilters` also accepts. - operator: Object.values(RuntimeFilterOp), - oper: Object.values(RuntimeFilterOp), - // `Applicability.level` on a filter or parameter update. - level: Object.values(ApplicabilityLevel), -}; - -/** - * Whether a value is a member of the enum its key is typed as. - * @param key The key the value sits under - * @param value The string value at that key - */ -function isEnumMember(key: string, value: string): boolean { - return ENUM_VALUED_PARAMS[key]?.includes(value) ?? false; -} - -/** - * Which dispatch branch inside `HostEventClient.triggerHostEvent` served the - * host event. This is the branch that ran, not the channel that ultimately - * carried the message: `custom-handler` means "a setter with custom logic ran", - * and both it and `ui-passthrough` can fall back to the legacy channel - * internally — a custom handler when the payload lacks the fields it needs, and - * a passthrough getter when the app returns no usable response. - */ -export type HostEventRoute = 'custom-handler' | 'ui-passthrough' | 'legacy'; - -/** - * How a host event trigger ended. Everything other than `success` is a case - * the host application cannot currently see in aggregate. - */ -export type HostEventStatus = - | 'success' - | 'error' - | 'timed-out' - | 'render-not-called' - | 'host-event-undefined' - | 'no-iframe'; - -/** - * The shape of a host event payload, with no values in it. - */ -export interface HostEventPayloadShape { - /** Whether the caller passed a payload with anything in it. */ - hasPayload: boolean; - /** Top-level container kind of the payload. */ - payloadType: 'none' | 'object' | 'array' | 'primitive' | 'unknown'; - /** Top-level key count for an object payload, or length for an array. */ - paramCount: number; - /** - * Sorted top-level parameter names. For an array payload these are the - * keys of the first element, which is what identifies, say, which filter - * fields a customer sets on `HostEvent.UpdateFilters`. - */ - paramKeys: string[]; - /** - * Key paths annotated with value type — `runtimeFilters:array(3)`, - * `runtimeFilters[].columnName:string`, `isPublic:boolean`. Every value is - * reduced to its type, except an SDK enum member (see the module comment), - * so no customer value ever appears here. - */ - paramShape: string[]; - /** Whether the walk hit {@link MAX_SHAPE_PATHS} or {@link MAX_SHAPE_DEPTH}. */ - shapeTruncated: boolean; -} - -const EMPTY_SHAPE: HostEventPayloadShape = { - hasPayload: false, - payloadType: 'none', - paramCount: 0, - paramKeys: [], - paramShape: [], - shapeTruncated: false, -}; - -/** - * Returns the key if it reads as a code identifier, and a placeholder if it - * could be customer data. - * @param key A key from a host event payload - */ -function sanitizeKey(key: string): string { - return key.length <= MAX_KEY_LENGTH && SAFE_KEY_PATTERN.test(key) ? key : REDACTED_KEY; -} - -/** - * Describes a leaf value by its type, so the value itself never leaves the - * browser. An SDK enum member is the one exception — see the module comment. - * @param value A leaf value from a host event payload - * @param key The key the value sits under, used to spot enum parameters - */ -function describeLeaf(value: unknown, key?: string): string { - if (value === null) { - return 'null'; - } - if (typeof value === 'string' && key && isEnumMember(key, value)) { - return value; - } - return typeof value; -} - -/** - * Whether a value should be walked into as a key-value record. - * @param value A value from a host event payload - */ -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - -interface ShapeAccumulator { - paths: string[]; - truncated: boolean; -} - -/** - * Walks a payload branch, appending `path:type` entries to the accumulator. - * The depth and path caps also bound cyclic payloads. - * @param value The value at this path - * @param path The dotted path to this value - * @param acc Collected paths and the truncation flag - * @param depth Current walk depth - * @param key The raw key this value sits under, if it has one - */ -function walkShape( - value: unknown, - path: string, - acc: ShapeAccumulator, - depth: number, - key?: string, -): void { - if (acc.paths.length >= MAX_SHAPE_PATHS) { - acc.truncated = true; - return; - } - - if (Array.isArray(value)) { - acc.paths.push(`${path}:array(${value.length})`); - if (value.length === 0) { - return; - } - if (depth >= MAX_SHAPE_DEPTH) { - acc.truncated = true; - return; - } - walkShape(value[0], `${path}[]`, acc, depth + 1, key); - return; - } - - if (isRecord(value)) { - const keys = Object.keys(value); - acc.paths.push(`${path}:object(${keys.length})`); - if (keys.length === 0) { - return; - } - if (depth >= MAX_SHAPE_DEPTH) { - acc.truncated = true; - return; - } - keys.sort().forEach((childKey) => { - walkShape( - value[childKey], `${path}.${sanitizeKey(childKey)}`, acc, depth + 1, childKey, - ); - }); - return; - } - - acc.paths.push(`${path}:${describeLeaf(value, key)}`); -} - -/** - * Summarizes a host event payload as shape only, never values. - * @param payload The payload passed to `trigger` - * @example - * ```js - * describeHostEventPayload({ runtimeFilters: [{ columnName: 'Region' }] }); - * // paramKeys: ['runtimeFilters'] - * // paramShape: ['runtimeFilters:array(1)', 'runtimeFilters[]:object(1)', - * // 'runtimeFilters[].columnName:string'] - * ``` - */ -export function describeHostEventPayload(payload: unknown): HostEventPayloadShape { - if (payload === undefined || payload === null) { - return { ...EMPTY_SHAPE }; - } - - try { - const acc: ShapeAccumulator = { paths: [], truncated: false }; - - if (Array.isArray(payload)) { - const firstElement = payload[0]; - walkShape(payload, ROOT_PATH, acc, 0); - return { - hasPayload: payload.length > 0, - payloadType: 'array', - paramCount: payload.length, - paramKeys: isRecord(firstElement) - ? Object.keys(firstElement).map(sanitizeKey).sort() - : [], - paramShape: acc.paths, - shapeTruncated: acc.truncated, - }; - } - - if (isRecord(payload)) { - const keys = Object.keys(payload); - keys.sort().forEach((key) => { - walkShape(payload[key], sanitizeKey(key), acc, 1, key); - }); - return { - hasPayload: keys.length > 0, - payloadType: 'object', - paramCount: keys.length, - paramKeys: keys.map(sanitizeKey), - paramShape: acc.paths, - shapeTruncated: acc.truncated, - }; - } - - return { - ...EMPTY_SHAPE, - hasPayload: true, - payloadType: 'primitive', - paramShape: [`${ROOT_PATH}:${describeLeaf(payload)}`], - }; - } catch (e) { - // A payload with a throwing getter must never break the trigger it is - // describing. - return { ...EMPTY_SHAPE, payloadType: 'unknown' }; - } -} - -/** - * The properties uploaded with a host event Mixpanel event. - */ -export interface HostEventTelemetryProps extends HostEventPayloadShape { - /** The host event that was triggered. */ - hostEvent: string; - /** The context the trigger was scoped to, or `none`. */ - contextType: string; - /** Which embed component triggered it, or `unknown`. */ - embedComponentType: string; - /** Version of the SDK the host application is on. */ - sdkVersion: string; -} - -/** - * Builds the property bag for a host event trigger. - * - * The name of the host event is a *property* here, not only a suffix on the - * Mixpanel event name, so that a single report can rank host events by usage - * instead of one report per event name. - * @param params Trigger details - * @param params.hostEvent The host event being triggered - * @param params.payload The payload passed to `trigger` - * @param params.context The context the trigger is scoped to - * @param params.embedComponentType The embed component that is triggering - */ -export function getHostEventTelemetryProps({ - hostEvent, - payload, - context, - embedComponentType, -}: { - hostEvent: HostEvent; - payload?: unknown; - context?: ContextType; - embedComponentType?: string; -}): HostEventTelemetryProps { - return { - hostEvent: String(hostEvent), - contextType: context ? String(context) : 'none', - embedComponentType: embedComponentType || 'unknown', - sdkVersion, - ...describeHostEventPayload(payload), - }; -} diff --git a/src/utils/processTrigger.ts b/src/utils/processTrigger.ts index 3f72db2e6..40c70c0d7 100644 --- a/src/utils/processTrigger.ts +++ b/src/utils/processTrigger.ts @@ -37,14 +37,6 @@ function postIframeMessage( export const TRIGGER_TIMEOUT = 30000; -/** - * Whether a settled `processTrigger` result is the timeout sentinel. - * - * `processTrigger` resolves — it does not reject — with an Error when the - * embedded app never answers, so without this check a timed-out trigger is - * indistinguishable from a successful one. - * @param value A value a `processTrigger` promise settled with - */ export const isTriggerTimeout = (value: unknown): boolean => value instanceof Error && value.message === ERROR_MESSAGE.TRIGGER_TIMED_OUT; From d3e1ee1578d7e291c77aee7377af8327b646661b Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 12:27:05 +0530 Subject: [PATCH 05/18] fix(telemetry): stop uploading the SDK's own event registrations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 Registration uploads were mostly noise: for a full-height Liveboard with two host handlers, 12 `visual-sdk-on-*` uploads went out and only 2 of them were the host application's. The other 10 were the SDK registering its own plumbing — auth, session timeout, iframe height, embed coordinates. `isRegisteredBySDK` was meant to mark those, but it did not work: - `V1Embed.on` overrode the method with a three-parameter signature and dropped the flag on the way to `TsEmbed.on`, so every internal registration on a Liveboard, Pinboard or App embed arrived marked as a host registration. - The full-height handlers in `liveboard.ts` and `app.ts` never passed it. With the flag repaired and honoured, that case goes from 12 uploads to 2, and what remains answers a real question: which embed events host applications subscribe to. Occurrence tracking is unaffected — `visual-sdk-embed-event` still reports every embed event that actually arrives. The `on()` upload also now goes through `reportEvent`, so it respects `disableSDKTracking` and stays off the critical path like the rest. Three specs asserted the old two-argument `on()` call and are updated to the four-argument form. The behaviour they cover — which handlers get registered for a full-height embed — is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/app.spec.ts | 10 +++++----- src/embed/app.ts | 11 +++++++---- src/embed/event-telemetry.spec.ts | 31 +++++++++++++++++++++++++++++++ src/embed/liveboard.spec.ts | 10 +++++----- src/embed/liveboard.ts | 16 +++++++++++----- src/embed/pinboard.spec.ts | 2 +- src/embed/ts-embed.ts | 13 +++++++++---- 7 files changed, 69 insertions(+), 24 deletions(-) diff --git a/src/embed/app.spec.ts b/src/embed/app.spec.ts index 9565e763f..28567a1ae 100644 --- a/src/embed/app.spec.ts +++ b/src/embed/app.spec.ts @@ -1813,10 +1813,10 @@ describe('App embed tests', () => { // Verify event handlers were registered await executeAfterWait(() => { - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything()); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RouteChange, expect.anything()); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedIframeCenter, expect.anything()); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.anything()); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything(), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RouteChange, expect.anything(), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedIframeCenter, expect.anything(), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.anything(), { start: false }, true); }, 100); }); @@ -2137,7 +2137,7 @@ describe('App embed tests', () => { await appEmbed.render(); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.any(Function)); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.any(Function), { start: false }, true); onSpy.mockRestore(); }); diff --git a/src/embed/app.ts b/src/embed/app.ts index 2c790dd82..caf6ba56e 100644 --- a/src/embed/app.ts +++ b/src/embed/app.ts @@ -1023,12 +1023,15 @@ export class AppEmbed extends V1Embed { viewConfig.embedComponentType = 'AppEmbed'; super(domSelector, viewConfig); if (this.viewConfig.fullHeight === true) { - this.on(EmbedEvent.RouteChange, this.setIframeHeightForNonEmbedLiveboard); - this.on(EmbedEvent.EmbedHeight, this.updateIFrameHeight); - this.on(EmbedEvent.EmbedIframeCenter, this.embedIframeCenter); + this.on( + EmbedEvent.RouteChange, + this.setIframeHeightForNonEmbedLiveboard, { start: false }, true, + ); + this.on(EmbedEvent.EmbedHeight, this.updateIFrameHeight, { start: false }, true); + this.on(EmbedEvent.EmbedIframeCenter, this.embedIframeCenter, { start: false }, true); this.on( EmbedEvent.RequestVisibleEmbedCoordinates, - this.requestVisibleEmbedCoordinatesHandler, + this.requestVisibleEmbedCoordinatesHandler, { start: false }, true, ); } } diff --git a/src/embed/event-telemetry.spec.ts b/src/embed/event-telemetry.spec.ts index dc9d64c67..b3d54b505 100644 --- a/src/embed/event-telemetry.spec.ts +++ b/src/embed/event-telemetry.spec.ts @@ -354,4 +354,35 @@ describe('Embed event telemetry', () => { expect(await embedEventUploads()).toHaveLength(0); }); + + test('reports a registration the host application made', async () => { + const embed = await renderLiveboard(); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + embed.on(EmbedEvent.Data, jest.fn()); + await flushTelemetry(); + + const uploads = uploadsOf( + mockUploadMixpanelEvent, `${MIXPANEL_EVENT.VISUAL_SDK_ON}-${EmbedEvent.Data}`, + ); + expect(uploads).toHaveLength(1); + expect(uploads[0]).toEqual( + expect.objectContaining({ + embedEvent: EmbedEvent.Data, + embedComponentType: 'LiveboardEmbed', + }), + ); + }); + + test('ignores the SDK registering its own handlers', async () => { + const embed = await renderLiveboard(); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + (embed as any).on(EmbedEvent.Data, jest.fn(), { start: false }, true); + await flushTelemetry(); + + expect(mockUploadMixpanelEvent).not.toHaveBeenCalled(); + }); }); diff --git a/src/embed/liveboard.spec.ts b/src/embed/liveboard.spec.ts index 46f84ed37..42a3135f3 100644 --- a/src/embed/liveboard.spec.ts +++ b/src/embed/liveboard.spec.ts @@ -877,7 +877,7 @@ describe('Liveboard/viz embed tests', () => { liveboardEmbed.render(); executeAfterWait(() => { - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything()); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything(), { start: false }, true); }); }); @@ -1997,10 +1997,10 @@ describe('Liveboard/viz embed tests', () => { await liveboardEmbed.render(); await executeAfterWait(() => { - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything()); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RouteChange, expect.anything()); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedIframeCenter, expect.anything()); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.anything()); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything(), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RouteChange, expect.anything(), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedIframeCenter, expect.anything(), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.anything(), { start: false }, true); }, 100); }); diff --git a/src/embed/liveboard.ts b/src/embed/liveboard.ts index 0b93bd3e4..196f10050 100644 --- a/src/embed/liveboard.ts +++ b/src/embed/liveboard.ts @@ -677,10 +677,16 @@ export class LiveboardEmbed extends V1Embed { 'Using full height with vizId might lead to unexpected behavior.'); } - this.on(EmbedEvent.RouteChange, this.setIframeHeightForNonEmbedLiveboard); - this.on(EmbedEvent.EmbedHeight, this.updateIFrameHeight); - this.on(EmbedEvent.EmbedIframeCenter, this.embedIframeCenter); - this.on(EmbedEvent.RequestVisibleEmbedCoordinates, this.requestVisibleEmbedCoordinatesHandler); + this.on( + EmbedEvent.RouteChange, + this.setIframeHeightForNonEmbedLiveboard, { start: false }, true, + ); + this.on(EmbedEvent.EmbedHeight, this.updateIFrameHeight, { start: false }, true); + this.on(EmbedEvent.EmbedIframeCenter, this.embedIframeCenter, { start: false }, true); + this.on( + EmbedEvent.RequestVisibleEmbedCoordinates, + this.requestVisibleEmbedCoordinatesHandler, { start: false }, true, + ); } } @@ -1091,7 +1097,7 @@ export class LiveboardEmbed extends V1Embed { this.hostElement.style.position = 'relative'; this.on(EmbedEvent.Data, () => { previewDiv.remove(); - }); + }, { start: false }, true); } catch (error) { console.error('Error fetching preview', error); } diff --git a/src/embed/pinboard.spec.ts b/src/embed/pinboard.spec.ts index b45212fe1..77b6f4c89 100644 --- a/src/embed/pinboard.spec.ts +++ b/src/embed/pinboard.spec.ts @@ -241,7 +241,7 @@ describe('Pinboard/viz embed tests', () => { pinboardEmbed.render(); executeAfterWait(() => { - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything()); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything(), { start: false }, true); }); }); }); diff --git a/src/embed/ts-embed.ts b/src/embed/ts-embed.ts index aa7b157a6..299fd5fdf 100644 --- a/src/embed/ts-embed.ts +++ b/src/embed/ts-embed.ts @@ -1536,9 +1536,13 @@ export class TsEmbed { options: MessageOptions = { start: false }, isRegisteredBySDK = false, ): typeof TsEmbed.prototype { - uploadMixpanelEvent(`${MIXPANEL_EVENT.VISUAL_SDK_ON}-${messageType}`, { - isRegisteredBySDK, - }); + if (!isRegisteredBySDK) { + reportEvent(`${MIXPANEL_EVENT.VISUAL_SDK_ON}-${messageType}`, () => ({ + embedEvent: String(messageType), + embedComponentType: this.viewConfig?.embedComponentType || 'unknown', + sdkVersion: version, + })); + } if (this.isRendered) { logger.warn('Please register event handlers before calling render'); } @@ -2380,9 +2384,10 @@ export class V1Embed extends TsEmbed { messageType: EmbedEvent, callback: MessageCallback, options: MessageOptions = { start: false }, + isRegisteredBySDK = false, ): typeof TsEmbed.prototype { const eventType = this.getCompatibleEventType(messageType); - return super.on(eventType, callback, options); + return super.on(eventType, callback, options, isRegisteredBySDK); } /** From aea8ca14b857a39a25b4b4fe67781e8a2b58888b Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 13:01:18 +0530 Subject: [PATCH 06/18] fix(telemetry): count only the handlers a dispatch ran, describe the payload first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 Review catches on the embed event hook. `handlerCount` used every handler registered for the event type, not the ones this dispatch invoked. `MessageOptions.start` is a supported way to subscribe to start and end separately, so an embed registering one of each for the same event reported `handlerCount: 2` on both dispatches when one handler ran each time — overstating the "is anyone listening" signal it exists to give. The payload is now described before the handlers run, rather than inside the deferred upload. Handlers receive the same object by reference, so one that normalises a field in place would otherwise have changed the shape being reported. Describing first also means the shape is what was dispatched. Only the upload stays deferred, which is the part that must not block; the walk itself is bounded to 20 key paths and 3 levels. Nothing is described at all when tracking is off, so the checks stay in front of the work. Also `parameters?.vizId` when building the "no answer found" message. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/event-telemetry.spec.ts | 19 ++++++++++ .../hostEventClient/host-event-client.ts | 2 +- src/embed/ts-embed.ts | 38 ++++++++++++------- src/utils/eventTelemetry.spec.ts | 4 -- src/utils/eventTelemetry.ts | 12 ++---- 5 files changed, 48 insertions(+), 27 deletions(-) diff --git a/src/embed/event-telemetry.spec.ts b/src/embed/event-telemetry.spec.ts index b3d54b505..c8a2feb16 100644 --- a/src/embed/event-telemetry.spec.ts +++ b/src/embed/event-telemetry.spec.ts @@ -317,6 +317,25 @@ describe('Embed event telemetry', () => { }); }); + test('counts only the handlers this dispatch actually ran', async () => { + const embed = await renderLiveboard(); + await flushTelemetry(); + embed.on(EmbedEvent.Data, jest.fn(), { start: true }); + embed.on(EmbedEvent.Data, jest.fn()); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + (embed as any).executeCallbacks(EmbedEvent.Data, { status: 'end' }); + const endUploads = await embedEventUploads(); + expect(endUploads[0].handlerCount).toBe(1); + + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + (embed as any).executeCallbacks(EmbedEvent.Data, { status: 'start' }); + const startUploads = await embedEventUploads(); + expect(startUploads[0].handlerCount).toBe(1); + }); + test('reports an embed event nobody is listening for', async () => { const embed = await renderLiveboard(); await flushTelemetry(); diff --git a/src/embed/hostEventClient/host-event-client.ts b/src/embed/hostEventClient/host-event-client.ts index c3f9852f3..e67c01afa 100644 --- a/src/embed/hostEventClient/host-event-client.ts +++ b/src/embed/hostEventClient/host-event-client.ts @@ -100,7 +100,7 @@ export class HostEventClient { const response = raw?.find?.((r) => r.error || r.value); if (!response) { - const error = `No answer found${parameters.vizId ? ` for vizId: ${parameters.vizId}` : ''}.`; + const error = `No answer found${parameters?.vizId ? ` for vizId: ${parameters.vizId}` : ''}.`; // A timeout arrives here as a missing response, because // processTrigger resolves with an Error rather than rejecting. The // thrown shape stays as it was; the flag lets telemetry tell an diff --git a/src/embed/ts-embed.ts b/src/embed/ts-embed.ts index 299fd5fdf..8adfaf037 100644 --- a/src/embed/ts-embed.ts +++ b/src/embed/ts-embed.ts @@ -76,6 +76,7 @@ import { getHostEventTelemetryProps, HostEventRoute, HostEventStatus, + isTelemetryEnabled, reportEvent, } from '../utils/eventTelemetry'; import { isTriggerTimeout } from '../utils/processTrigger'; @@ -1434,6 +1435,14 @@ export class TsEmbed { const allHandlers = this.eventHandlerMap.get(EmbedEvent.ALL) || []; const callbacks = [...eventHandlers, ...allHandlers]; const dataStatus = data?.status || embedEventStatus.END; + const telemetryProps = isTelemetryEnabled() + ? getEmbedEventTelemetryProps({ + embedEvent: eventType, + payload: data, + embedComponentType: this.viewConfig?.embedComponentType, + }) + : null; + let invokedHandlers = 0; callbacks.forEach((callbackObj) => { if ( // When start status is true it trigger only start releated @@ -1443,16 +1452,17 @@ export class TsEmbed { // payload (!callbackObj.options.start && dataStatus === embedEventStatus.END) ) { + invokedHandlers += 1; const responder = this.createEmbedEventResponder(eventPort, eventType); callbackObj.callback(data, responder); } }); - reportEvent(MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT, () => getEmbedEventTelemetryProps({ - embedEvent: eventType, - payload: data, - embedComponentType: this.viewConfig?.embedComponentType, - handlerCount: callbacks.length, - })); + if (telemetryProps) { + reportEvent(MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT, { + ...telemetryProps, + handlerCount: invokedHandlers, + }); + } } /** @@ -1537,11 +1547,11 @@ export class TsEmbed { isRegisteredBySDK = false, ): typeof TsEmbed.prototype { if (!isRegisteredBySDK) { - reportEvent(`${MIXPANEL_EVENT.VISUAL_SDK_ON}-${messageType}`, () => ({ + reportEvent(`${MIXPANEL_EVENT.VISUAL_SDK_ON}-${messageType}`, { embedEvent: String(messageType), embedComponentType: this.viewConfig?.embedComponentType || 'unknown', sdkVersion: version, - })); + }); } if (this.isRendered) { logger.warn('Please register event handlers before calling render'); @@ -1700,20 +1710,22 @@ export class TsEmbed { const triggerStartedAt = Date.now(); let route: HostEventRoute | undefined; const reportHostEvent = (status: HostEventStatus, errorCode?: EmbedErrorCodes) => { - const durationMs = Date.now() - triggerStartedAt; - const buildProps = () => getHostEventTelemetryProps({ + if (!isTelemetryEnabled()) { + return; + } + const props = getHostEventTelemetryProps({ hostEvent: messageType, payload: data, context, embedComponentType: this.viewConfig?.embedComponentType, status, - durationMs, + durationMs: Date.now() - triggerStartedAt, route, errorCode, }); - reportEvent(`${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`, buildProps); + reportEvent(`${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`, props); if (status === 'timed-out') { - reportEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT_NO_RESPONSE, buildProps); + reportEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT_NO_RESPONSE, props); } }; diff --git a/src/utils/eventTelemetry.spec.ts b/src/utils/eventTelemetry.spec.ts index 48a39fb82..14c78ba71 100644 --- a/src/utils/eventTelemetry.spec.ts +++ b/src/utils/eventTelemetry.spec.ts @@ -260,14 +260,12 @@ describe('getEmbedEventTelemetryProps', () => { data: { columnNames: ['Region'], rows: [['west', 100]] }, }, embedComponentType: 'LiveboardEmbed', - handlerCount: 2, }); expect(props).toEqual( expect.objectContaining({ embedEvent: EmbedEvent.Data, embedComponentType: 'LiveboardEmbed', eventStatus: 'end', - handlerCount: 2, sdkVersion: version, }), ); @@ -281,7 +279,6 @@ describe('getEmbedEventTelemetryProps', () => { data: { columnNames: ['Region'], rows: [['west', 100]] }, answerName: 'Quarterly revenue', }, - handlerCount: 0, }); const serialized = JSON.stringify(props); ['Region', 'west', 'Quarterly revenue'].forEach((value) => { @@ -297,7 +294,6 @@ describe('getEmbedEventTelemetryProps', () => { const props = getEmbedEventTelemetryProps({ embedEvent: EmbedEvent.Data, payload: wide, - handlerCount: 1, }); expect(props.paramShape).toHaveLength(MAX_EMBED_SHAPE_PATHS); expect(props.shapeTruncated).toBe(true); diff --git a/src/utils/eventTelemetry.ts b/src/utils/eventTelemetry.ts index 8ef4af676..f964597a3 100644 --- a/src/utils/eventTelemetry.ts +++ b/src/utils/eventTelemetry.ts @@ -212,16 +212,13 @@ const runWhenIdle = (work: () => void): void => { setTimeout(work, 0); }; -export const reportEvent = ( - eventId: string, - buildProps: () => Record, -): void => { +export const reportEvent = (eventId: string, props: Record): void => { if (!isTelemetryEnabled()) { return; } runWhenIdle(() => { try { - uploadMixpanelEvent(eventId, buildProps()); + uploadMixpanelEvent(eventId, props); } catch (e) { logger.debug('Could not report telemetry for', eventId, e); } @@ -262,17 +259,14 @@ export const getEmbedEventTelemetryProps = ({ embedEvent, payload, embedComponentType, - handlerCount, }: { embedEvent: EmbedEvent; payload?: any; embedComponentType?: string; - handlerCount: number; -}): EmbedEventTelemetryProps => ({ +}): Omit => ({ embedEvent: String(embedEvent), embedComponentType: embedComponentType || 'unknown', sdkVersion, eventStatus: payload?.status ? String(payload.status) : 'none', - handlerCount, ...describePayload(payload, MAX_EMBED_SHAPE_PATHS), }); From d711b2eba5035e5ba0415859dc934ff7cfe9e548 Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 13:16:42 +0530 Subject: [PATCH 07/18] feat(telemetry): two response-aware events, one per direction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 Both directions can be responded to, and neither response was being captured. A trigger resolves with whatever the embedded app sends back, and an embed event hands the host application a responder — `on(ApiIntercept, responder)` — so the interesting part of an interception is the answer, not the question. Two events now tell the whole story of one exchange: `visual-sdk-host-event`, once per trigger: the host event, its parameter shape, the dispatch route, the outcome, how long it took, whether the app responded, and the shape of what came back. `visual-sdk-embed-event`, once per embed event: the event, its payload shape, how many handlers ran, whether the event could be responded to at all, whether the host application responded, how long that took, and the shape of the response. `canRespond` comes from the presence of a MessagePort, which is what makes a response possible, so only events that can actually be answered are waited on — five seconds, then the exchange is recorded as unanswered. Everything else is reported as soon as the handlers have run. Response shapes go through the same summariser as payloads, so a response reports `responseKeys` and `responseShape` and never a value. `visual-sdk-trigger-` goes back to exactly what it was before this branch: fired at call time, no properties. The new event carries the data instead, so no existing report changes meaning. That also retires `visual-sdk-host-event-no-response`, whose only signal is now `responded: false` on the story. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/event-telemetry.spec.ts | 163 ++++++++++++++++++++++++------ src/embed/ts-embed.ts | 84 +++++++++++---- src/mixpanel-service.spec.ts | 2 +- src/mixpanel-service.ts | 2 +- src/utils/eventTelemetry.ts | 23 ++++- 5 files changed, 219 insertions(+), 55 deletions(-) diff --git a/src/embed/event-telemetry.spec.ts b/src/embed/event-telemetry.spec.ts index c8a2feb16..1c9673309 100644 --- a/src/embed/event-telemetry.spec.ts +++ b/src/embed/event-telemetry.spec.ts @@ -9,6 +9,7 @@ import * as authInstance from '../auth'; import * as mixpanelInstance from '../mixpanel-service'; import { MIXPANEL_EVENT } from '../mixpanel-service'; import * as processTriggerInstance from '../utils/processTrigger'; +import { RESPONSE_WAIT_MS } from '../utils/eventTelemetry'; const flushTelemetry = () => new Promise((resolve) => setTimeout(resolve, 5)); @@ -49,12 +50,9 @@ describe('Host event telemetry', () => { jest.restoreAllMocks(); }); - const triggerProps = async (hostEvent: HostEvent) => { + const triggerProps = async () => { await flushTelemetry(); - const uploads = uploadsOf( - mockUploadMixpanelEvent, - `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${hostEvent}`, - ); + const uploads = uploadsOf(mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT); expect(uploads).toHaveLength(1); return uploads[0]; }; @@ -76,15 +74,15 @@ describe('Host event telemetry', () => { ); }; - test('enriches the existing per-event upload instead of adding another', async () => { + test('tells the whole story of a trigger, including what came back', async () => { + mockProcessTrigger.mockResolvedValue({ session: 'ok', answerId: 'a-1' }); const embed = await renderLiveboard(); await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); - await flushTelemetry(); - expect(await triggerProps(HostEvent.DownloadAsCsv)).toEqual( + expect(await triggerProps()).toEqual( expect.objectContaining({ hostEvent: HostEvent.DownloadAsCsv, embedComponentType: 'LiveboardEmbed', @@ -96,11 +94,39 @@ describe('Host event telemetry', () => { status: 'success', route: 'legacy', durationMs: expect.any(Number), + responded: true, + responseType: 'object', + responseKeys: ['answerId', 'session'], + responseShape: ['answerId:string', 'session:string'], }), ); - expect(mockUploadMixpanelEvent.mock.calls.map(([id]) => id)).toEqual([ + }); + + test('leaves the legacy per-event upload exactly as it was', async () => { + const embed = await renderLiveboard(); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + + expect(mockUploadMixpanelEvent).toHaveBeenCalledWith( `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${HostEvent.DownloadAsCsv}`, - ]); + ); + }); + + test('never reports a response value', async () => { + mockProcessTrigger.mockResolvedValue({ answerName: 'Quarterly revenue', rows: [['west']] }); + const embed = await renderLiveboard(); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + + const props = await triggerProps(); + expect(props.responseKeys).toEqual(['answerName', 'rows']); + ['Quarterly revenue', 'west'].forEach((value) => { + expect(JSON.stringify(props)).not.toContain(value); + }); }); test('reports parameter names and enum members, never customer values', async () => { @@ -116,7 +142,7 @@ describe('Host event telemetry', () => { const serialized = JSON.stringify(mockUploadMixpanelEvent.mock.calls); ['Region', 'west'].forEach((value) => expect(serialized).not.toContain(value)); - const props = await triggerProps(HostEvent.UpdateRuntimeFilters); + const props = await triggerProps(); expect(props.paramKeys).toEqual(['columnName', 'operator', 'values']); expect(props.paramShape).toEqual( expect.arrayContaining([ @@ -127,24 +153,18 @@ describe('Host event telemetry', () => { ); }); - test('sends a no-response event when the app never answers', async () => { + test('records that the app never answered', async () => { mockProcessTrigger.mockResolvedValue(new Error(ERROR_MESSAGE.TRIGGER_TIMED_OUT)); const embed = await renderLiveboard(); await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); - await flushTelemetry(); - expect((await triggerProps(HostEvent.DownloadAsCsv)).status).toBe('timed-out'); - const noResponse = uploadsOf( - mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT_NO_RESPONSE, - ); - expect(noResponse).toHaveLength(1); - expect(noResponse[0]).toEqual( + expect(await triggerProps()).toEqual( expect.objectContaining({ - hostEvent: HostEvent.DownloadAsCsv, status: 'timed-out', + responded: false, durationMs: expect.any(Number), }), ); @@ -159,7 +179,7 @@ describe('Host event telemetry', () => { await expect(embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' })).rejects.toThrow(); await flushTelemetry(); - const props = await triggerProps(HostEvent.DownloadAsCsv); + const props = await triggerProps(); expect(props.status).toBe('error'); expect(JSON.stringify(props)).not.toContain('4c8a1b2e'); }); @@ -173,7 +193,7 @@ describe('Host event telemetry', () => { await embed.trigger(HostEvent.GetTabs, {}); await flushTelemetry(); - expect((await triggerProps(HostEvent.GetTabs)).route).toBe('ui-passthrough'); + expect((await triggerProps()).route).toBe('ui-passthrough'); }); test('reports the legacy route when the app lacks the passthrough key', async () => { @@ -185,7 +205,7 @@ describe('Host event telemetry', () => { await embed.trigger(HostEvent.GetTabs, {}); await flushTelemetry(); - expect((await triggerProps(HostEvent.GetTabs)).route).toBe('legacy'); + expect((await triggerProps()).route).toBe('legacy'); }); test('reports the custom-handler route for a setter with custom logic', async () => { @@ -200,7 +220,7 @@ describe('Host event telemetry', () => { }); await flushTelemetry(); - expect(await triggerProps(HostEvent.Pin)).toEqual( + expect(await triggerProps()).toEqual( expect.objectContaining({ route: 'custom-handler', paramKeys: ['liveboardId', 'newVizName'], @@ -225,10 +245,9 @@ describe('Host event telemetry', () => { ).rejects.toBeDefined(); await flushTelemetry(); - expect((await triggerProps(HostEvent.Pin)).status).toBe('timed-out'); - expect( - uploadsOf(mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT_NO_RESPONSE), - ).toHaveLength(1); + const props = await triggerProps(); + expect(props.status).toBe('timed-out'); + expect(props.responded).toBe(false); }); test('reports a trigger called before render', async () => { @@ -247,7 +266,7 @@ describe('Host event telemetry', () => { await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); await flushTelemetry(); - expect(await triggerProps(HostEvent.DownloadAsCsv)).toEqual( + expect(await triggerProps()).toEqual( expect.objectContaining({ status: 'render-not-called', errorCode: EmbedErrorCodes.RENDER_NOT_CALLED, @@ -255,7 +274,7 @@ describe('Host event telemetry', () => { ); }); - test('uploads nothing at all when the host application disabled tracking', async () => { + test('builds no telemetry when the host application disabled tracking', async () => { const embed = await renderLiveboard({ disableSDKTracking: true }); await flushTelemetry(); mockUploadMixpanelEvent.mockClear(); @@ -263,7 +282,12 @@ describe('Host event telemetry', () => { await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); await flushTelemetry(); - expect(mockUploadMixpanelEvent).not.toHaveBeenCalled(); + expect( + uploadsOf(mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT), + ).toHaveLength(0); + expect(mockUploadMixpanelEvent.mock.calls.map(([id]) => id)).toEqual([ + `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${HostEvent.DownloadAsCsv}`, + ]); }); }); @@ -336,6 +360,83 @@ describe('Embed event telemetry', () => { expect(startUploads[0].handlerCount).toBe(1); }); + test('tells the whole story when the host application responds', async () => { + const embed = await renderLiveboard(); + await flushTelemetry(); + embed.on(EmbedEvent.ApiIntercept, (_data: any, responder: any) => { + responder({ allow: true, answerName: 'Quarterly revenue' }); + }); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + (embed as any).executeCallbacks( + EmbedEvent.ApiIntercept, + { status: 'end', url: '/api/rest/2.0/metadata/search' }, + { postMessage: jest.fn() }, + ); + + const uploads = await embedEventUploads(); + expect(uploads).toHaveLength(1); + expect(uploads[0]).toEqual( + expect.objectContaining({ + embedEvent: EmbedEvent.ApiIntercept, + canRespond: true, + responded: true, + handlerCount: 1, + responseTimeMs: expect.any(Number), + }), + ); + expect(uploads[0].responseKeys).toEqual(['allow', 'answerName']); + expect(JSON.stringify(uploads[0])).not.toContain('Quarterly revenue'); + }); + + test('records that the host application never responded', async () => { + const embed = await renderLiveboard(); + await flushTelemetry(); + embed.on(EmbedEvent.ApiIntercept, jest.fn()); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + jest.useFakeTimers(); + try { + (embed as any).executeCallbacks( + EmbedEvent.ApiIntercept, + { status: 'end' }, + { postMessage: jest.fn() }, + ); + jest.advanceTimersByTime(1000); + expect( + uploadsOf(mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT), + ).toHaveLength(0); + + jest.advanceTimersByTime(RESPONSE_WAIT_MS + 100); + const uploads = uploadsOf( + mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT, + ); + expect(uploads).toHaveLength(1); + expect(uploads[0]).toEqual( + expect.objectContaining({ canRespond: true, responded: false }), + ); + } finally { + jest.useRealTimers(); + } + }); + + test('does not wait for a response an event cannot receive', async () => { + const embed = await renderLiveboard(); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + (embed as any).executeCallbacks(EmbedEvent.Data, { status: 'end' }); + + const uploads = await embedEventUploads(); + expect(uploads).toHaveLength(1); + expect(uploads[0]).toEqual( + expect.objectContaining({ canRespond: false, responded: false }), + ); + expect('responseTimeMs' in uploads[0]).toBe(false); + }); + test('reports an embed event nobody is listening for', async () => { const embed = await renderLiveboard(); await flushTelemetry(); diff --git a/src/embed/ts-embed.ts b/src/embed/ts-embed.ts index 8adfaf037..8d600a52d 100644 --- a/src/embed/ts-embed.ts +++ b/src/embed/ts-embed.ts @@ -73,11 +73,14 @@ import { import { uploadMixpanelEvent, MIXPANEL_EVENT } from '../mixpanel-service'; import { getEmbedEventTelemetryProps, + describeResponse, getHostEventTelemetryProps, HostEventRoute, HostEventStatus, isTelemetryEnabled, + MAX_EMBED_SHAPE_PATHS, reportEvent, + RESPONSE_WAIT_MS, } from '../utils/eventTelemetry'; import { isTriggerTimeout } from '../utils/processTrigger'; import { processEventData, processAuthFailure } from '../utils/processData'; @@ -1442,7 +1445,29 @@ export class TsEmbed { embedComponentType: this.viewConfig?.embedComponentType, }) : null; + const dispatchedAt = Date.now(); + const canRespond = !!eventPort; + let reported = false; let invokedHandlers = 0; + const reportEmbedEvent = (response?: { payload: unknown; at: number }) => { + if (reported || !telemetryProps) { + return; + } + reported = true; + reportEvent(MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT, { + ...telemetryProps, + handlerCount: invokedHandlers, + canRespond, + responded: !!response, + ...(response + ? { + responseTimeMs: response.at - dispatchedAt, + ...describeResponse(response.payload, MAX_EMBED_SHAPE_PATHS), + } + : {}), + }); + }; + callbacks.forEach((callbackObj) => { if ( // When start status is true it trigger only start releated @@ -1454,15 +1479,21 @@ export class TsEmbed { ) { invokedHandlers += 1; const responder = this.createEmbedEventResponder(eventPort, eventType); - callbackObj.callback(data, responder); + callbackObj.callback(data, (payload: any) => { + reportEmbedEvent({ payload, at: Date.now() }); + return responder(payload); + }); } }); - if (telemetryProps) { - reportEvent(MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT, { - ...telemetryProps, - handlerCount: invokedHandlers, - }); + + if (!telemetryProps) { + return; + } + if (canRespond && !reported) { + setTimeout(() => reportEmbedEvent(), RESPONSE_WAIT_MS); + return; } + reportEmbedEvent(); } /** @@ -1709,26 +1740,33 @@ export class TsEmbed { ): Promise> { const triggerStartedAt = Date.now(); let route: HostEventRoute | undefined; - const reportHostEvent = (status: HostEventStatus, errorCode?: EmbedErrorCodes) => { + const reportHostEvent = ( + status: HostEventStatus, + errorCode?: EmbedErrorCodes, + response?: unknown, + ) => { if (!isTelemetryEnabled()) { return; } - const props = getHostEventTelemetryProps({ - hostEvent: messageType, - payload: data, - context, - embedComponentType: this.viewConfig?.embedComponentType, - status, - durationMs: Date.now() - triggerStartedAt, - route, - errorCode, + const responded = status === 'success'; + reportEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT, { + ...getHostEventTelemetryProps({ + hostEvent: messageType, + payload: data, + context, + embedComponentType: this.viewConfig?.embedComponentType, + status, + durationMs: Date.now() - triggerStartedAt, + route, + errorCode, + }), + responded, + ...(responded ? describeResponse(response) : {}), }); - reportEvent(`${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`, props); - if (status === 'timed-out') { - reportEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT_NO_RESPONSE, props); - } }; + uploadMixpanelEvent(`${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`); + if (!this.isRendered) { reportHostEvent('render-not-called', EmbedErrorCodes.RENDER_NOT_CALLED); this.handleError({ @@ -1767,7 +1805,11 @@ export class TsEmbed { route = dispatchRoute; }) .then((response) => { - reportHostEvent(isTriggerTimeout(response) ? 'timed-out' : 'success'); + if (isTriggerTimeout(response)) { + reportHostEvent('timed-out'); + } else { + reportHostEvent('success', undefined, response); + } return response; }) .catch( diff --git a/src/mixpanel-service.spec.ts b/src/mixpanel-service.spec.ts index c421b2877..5f8c35ef0 100644 --- a/src/mixpanel-service.spec.ts +++ b/src/mixpanel-service.spec.ts @@ -87,7 +87,7 @@ describe('Unit test for mixpanel', () => { test('caps the pre-init queue, so tracking left uninitialized cannot grow it', () => { testResetMixpanel(); for (let i = 0; i < MAX_QUEUED_EVENTS + 50; i += 1) { - uploadMixpanelEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT_NO_RESPONSE, { index: i }); + uploadMixpanelEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT, { index: i }); } const sessionInfo = { mixpanelToken: 'abc123', diff --git a/src/mixpanel-service.ts b/src/mixpanel-service.ts index 61c11d195..b1a84308a 100644 --- a/src/mixpanel-service.ts +++ b/src/mixpanel-service.ts @@ -22,7 +22,7 @@ export const MIXPANEL_EVENT = { VISUAL_SDK_RENDER_COMPLETE: 'visual-sdk-render-complete', VISUAL_SDK_RENDER_FAILED: 'visual-sdk-render-failed', VISUAL_SDK_TRIGGER: 'visual-sdk-trigger', - VISUAL_SDK_HOST_EVENT_NO_RESPONSE: 'visual-sdk-host-event-no-response', + VISUAL_SDK_HOST_EVENT: 'visual-sdk-host-event', VISUAL_SDK_EMBED_EVENT: 'visual-sdk-embed-event', VISUAL_SDK_ON: 'visual-sdk-on', VISUAL_SDK_IFRAME_LOAD_PERFORMANCE: 'visual-sdk-iframe-load-performance', diff --git a/src/utils/eventTelemetry.ts b/src/utils/eventTelemetry.ts index f964597a3..0b4aa306d 100644 --- a/src/utils/eventTelemetry.ts +++ b/src/utils/eventTelemetry.ts @@ -11,6 +11,7 @@ export const MAX_EMBED_SHAPE_PATHS = 20; export const MAX_KEY_LENGTH = 40; export const REDACTED_KEY = 'redactedKey'; export const IDLE_TIMEOUT = 2000; +export const RESPONSE_WAIT_MS = 5000; const ROOT_PATH = 'payload'; const SAFE_KEY_PATTERN = /^[A-Za-z_$][A-Za-z0-9_$]*$/; @@ -64,6 +65,14 @@ export interface EmbedEventTelemetryProps extends PayloadShape { sdkVersion: string; eventStatus: string; handlerCount: number; + canRespond: boolean; + responded: boolean; +} + +export interface ResponseShape { + responseType: PayloadShape['payloadType']; + responseKeys: string[]; + responseShape: string[]; } interface ShapeAccumulator { @@ -201,6 +210,18 @@ export const describePayload = ( } }; +export const describeResponse = ( + payload: unknown, + maxPaths = MAX_SHAPE_PATHS, +): ResponseShape => { + const shape = describePayload(payload, maxPaths); + return { + responseType: shape.payloadType, + responseKeys: shape.paramKeys, + responseShape: shape.paramShape, + }; +}; + export const isTelemetryEnabled = (): boolean => !getEmbedConfig()?.disableSDKTracking; const runWhenIdle = (work: () => void): void => { @@ -263,7 +284,7 @@ export const getEmbedEventTelemetryProps = ({ embedEvent: EmbedEvent; payload?: any; embedComponentType?: string; -}): Omit => ({ +}): Omit => ({ embedEvent: String(embedEvent), embedComponentType: embedComponentType || 'unknown', sdkVersion, From bb33dd47e62b1d6de5b5b7f6c420487a157af8f1 Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 13:54:20 +0530 Subject: [PATCH 08/18] fix(telemetry): keep handlerCount accurate when a handler responds inline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 Review catch, and a regression from the commit that made handlerCount accurate in the first place. Reporting from inside the responder meant a handler that answers synchronously — the documented ApiIntercept and OnBeforeGetVizDataIntercept pattern — uploaded the event mid-loop, freezing handlerCount at that handler's position and turning the post-loop report into a no-op. Two intercept handlers where the first answers inline reported 1, not 2. The responder now records the response and reports only once the dispatch loop has finished, so the count is final either way: a synchronous answer is reported immediately after the loop, an asynchronous one when it arrives, and an absent one after the response window. Only the first response is recorded, which matches the port: the first answer is the one the embedded app receives. Regression test verified to fail without the fix. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/event-telemetry.spec.ts | 23 +++++++++++++++++++++++ src/embed/ts-embed.ts | 19 +++++++++++++++---- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/embed/event-telemetry.spec.ts b/src/embed/event-telemetry.spec.ts index 1c9673309..5868dc8f0 100644 --- a/src/embed/event-telemetry.spec.ts +++ b/src/embed/event-telemetry.spec.ts @@ -390,6 +390,29 @@ describe('Embed event telemetry', () => { expect(JSON.stringify(uploads[0])).not.toContain('Quarterly revenue'); }); + test('counts every handler even when an early one responds synchronously', async () => { + const embed = await renderLiveboard(); + await flushTelemetry(); + embed.on(EmbedEvent.ApiIntercept, (_data: any, responder: any) => { + responder({ allow: true }); + }); + embed.on(EmbedEvent.ApiIntercept, jest.fn()); + await flushTelemetry(); + mockUploadMixpanelEvent.mockClear(); + + (embed as any).executeCallbacks( + EmbedEvent.ApiIntercept, + { status: 'end' }, + { postMessage: jest.fn() }, + ); + + const uploads = await embedEventUploads(); + expect(uploads).toHaveLength(1); + expect(uploads[0]).toEqual( + expect.objectContaining({ handlerCount: 2, responded: true }), + ); + }); + test('records that the host application never responded', async () => { const embed = await renderLiveboard(); await flushTelemetry(); diff --git a/src/embed/ts-embed.ts b/src/embed/ts-embed.ts index 8d600a52d..bab16360a 100644 --- a/src/embed/ts-embed.ts +++ b/src/embed/ts-embed.ts @@ -1448,8 +1448,10 @@ export class TsEmbed { const dispatchedAt = Date.now(); const canRespond = !!eventPort; let reported = false; + let dispatchComplete = false; let invokedHandlers = 0; - const reportEmbedEvent = (response?: { payload: unknown; at: number }) => { + let response: { payload: unknown; at: number } | undefined; + const reportEmbedEvent = () => { if (reported || !telemetryProps) { return; } @@ -1467,6 +1469,14 @@ export class TsEmbed { : {}), }); }; + // A handler can respond while the loop is still running, so the + // response is recorded now and reported once the count is final. + const recordResponse = (payload: unknown) => { + response = response || { payload, at: Date.now() }; + if (dispatchComplete) { + reportEmbedEvent(); + } + }; callbacks.forEach((callbackObj) => { if ( @@ -1480,17 +1490,18 @@ export class TsEmbed { invokedHandlers += 1; const responder = this.createEmbedEventResponder(eventPort, eventType); callbackObj.callback(data, (payload: any) => { - reportEmbedEvent({ payload, at: Date.now() }); + recordResponse(payload); return responder(payload); }); } }); + dispatchComplete = true; if (!telemetryProps) { return; } - if (canRespond && !reported) { - setTimeout(() => reportEmbedEvent(), RESPONSE_WAIT_MS); + if (canRespond && !response) { + setTimeout(reportEmbedEvent, RESPONSE_WAIT_MS); return; } reportEmbedEvent(); From 84fe01752cb81dfbbe9c845625e88cc3cdff14d7 Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 14:08:03 +0530 Subject: [PATCH 09/18] =?UTF-8?q?refactor(telemetry):=20reduce=20to=20PR?= =?UTF-8?q?=201=20=E2=80=94=20host=20event=20parameters=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 Splitting this work up, and this branch keeps only the first slice: which host event is triggered and which parameters it uses. No existing SDK behaviour changes. `visual-sdk-trigger-` keeps its name, its timing and its call site; it just carries properties now, where before it carried none. One existing file is touched, by ten lines. Everything else is removed from this branch and preserved at bb33dd47 for the follow-up PRs: the outcome and duration of a trigger, the response story for both directions, embed event tracking, the registration-noise work, the custom-handler timeout fix, and the pre-init queue cap. Dropped outright, per review: `isRegisteredBySDK`. The flag was broken, and propagating it meant `{ start: false }, true` at every internal call site to reach a positional parameter past a default. Whether a host application listens to an embed event is better answered by counting the handlers a dispatch actually ran, which needs no flag and belongs with the embed event work anyway. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/app.spec.ts | 10 +- src/embed/app.ts | 11 +- src/embed/event-telemetry.spec.ts | 531 ------------------ src/embed/host-event-telemetry.spec.ts | 95 ++++ .../hostEventClient/host-event-client.ts | 43 +- src/embed/liveboard.spec.ts | 10 +- src/embed/liveboard.ts | 16 +- src/embed/pinboard.spec.ts | 2 +- src/embed/ts-embed.ts | 175 +----- src/mixpanel-service.spec.ts | 15 - src/mixpanel-service.ts | 9 +- ...try.spec.ts => hostEventTelemetry.spec.ts} | 0 ...ventTelemetry.ts => hostEventTelemetry.ts} | 0 src/utils/processTrigger.ts | 3 - 14 files changed, 155 insertions(+), 765 deletions(-) delete mode 100644 src/embed/event-telemetry.spec.ts create mode 100644 src/embed/host-event-telemetry.spec.ts rename src/utils/{eventTelemetry.spec.ts => hostEventTelemetry.spec.ts} (100%) rename src/utils/{eventTelemetry.ts => hostEventTelemetry.ts} (100%) diff --git a/src/embed/app.spec.ts b/src/embed/app.spec.ts index 28567a1ae..9565e763f 100644 --- a/src/embed/app.spec.ts +++ b/src/embed/app.spec.ts @@ -1813,10 +1813,10 @@ describe('App embed tests', () => { // Verify event handlers were registered await executeAfterWait(() => { - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything(), { start: false }, true); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RouteChange, expect.anything(), { start: false }, true); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedIframeCenter, expect.anything(), { start: false }, true); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.anything(), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything()); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RouteChange, expect.anything()); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedIframeCenter, expect.anything()); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.anything()); }, 100); }); @@ -2137,7 +2137,7 @@ describe('App embed tests', () => { await appEmbed.render(); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.any(Function), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.any(Function)); onSpy.mockRestore(); }); diff --git a/src/embed/app.ts b/src/embed/app.ts index caf6ba56e..2c790dd82 100644 --- a/src/embed/app.ts +++ b/src/embed/app.ts @@ -1023,15 +1023,12 @@ export class AppEmbed extends V1Embed { viewConfig.embedComponentType = 'AppEmbed'; super(domSelector, viewConfig); if (this.viewConfig.fullHeight === true) { - this.on( - EmbedEvent.RouteChange, - this.setIframeHeightForNonEmbedLiveboard, { start: false }, true, - ); - this.on(EmbedEvent.EmbedHeight, this.updateIFrameHeight, { start: false }, true); - this.on(EmbedEvent.EmbedIframeCenter, this.embedIframeCenter, { start: false }, true); + this.on(EmbedEvent.RouteChange, this.setIframeHeightForNonEmbedLiveboard); + this.on(EmbedEvent.EmbedHeight, this.updateIFrameHeight); + this.on(EmbedEvent.EmbedIframeCenter, this.embedIframeCenter); this.on( EmbedEvent.RequestVisibleEmbedCoordinates, - this.requestVisibleEmbedCoordinatesHandler, { start: false }, true, + this.requestVisibleEmbedCoordinatesHandler, ); } } diff --git a/src/embed/event-telemetry.spec.ts b/src/embed/event-telemetry.spec.ts deleted file mode 100644 index 5868dc8f0..000000000 --- a/src/embed/event-telemetry.spec.ts +++ /dev/null @@ -1,531 +0,0 @@ -import { - init, AuthType, LiveboardEmbed, HostEvent, EmbedEvent, EmbedErrorCodes, RuntimeFilterOp, -} from '../index'; -import { getDocumentBody, getRootEl } from '../test/test-utils'; -import { ERROR_MESSAGE } from '../errors'; -import { UIPassthroughEvent } from './hostEventClient/contracts'; -import { logger } from '../utils/logger'; -import * as authInstance from '../auth'; -import * as mixpanelInstance from '../mixpanel-service'; -import { MIXPANEL_EVENT } from '../mixpanel-service'; -import * as processTriggerInstance from '../utils/processTrigger'; -import { RESPONSE_WAIT_MS } from '../utils/eventTelemetry'; - -const flushTelemetry = () => new Promise((resolve) => setTimeout(resolve, 5)); - -const uploadsOf = (mock: jest.SpyInstance, eventId: string) => mock.mock.calls - .filter(([id]) => id === eventId) - .map(([, props]) => props as Record); - -const renderLiveboard = async (config: Record = {}) => { - init({ - thoughtSpotHost: 'https://tshost', - authType: AuthType.None, - ...config, - }); - const embed = new LiveboardEmbed(getRootEl(), { - frameParams: { width: '100%', height: '100%' }, - liveboardId: '4c8a1b2e-0000-0000-0000-000000000001', - }); - await embed.render(); - return embed; -}; - -describe('Host event telemetry', () => { - let mockUploadMixpanelEvent: jest.SpyInstance; - let mockProcessTrigger: jest.SpyInstance; - - beforeEach(() => { - document.body.innerHTML = getDocumentBody(); - jest.spyOn(authInstance, 'postLoginService').mockImplementation( - () => Promise.resolve(true as any), - ); - mockUploadMixpanelEvent = jest.spyOn(mixpanelInstance, 'uploadMixpanelEvent'); - mockProcessTrigger = jest - .spyOn(processTriggerInstance, 'processTrigger') - .mockResolvedValue({ session: 'ok' }); - }); - - afterEach(() => { - jest.restoreAllMocks(); - }); - - const triggerProps = async () => { - await flushTelemetry(); - const uploads = uploadsOf(mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT); - expect(uploads).toHaveLength(1); - return uploads[0]; - }; - - const mockPassthroughApp = ( - keys: string[], - passthroughResult: any = [{ value: { ok: true } }], - ) => { - mockProcessTrigger.mockImplementation( - (_iFrame: any, messageType: any, _host: any, data: any) => { - if (messageType !== HostEvent.UIPassthrough) { - return Promise.resolve({ session: 'ok' }); - } - if (data?.type === UIPassthroughEvent.GetAvailableUIPassthroughs) { - return Promise.resolve([{ value: { keys } }]); - } - return Promise.resolve(passthroughResult); - }, - ); - }; - - test('tells the whole story of a trigger, including what came back', async () => { - mockProcessTrigger.mockResolvedValue({ session: 'ok', answerId: 'a-1' }); - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); - - expect(await triggerProps()).toEqual( - expect.objectContaining({ - hostEvent: HostEvent.DownloadAsCsv, - embedComponentType: 'LiveboardEmbed', - contextType: 'none', - hasPayload: true, - paramCount: 1, - paramKeys: ['vizId'], - paramShape: ['vizId:string'], - status: 'success', - route: 'legacy', - durationMs: expect.any(Number), - responded: true, - responseType: 'object', - responseKeys: ['answerId', 'session'], - responseShape: ['answerId:string', 'session:string'], - }), - ); - }); - - test('leaves the legacy per-event upload exactly as it was', async () => { - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); - - expect(mockUploadMixpanelEvent).toHaveBeenCalledWith( - `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${HostEvent.DownloadAsCsv}`, - ); - }); - - test('never reports a response value', async () => { - mockProcessTrigger.mockResolvedValue({ answerName: 'Quarterly revenue', rows: [['west']] }); - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); - - const props = await triggerProps(); - expect(props.responseKeys).toEqual(['answerName', 'rows']); - ['Quarterly revenue', 'west'].forEach((value) => { - expect(JSON.stringify(props)).not.toContain(value); - }); - }); - - test('reports parameter names and enum members, never customer values', async () => { - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await embed.trigger(HostEvent.UpdateRuntimeFilters, [ - { columnName: 'Region', operator: RuntimeFilterOp.EQ, values: ['west'] }, - ]); - await flushTelemetry(); - - const serialized = JSON.stringify(mockUploadMixpanelEvent.mock.calls); - ['Region', 'west'].forEach((value) => expect(serialized).not.toContain(value)); - - const props = await triggerProps(); - expect(props.paramKeys).toEqual(['columnName', 'operator', 'values']); - expect(props.paramShape).toEqual( - expect.arrayContaining([ - 'payload[].columnName:string', - 'payload[].operator:EQ', - 'payload[].values:array(1)', - ]), - ); - }); - - test('records that the app never answered', async () => { - mockProcessTrigger.mockResolvedValue(new Error(ERROR_MESSAGE.TRIGGER_TIMED_OUT)); - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); - - expect(await triggerProps()).toEqual( - expect.objectContaining({ - status: 'timed-out', - responded: false, - durationMs: expect.any(Number), - }), - ); - }); - - test('reports a failed trigger without its error message', async () => { - mockProcessTrigger.mockRejectedValue(new Error('Answer 4c8a1b2e not found')); - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await expect(embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' })).rejects.toThrow(); - await flushTelemetry(); - - const props = await triggerProps(); - expect(props.status).toBe('error'); - expect(JSON.stringify(props)).not.toContain('4c8a1b2e'); - }); - - test('reports the ui-passthrough route for a getter the app supports', async () => { - mockPassthroughApp([UIPassthroughEvent.GetTabs]); - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await embed.trigger(HostEvent.GetTabs, {}); - await flushTelemetry(); - - expect((await triggerProps()).route).toBe('ui-passthrough'); - }); - - test('reports the legacy route when the app lacks the passthrough key', async () => { - mockPassthroughApp(['someUnrelatedPassthrough']); - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await embed.trigger(HostEvent.GetTabs, {}); - await flushTelemetry(); - - expect((await triggerProps()).route).toBe('legacy'); - }); - - test('reports the custom-handler route for a setter with custom logic', async () => { - mockPassthroughApp([UIPassthroughEvent.PinAnswerToLiveboard]); - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await embed.trigger(HostEvent.Pin, { - newVizName: 'Quarterly revenue', - liveboardId: '4c8a1b2e-0000-0000-0000-000000000002', - }); - await flushTelemetry(); - - expect(await triggerProps()).toEqual( - expect.objectContaining({ - route: 'custom-handler', - paramKeys: ['liveboardId', 'newVizName'], - }), - ); - }); - - test('reports a custom-handler trigger that the app never answered as timed out', async () => { - mockPassthroughApp( - [UIPassthroughEvent.PinAnswerToLiveboard], - new Error(ERROR_MESSAGE.TRIGGER_TIMED_OUT), - ); - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await expect( - embed.trigger(HostEvent.Pin, { - newVizName: 'Quarterly revenue', - liveboardId: '4c8a1b2e-0000-0000-0000-000000000002', - }), - ).rejects.toBeDefined(); - await flushTelemetry(); - - const props = await triggerProps(); - expect(props.status).toBe('timed-out'); - expect(props.responded).toBe(false); - }); - - test('reports a trigger called before render', async () => { - jest.spyOn(logger, 'error').mockImplementation(() => undefined); - init({ - thoughtSpotHost: 'https://tshost', - authType: AuthType.None, - }); - const embed = new LiveboardEmbed(getRootEl(), { - frameParams: { width: '100%', height: '100%' }, - liveboardId: '4c8a1b2e-0000-0000-0000-000000000001', - }); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); - await flushTelemetry(); - - expect(await triggerProps()).toEqual( - expect.objectContaining({ - status: 'render-not-called', - errorCode: EmbedErrorCodes.RENDER_NOT_CALLED, - }), - ); - }); - - test('builds no telemetry when the host application disabled tracking', async () => { - const embed = await renderLiveboard({ disableSDKTracking: true }); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); - await flushTelemetry(); - - expect( - uploadsOf(mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT), - ).toHaveLength(0); - expect(mockUploadMixpanelEvent.mock.calls.map(([id]) => id)).toEqual([ - `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${HostEvent.DownloadAsCsv}`, - ]); - }); -}); - -describe('Embed event telemetry', () => { - let mockUploadMixpanelEvent: jest.SpyInstance; - - beforeEach(() => { - document.body.innerHTML = getDocumentBody(); - jest.spyOn(authInstance, 'postLoginService').mockImplementation( - () => Promise.resolve(true as any), - ); - jest.spyOn(processTriggerInstance, 'processTrigger').mockResolvedValue({ session: 'ok' }); - mockUploadMixpanelEvent = jest.spyOn(mixpanelInstance, 'uploadMixpanelEvent'); - }); - - afterEach(() => { - jest.restoreAllMocks(); - }); - - const embedEventUploads = async () => { - await flushTelemetry(); - return uploadsOf(mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT); - }; - - test('reports an embed event the app sent, with types only', async () => { - const embed = await renderLiveboard(); - const handler = jest.fn(); - embed.on(EmbedEvent.Data, handler); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - (embed as any).executeCallbacks(EmbedEvent.Data, { - status: 'end', - data: { columnNames: ['Region'], rows: [['west', 100]] }, - answerName: 'Quarterly revenue', - }); - - const uploads = await embedEventUploads(); - expect(uploads).toHaveLength(1); - expect(uploads[0]).toEqual( - expect.objectContaining({ - embedEvent: EmbedEvent.Data, - embedComponentType: 'LiveboardEmbed', - eventStatus: 'end', - handlerCount: 1, - }), - ); - expect(uploads[0].paramKeys).toEqual(['answerName', 'data', 'status']); - ['Region', 'west', 'Quarterly revenue'].forEach((value) => { - expect(JSON.stringify(uploads[0])).not.toContain(value); - }); - }); - - test('counts only the handlers this dispatch actually ran', async () => { - const embed = await renderLiveboard(); - await flushTelemetry(); - embed.on(EmbedEvent.Data, jest.fn(), { start: true }); - embed.on(EmbedEvent.Data, jest.fn()); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - (embed as any).executeCallbacks(EmbedEvent.Data, { status: 'end' }); - const endUploads = await embedEventUploads(); - expect(endUploads[0].handlerCount).toBe(1); - - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - (embed as any).executeCallbacks(EmbedEvent.Data, { status: 'start' }); - const startUploads = await embedEventUploads(); - expect(startUploads[0].handlerCount).toBe(1); - }); - - test('tells the whole story when the host application responds', async () => { - const embed = await renderLiveboard(); - await flushTelemetry(); - embed.on(EmbedEvent.ApiIntercept, (_data: any, responder: any) => { - responder({ allow: true, answerName: 'Quarterly revenue' }); - }); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - (embed as any).executeCallbacks( - EmbedEvent.ApiIntercept, - { status: 'end', url: '/api/rest/2.0/metadata/search' }, - { postMessage: jest.fn() }, - ); - - const uploads = await embedEventUploads(); - expect(uploads).toHaveLength(1); - expect(uploads[0]).toEqual( - expect.objectContaining({ - embedEvent: EmbedEvent.ApiIntercept, - canRespond: true, - responded: true, - handlerCount: 1, - responseTimeMs: expect.any(Number), - }), - ); - expect(uploads[0].responseKeys).toEqual(['allow', 'answerName']); - expect(JSON.stringify(uploads[0])).not.toContain('Quarterly revenue'); - }); - - test('counts every handler even when an early one responds synchronously', async () => { - const embed = await renderLiveboard(); - await flushTelemetry(); - embed.on(EmbedEvent.ApiIntercept, (_data: any, responder: any) => { - responder({ allow: true }); - }); - embed.on(EmbedEvent.ApiIntercept, jest.fn()); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - (embed as any).executeCallbacks( - EmbedEvent.ApiIntercept, - { status: 'end' }, - { postMessage: jest.fn() }, - ); - - const uploads = await embedEventUploads(); - expect(uploads).toHaveLength(1); - expect(uploads[0]).toEqual( - expect.objectContaining({ handlerCount: 2, responded: true }), - ); - }); - - test('records that the host application never responded', async () => { - const embed = await renderLiveboard(); - await flushTelemetry(); - embed.on(EmbedEvent.ApiIntercept, jest.fn()); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - jest.useFakeTimers(); - try { - (embed as any).executeCallbacks( - EmbedEvent.ApiIntercept, - { status: 'end' }, - { postMessage: jest.fn() }, - ); - jest.advanceTimersByTime(1000); - expect( - uploadsOf(mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT), - ).toHaveLength(0); - - jest.advanceTimersByTime(RESPONSE_WAIT_MS + 100); - const uploads = uploadsOf( - mockUploadMixpanelEvent, MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT, - ); - expect(uploads).toHaveLength(1); - expect(uploads[0]).toEqual( - expect.objectContaining({ canRespond: true, responded: false }), - ); - } finally { - jest.useRealTimers(); - } - }); - - test('does not wait for a response an event cannot receive', async () => { - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - (embed as any).executeCallbacks(EmbedEvent.Data, { status: 'end' }); - - const uploads = await embedEventUploads(); - expect(uploads).toHaveLength(1); - expect(uploads[0]).toEqual( - expect.objectContaining({ canRespond: false, responded: false }), - ); - expect('responseTimeMs' in uploads[0]).toBe(false); - }); - - test('reports an embed event nobody is listening for', async () => { - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - (embed as any).executeCallbacks(EmbedEvent.Error, { status: 'end', error: 'boom' }); - - const uploads = await embedEventUploads(); - expect(uploads).toHaveLength(1); - expect(uploads[0].handlerCount).toBe(0); - expect(uploads[0].embedEvent).toBe(EmbedEvent.Error); - }); - - test('does not block the host application handler', async () => { - const embed = await renderLiveboard(); - const order: string[] = []; - embed.on(EmbedEvent.Data, () => order.push('handler')); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - mockUploadMixpanelEvent.mockImplementation(() => order.push('telemetry')); - - (embed as any).executeCallbacks(EmbedEvent.Data, { status: 'end' }); - - expect(order).toEqual(['handler']); - await flushTelemetry(); - expect(order).toEqual(['handler', 'telemetry']); - }); - - test('uploads nothing at all when the host application disabled tracking', async () => { - const embed = await renderLiveboard({ disableSDKTracking: true }); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - (embed as any).executeCallbacks(EmbedEvent.Data, { status: 'end' }); - - expect(await embedEventUploads()).toHaveLength(0); - }); - - test('reports a registration the host application made', async () => { - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - embed.on(EmbedEvent.Data, jest.fn()); - await flushTelemetry(); - - const uploads = uploadsOf( - mockUploadMixpanelEvent, `${MIXPANEL_EVENT.VISUAL_SDK_ON}-${EmbedEvent.Data}`, - ); - expect(uploads).toHaveLength(1); - expect(uploads[0]).toEqual( - expect.objectContaining({ - embedEvent: EmbedEvent.Data, - embedComponentType: 'LiveboardEmbed', - }), - ); - }); - - test('ignores the SDK registering its own handlers', async () => { - const embed = await renderLiveboard(); - await flushTelemetry(); - mockUploadMixpanelEvent.mockClear(); - - (embed as any).on(EmbedEvent.Data, jest.fn(), { start: false }, true); - await flushTelemetry(); - - expect(mockUploadMixpanelEvent).not.toHaveBeenCalled(); - }); -}); diff --git a/src/embed/host-event-telemetry.spec.ts b/src/embed/host-event-telemetry.spec.ts new file mode 100644 index 000000000..8f63688f2 --- /dev/null +++ b/src/embed/host-event-telemetry.spec.ts @@ -0,0 +1,95 @@ +import { + init, AuthType, LiveboardEmbed, HostEvent, RuntimeFilterOp, +} from '../index'; +import { getDocumentBody, getRootEl } from '../test/test-utils'; +import * as authInstance from '../auth'; +import * as mixpanelInstance from '../mixpanel-service'; +import { MIXPANEL_EVENT } from '../mixpanel-service'; +import * as processTriggerInstance from '../utils/processTrigger'; + +describe('Host event parameter telemetry', () => { + let mockUploadMixpanelEvent: jest.SpyInstance; + + beforeEach(() => { + document.body.innerHTML = getDocumentBody(); + jest.spyOn(authInstance, 'postLoginService').mockImplementation( + () => Promise.resolve(true as any), + ); + jest.spyOn(processTriggerInstance, 'processTrigger').mockResolvedValue({ session: 'ok' }); + mockUploadMixpanelEvent = jest.spyOn(mixpanelInstance, 'uploadMixpanelEvent'); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + const renderLiveboard = async () => { + init({ thoughtSpotHost: 'https://tshost', authType: AuthType.None }); + const embed = new LiveboardEmbed(getRootEl(), { + frameParams: { width: '100%', height: '100%' }, + liveboardId: '4c8a1b2e-0000-0000-0000-000000000001', + }); + await embed.render(); + return embed; + }; + + const triggerProps = (hostEvent: HostEvent) => { + const uploads = mockUploadMixpanelEvent.mock.calls.filter( + ([id]) => id === `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${hostEvent}`, + ); + expect(uploads).toHaveLength(1); + return uploads[0][1] as Record; + }; + + test('reports which host event was triggered and which parameters it used', async () => { + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + + expect(triggerProps(HostEvent.DownloadAsCsv)).toEqual( + expect.objectContaining({ + hostEvent: HostEvent.DownloadAsCsv, + embedComponentType: 'LiveboardEmbed', + contextType: 'none', + hasPayload: true, + paramCount: 1, + paramKeys: ['vizId'], + paramShape: ['vizId:string'], + }), + ); + }); + + test('keeps the existing event name, so existing reports still work', async () => { + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }); + + expect(mockUploadMixpanelEvent.mock.calls.map(([id]) => id)).toEqual([ + `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${HostEvent.DownloadAsCsv}`, + ]); + }); + + test('reports parameter names and enum members, never customer values', async () => { + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + + await embed.trigger(HostEvent.UpdateRuntimeFilters, [ + { columnName: 'Region', operator: RuntimeFilterOp.EQ, values: ['west'] }, + ]); + + const serialized = JSON.stringify(mockUploadMixpanelEvent.mock.calls); + ['Region', 'west'].forEach((value) => expect(serialized).not.toContain(value)); + + const props = triggerProps(HostEvent.UpdateRuntimeFilters); + expect(props.paramKeys).toEqual(['columnName', 'operator', 'values']); + expect(props.paramShape).toEqual( + expect.arrayContaining([ + 'payload[].columnName:string', + 'payload[].operator:EQ', + 'payload[].values:array(1)', + ]), + ); + }); +}); diff --git a/src/embed/hostEventClient/host-event-client.ts b/src/embed/hostEventClient/host-event-client.ts index e67c01afa..46e0f8cb0 100644 --- a/src/embed/hostEventClient/host-event-client.ts +++ b/src/embed/hostEventClient/host-event-client.ts @@ -1,9 +1,5 @@ import { ContextType, HostEvent } from '../../types'; -import { HostEventRoute } from '../../utils/eventTelemetry'; -import { - isTriggerTimeout, - processTrigger as processTriggerService, -} from '../../utils/processTrigger'; +import { processTrigger as processTriggerService } from '../../utils/processTrigger'; import { getEmbedConfig } from '../embedConfig'; import { isValidUpdateFiltersPayload, @@ -96,16 +92,12 @@ export class HostEventClient { parameters: UIPassthroughRequest, context?: ContextType, ): Promise> { - const raw = await this.triggerUIPassthroughApi(apiName, parameters, context); - const response = raw?.find?.((r) => r.error || r.value); + const response = (await this.triggerUIPassthroughApi(apiName, parameters, context)) + ?.find?.((r) => r.error || r.value); if (!response) { - const error = `No answer found${parameters?.vizId ? ` for vizId: ${parameters.vizId}` : ''}.`; - // A timeout arrives here as a missing response, because - // processTrigger resolves with an Error rather than rejecting. The - // thrown shape stays as it was; the flag lets telemetry tell an - // unanswered trigger from a genuine "no answer". - throw isTriggerTimeout(raw) ? { error, isTimeout: true } : { error }; + const error = `No answer found${parameters.vizId ? ` for vizId: ${parameters.vizId}` : ''}.`; + throw { error }; } const errors = response.error @@ -286,11 +278,6 @@ export class HostEventClient { * @param hostEvent - The host event to trigger * @param payload - Optional payload for the event * @param context - Optional context (e.g. vizId) for scoped operations - * @param onRoute - Optional telemetry hook, called with the dispatch branch - * taken here. It reports which branch ran, not which channel ultimately - * carried the message: a custom handler can fall back to the legacy channel - * itself, and `ui-passthrough` falls back too when the app returns no usable - * response. */ public async triggerHostEvent< HostEventT extends HostEvent, @@ -300,7 +287,6 @@ export class HostEventClient { hostEvent: HostEventT, payload?: TriggerPayload, context?: ContextT, - onRoute?: (route: HostEventRoute) => void, ): Promise> { const customHandler = this.customHandlers[hostEvent]; const passthroughEvent = PASSTHROUGH_MAP[hostEvent]; @@ -308,22 +294,15 @@ export class HostEventClient { // If embedded app supports passthrough but not this event, use legacy channel const keys = passthroughEvent ? await this.getAvailableUIPassthroughKeys(context as ContextType) : []; if (passthroughEvent && keys.length > 0 && !keys.includes(passthroughEvent)) { - onRoute?.('legacy'); return this.hostEventFallback(hostEvent, payload, context) as any; } // Custom handler (setters) > getter passthrough > legacy fallback - if (customHandler) { - onRoute?.('custom-handler'); - return customHandler(payload, context as ContextType) as any; - } - if (passthroughEvent) { - onRoute?.('ui-passthrough'); - return this.getDataWithPassthroughFallback( - passthroughEvent, hostEvent, payload, context as ContextType, - ) as any; - } - onRoute?.('legacy'); - return this.hostEventFallback(hostEvent, payload, context) as any; + return (customHandler + ? customHandler(payload, context as ContextType) + : passthroughEvent + ? this.getDataWithPassthroughFallback(passthroughEvent, hostEvent, payload, context as ContextType) + : this.hostEventFallback(hostEvent, payload, context) + ) as any; } } diff --git a/src/embed/liveboard.spec.ts b/src/embed/liveboard.spec.ts index 42a3135f3..46f84ed37 100644 --- a/src/embed/liveboard.spec.ts +++ b/src/embed/liveboard.spec.ts @@ -877,7 +877,7 @@ describe('Liveboard/viz embed tests', () => { liveboardEmbed.render(); executeAfterWait(() => { - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything(), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything()); }); }); @@ -1997,10 +1997,10 @@ describe('Liveboard/viz embed tests', () => { await liveboardEmbed.render(); await executeAfterWait(() => { - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything(), { start: false }, true); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RouteChange, expect.anything(), { start: false }, true); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedIframeCenter, expect.anything(), { start: false }, true); - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.anything(), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything()); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RouteChange, expect.anything()); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedIframeCenter, expect.anything()); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.RequestVisibleEmbedCoordinates, expect.anything()); }, 100); }); diff --git a/src/embed/liveboard.ts b/src/embed/liveboard.ts index 196f10050..0b93bd3e4 100644 --- a/src/embed/liveboard.ts +++ b/src/embed/liveboard.ts @@ -677,16 +677,10 @@ export class LiveboardEmbed extends V1Embed { 'Using full height with vizId might lead to unexpected behavior.'); } - this.on( - EmbedEvent.RouteChange, - this.setIframeHeightForNonEmbedLiveboard, { start: false }, true, - ); - this.on(EmbedEvent.EmbedHeight, this.updateIFrameHeight, { start: false }, true); - this.on(EmbedEvent.EmbedIframeCenter, this.embedIframeCenter, { start: false }, true); - this.on( - EmbedEvent.RequestVisibleEmbedCoordinates, - this.requestVisibleEmbedCoordinatesHandler, { start: false }, true, - ); + this.on(EmbedEvent.RouteChange, this.setIframeHeightForNonEmbedLiveboard); + this.on(EmbedEvent.EmbedHeight, this.updateIFrameHeight); + this.on(EmbedEvent.EmbedIframeCenter, this.embedIframeCenter); + this.on(EmbedEvent.RequestVisibleEmbedCoordinates, this.requestVisibleEmbedCoordinatesHandler); } } @@ -1097,7 +1091,7 @@ export class LiveboardEmbed extends V1Embed { this.hostElement.style.position = 'relative'; this.on(EmbedEvent.Data, () => { previewDiv.remove(); - }, { start: false }, true); + }); } catch (error) { console.error('Error fetching preview', error); } diff --git a/src/embed/pinboard.spec.ts b/src/embed/pinboard.spec.ts index 77b6f4c89..b45212fe1 100644 --- a/src/embed/pinboard.spec.ts +++ b/src/embed/pinboard.spec.ts @@ -241,7 +241,7 @@ describe('Pinboard/viz embed tests', () => { pinboardEmbed.render(); executeAfterWait(() => { - expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything(), { start: false }, true); + expect(onSpy).toHaveBeenCalledWith(EmbedEvent.EmbedHeight, expect.anything()); }); }); }); diff --git a/src/embed/ts-embed.ts b/src/embed/ts-embed.ts index bab16360a..10ed5d535 100644 --- a/src/embed/ts-embed.ts +++ b/src/embed/ts-embed.ts @@ -71,18 +71,6 @@ import { BaseViewConfig, } from '../types'; import { uploadMixpanelEvent, MIXPANEL_EVENT } from '../mixpanel-service'; -import { - getEmbedEventTelemetryProps, - describeResponse, - getHostEventTelemetryProps, - HostEventRoute, - HostEventStatus, - isTelemetryEnabled, - MAX_EMBED_SHAPE_PATHS, - reportEvent, - RESPONSE_WAIT_MS, -} from '../utils/eventTelemetry'; -import { isTriggerTimeout } from '../utils/processTrigger'; import { processEventData, processAuthFailure } from '../utils/processData'; import { version } from '../utils/sdk-version'; import { @@ -1438,46 +1426,6 @@ export class TsEmbed { const allHandlers = this.eventHandlerMap.get(EmbedEvent.ALL) || []; const callbacks = [...eventHandlers, ...allHandlers]; const dataStatus = data?.status || embedEventStatus.END; - const telemetryProps = isTelemetryEnabled() - ? getEmbedEventTelemetryProps({ - embedEvent: eventType, - payload: data, - embedComponentType: this.viewConfig?.embedComponentType, - }) - : null; - const dispatchedAt = Date.now(); - const canRespond = !!eventPort; - let reported = false; - let dispatchComplete = false; - let invokedHandlers = 0; - let response: { payload: unknown; at: number } | undefined; - const reportEmbedEvent = () => { - if (reported || !telemetryProps) { - return; - } - reported = true; - reportEvent(MIXPANEL_EVENT.VISUAL_SDK_EMBED_EVENT, { - ...telemetryProps, - handlerCount: invokedHandlers, - canRespond, - responded: !!response, - ...(response - ? { - responseTimeMs: response.at - dispatchedAt, - ...describeResponse(response.payload, MAX_EMBED_SHAPE_PATHS), - } - : {}), - }); - }; - // A handler can respond while the loop is still running, so the - // response is recorded now and reported once the count is final. - const recordResponse = (payload: unknown) => { - response = response || { payload, at: Date.now() }; - if (dispatchComplete) { - reportEmbedEvent(); - } - }; - callbacks.forEach((callbackObj) => { if ( // When start status is true it trigger only start releated @@ -1487,24 +1435,10 @@ export class TsEmbed { // payload (!callbackObj.options.start && dataStatus === embedEventStatus.END) ) { - invokedHandlers += 1; const responder = this.createEmbedEventResponder(eventPort, eventType); - callbackObj.callback(data, (payload: any) => { - recordResponse(payload); - return responder(payload); - }); + callbackObj.callback(data, responder); } }); - dispatchComplete = true; - - if (!telemetryProps) { - return; - } - if (canRespond && !response) { - setTimeout(reportEmbedEvent, RESPONSE_WAIT_MS); - return; - } - reportEmbedEvent(); } /** @@ -1588,13 +1522,9 @@ export class TsEmbed { options: MessageOptions = { start: false }, isRegisteredBySDK = false, ): typeof TsEmbed.prototype { - if (!isRegisteredBySDK) { - reportEvent(`${MIXPANEL_EVENT.VISUAL_SDK_ON}-${messageType}`, { - embedEvent: String(messageType), - embedComponentType: this.viewConfig?.embedComponentType || 'unknown', - sdkVersion: version, - }); - } + uploadMixpanelEvent(`${MIXPANEL_EVENT.VISUAL_SDK_ON}-${messageType}`, { + isRegisteredBySDK, + }); if (this.isRendered) { logger.warn('Please register event handlers before calling render'); } @@ -1749,37 +1679,9 @@ export class TsEmbed { data: TriggerPayload = {} as any, context?: ContextT, ): Promise> { - const triggerStartedAt = Date.now(); - let route: HostEventRoute | undefined; - const reportHostEvent = ( - status: HostEventStatus, - errorCode?: EmbedErrorCodes, - response?: unknown, - ) => { - if (!isTelemetryEnabled()) { - return; - } - const responded = status === 'success'; - reportEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT, { - ...getHostEventTelemetryProps({ - hostEvent: messageType, - payload: data, - context, - embedComponentType: this.viewConfig?.embedComponentType, - status, - durationMs: Date.now() - triggerStartedAt, - route, - errorCode, - }), - responded, - ...(responded ? describeResponse(response) : {}), - }); - }; - uploadMixpanelEvent(`${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`); if (!this.isRendered) { - reportHostEvent('render-not-called', EmbedErrorCodes.RENDER_NOT_CALLED); this.handleError({ errorType: ErrorDetailsTypes.VALIDATION_ERROR, message: ERROR_MESSAGE.RENDER_BEFORE_EVENTS_REQUIRED, @@ -1790,7 +1692,6 @@ export class TsEmbed { } if (!messageType) { - reportHostEvent('host-event-undefined', EmbedErrorCodes.HOST_EVENT_TYPE_UNDEFINED); this.handleError({ errorType: ErrorDetailsTypes.VALIDATION_ERROR, message: ERROR_MESSAGE.HOST_EVENT_TYPE_UNDEFINED, @@ -1806,53 +1707,34 @@ export class TsEmbed { logger.debug( `Cannot trigger ${messageType} - iframe not available (likely due to auth failure)`, ); - reportHostEvent('no-iframe'); return null; } // send an empty object, this is needed for liveboard default handlers - return this.hostEventClient - .triggerHostEvent(messageType, data, context, (dispatchRoute) => { - route = dispatchRoute; - }) - .then((response) => { - if (isTriggerTimeout(response)) { - reportHostEvent('timed-out'); - } else { - reportHostEvent('success', undefined, response); - } - return response; - }) - .catch( - ( - err: Error & { - isValidationError?: boolean; - isTimeout?: boolean; - embedErrorDetails?: { - errorType: ErrorDetailsTypes; - message: string; - code: EmbedErrorCodes; - error: string; - }; - }, - ): Promise => { - if (err?.isValidationError) { - const errorDetails = err.embedErrorDetails ?? { - errorType: ErrorDetailsTypes.VALIDATION_ERROR, - message: err.message || ERROR_MESSAGE.UPDATEFILTERS_INVALID_PAYLOAD, - code: EmbedErrorCodes.UPDATEFILTERS_INVALID_PAYLOAD, - error: err.message, - }; - this.handleError(errorDetails); - reportHostEvent('error', errorDetails.code); - } else if (err?.isTimeout) { - reportHostEvent('timed-out'); - } else { - reportHostEvent('error'); - } - throw err; + return this.hostEventClient.triggerHostEvent(messageType, data, context).catch( + ( + err: Error & { + isValidationError?: boolean; + embedErrorDetails?: { + errorType: ErrorDetailsTypes; + message: string; + code: EmbedErrorCodes; + error: string; + }; }, - ); + ): Promise => { + if (err?.isValidationError) { + const errorDetails = err.embedErrorDetails ?? { + errorType: ErrorDetailsTypes.VALIDATION_ERROR, + message: err.message || ERROR_MESSAGE.UPDATEFILTERS_INVALID_PAYLOAD, + code: EmbedErrorCodes.UPDATEFILTERS_INVALID_PAYLOAD, + error: err.message, + }; + this.handleError(errorDetails); + } + throw err; + }, + ); } /** @@ -2449,10 +2331,9 @@ export class V1Embed extends TsEmbed { messageType: EmbedEvent, callback: MessageCallback, options: MessageOptions = { start: false }, - isRegisteredBySDK = false, ): typeof TsEmbed.prototype { const eventType = this.getCompatibleEventType(messageType); - return super.on(eventType, callback, options, isRegisteredBySDK); + return super.on(eventType, callback, options); } /** diff --git a/src/mixpanel-service.spec.ts b/src/mixpanel-service.spec.ts index 5f8c35ef0..9fe7c229d 100644 --- a/src/mixpanel-service.spec.ts +++ b/src/mixpanel-service.spec.ts @@ -3,7 +3,6 @@ import { initMixpanel, uploadMixpanelEvent, MIXPANEL_EVENT, - MAX_QUEUED_EVENTS, testResetMixpanel, } from './mixpanel-service'; import { AuthType } from './types'; @@ -84,20 +83,6 @@ describe('Unit test for mixpanel', () => { expect(mixpanel.track).toHaveBeenCalledTimes(2); }); - test('caps the pre-init queue, so tracking left uninitialized cannot grow it', () => { - testResetMixpanel(); - for (let i = 0; i < MAX_QUEUED_EVENTS + 50; i += 1) { - uploadMixpanelEvent(MIXPANEL_EVENT.VISUAL_SDK_HOST_EVENT, { index: i }); - } - const sessionInfo = { - mixpanelToken: 'abc123', - userGUID: '12345', - isPublicUser: false, - } as SessionInfo; - initMixpanel(sessionInfo); - expect(mixpanel.track).toHaveBeenCalledTimes(MAX_QUEUED_EVENTS); - }); - test('init mixpanel with no mixpanel token', () => { jest.spyOn(logger, 'error').mockImplementation(() => {}); initMixpanel({ test: 'dummy' } as any); diff --git a/src/mixpanel-service.ts b/src/mixpanel-service.ts index b1a84308a..09da666b5 100644 --- a/src/mixpanel-service.ts +++ b/src/mixpanel-service.ts @@ -22,8 +22,6 @@ export const MIXPANEL_EVENT = { VISUAL_SDK_RENDER_COMPLETE: 'visual-sdk-render-complete', VISUAL_SDK_RENDER_FAILED: 'visual-sdk-render-failed', VISUAL_SDK_TRIGGER: 'visual-sdk-trigger', - VISUAL_SDK_HOST_EVENT: 'visual-sdk-host-event', - VISUAL_SDK_EMBED_EVENT: 'visual-sdk-embed-event', VISUAL_SDK_ON: 'visual-sdk-on', VISUAL_SDK_IFRAME_LOAD_PERFORMANCE: 'visual-sdk-iframe-load-performance', VISUAL_SDK_EMBED_CREATE: 'visual-sdk-embed-create', @@ -37,8 +35,6 @@ export const MIXPANEL_EVENT = { let isMixpanelInitialized = false; let eventQueue: { eventId: string; eventProps: any }[] = []; -export const MAX_QUEUED_EVENTS = 100; - /** * Pushes the event with its Property key-value map to mixpanel. * @param eventId @@ -46,12 +42,9 @@ export const MAX_QUEUED_EVENTS = 100; */ export function uploadMixpanelEvent(eventId: string, eventProps = {}): void { if (!isMixpanelInitialized) { - if (eventQueue.length < MAX_QUEUED_EVENTS) { - eventQueue.push({ eventId, eventProps }); - } + eventQueue.push({ eventId, eventProps }); return; } - mixpanelInstance.track(eventId, eventProps); } diff --git a/src/utils/eventTelemetry.spec.ts b/src/utils/hostEventTelemetry.spec.ts similarity index 100% rename from src/utils/eventTelemetry.spec.ts rename to src/utils/hostEventTelemetry.spec.ts diff --git a/src/utils/eventTelemetry.ts b/src/utils/hostEventTelemetry.ts similarity index 100% rename from src/utils/eventTelemetry.ts rename to src/utils/hostEventTelemetry.ts diff --git a/src/utils/processTrigger.ts b/src/utils/processTrigger.ts index 40c70c0d7..761eb6463 100644 --- a/src/utils/processTrigger.ts +++ b/src/utils/processTrigger.ts @@ -37,9 +37,6 @@ function postIframeMessage( export const TRIGGER_TIMEOUT = 30000; -export const isTriggerTimeout = (value: unknown): boolean => value instanceof Error - && value.message === ERROR_MESSAGE.TRIGGER_TIMED_OUT; - /** * * @param iFrame From a94ebf6f03b2c76e30cb8827c16c3714c9adf8fa Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 14:23:20 +0530 Subject: [PATCH 10/18] refactor(telemetry): dump the payload as a type map, drop the walker SCAL-333657 Far too much code for the question being asked. The recursive walker, depth and path caps, truncation flag, payload-type union and cyclic-payload handling all existed to serve nesting that "which parameters are used" does not need. The payload is now dumped as one flat key-to-type map, built with lodash mapValues/mapKeys over the top level: { vizId: 'string', operator: 'EQ', values: 'array(2)' } An array payload reads its parameters from the first element, which is what UpdateRuntimeFilters and UpdateFilters send, so an enum member is still visible where it matters. No recursion, so nothing to bound and no cyclic payload to survive. 189 lines to 64, and the spec 238 to 93. Note on lodash: it has no "type of value" helper, and this repo's own getTypeFromValue returns answer-column types, not JavaScript ones. So the leaf check is still typeof plus Array.isArray; lodash contributes the object mapping and isPlainObject. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/host-event-telemetry.spec.ts | 18 +- src/embed/ts-embed.ts | 11 +- src/utils/hostEventTelemetry.spec.ts | 324 +++++-------------------- src/utils/hostEventTelemetry.ts | 291 +++------------------- 4 files changed, 105 insertions(+), 539 deletions(-) diff --git a/src/embed/host-event-telemetry.spec.ts b/src/embed/host-event-telemetry.spec.ts index 8f63688f2..187a02e01 100644 --- a/src/embed/host-event-telemetry.spec.ts +++ b/src/embed/host-event-telemetry.spec.ts @@ -52,10 +52,8 @@ describe('Host event parameter telemetry', () => { hostEvent: HostEvent.DownloadAsCsv, embedComponentType: 'LiveboardEmbed', contextType: 'none', - hasPayload: true, - paramCount: 1, + params: { vizId: 'string' }, paramKeys: ['vizId'], - paramShape: ['vizId:string'], }), ); }); @@ -82,14 +80,10 @@ describe('Host event parameter telemetry', () => { const serialized = JSON.stringify(mockUploadMixpanelEvent.mock.calls); ['Region', 'west'].forEach((value) => expect(serialized).not.toContain(value)); - const props = triggerProps(HostEvent.UpdateRuntimeFilters); - expect(props.paramKeys).toEqual(['columnName', 'operator', 'values']); - expect(props.paramShape).toEqual( - expect.arrayContaining([ - 'payload[].columnName:string', - 'payload[].operator:EQ', - 'payload[].values:array(1)', - ]), - ); + expect(triggerProps(HostEvent.UpdateRuntimeFilters).params).toEqual({ + columnName: 'string', + operator: 'EQ', + values: 'array(1)', + }); }); }); diff --git a/src/embed/ts-embed.ts b/src/embed/ts-embed.ts index 10ed5d535..4e8782956 100644 --- a/src/embed/ts-embed.ts +++ b/src/embed/ts-embed.ts @@ -71,6 +71,7 @@ import { BaseViewConfig, } from '../types'; import { uploadMixpanelEvent, MIXPANEL_EVENT } from '../mixpanel-service'; +import { getHostEventTelemetryProps } from '../utils/hostEventTelemetry'; import { processEventData, processAuthFailure } from '../utils/processData'; import { version } from '../utils/sdk-version'; import { @@ -1679,7 +1680,15 @@ export class TsEmbed { data: TriggerPayload = {} as any, context?: ContextT, ): Promise> { - uploadMixpanelEvent(`${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`); + uploadMixpanelEvent( + `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`, + getHostEventTelemetryProps({ + hostEvent: messageType, + payload: data, + context, + embedComponentType: this.viewConfig?.embedComponentType, + }), + ); if (!this.isRendered) { this.handleError({ diff --git a/src/utils/hostEventTelemetry.spec.ts b/src/utils/hostEventTelemetry.spec.ts index 14c78ba71..71f1dd702 100644 --- a/src/utils/hostEventTelemetry.spec.ts +++ b/src/utils/hostEventTelemetry.spec.ts @@ -1,301 +1,93 @@ -import { - describePayload, - getEmbedEventTelemetryProps, - getHostEventTelemetryProps, - MAX_EMBED_SHAPE_PATHS, - MAX_SHAPE_PATHS, - REDACTED_KEY, -} from './eventTelemetry'; -import { ContextType, EmbedEvent, HostEvent, RuntimeFilterOp } from '../types'; -import { ApplicabilityLevel } from '../embed/hostEventClient/contracts'; +import { describeParams, getHostEventTelemetryProps, REDACTED_KEY } from './hostEventTelemetry'; +import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; import { version } from './sdk-version'; -describe('describePayload', () => { - test('reports no payload for undefined and null', () => { - [undefined, null].forEach((payload) => { - expect(describePayload(payload)).toEqual({ - hasPayload: false, - payloadType: 'none', - paramCount: 0, - paramKeys: [], - paramShape: [], - shapeTruncated: false, - }); - }); - }); - - test('reports an empty object as a payload with no parameters', () => { - const shape = describePayload({}); - expect(shape.hasPayload).toBe(false); - expect(shape.payloadType).toBe('object'); - expect(shape.paramCount).toBe(0); - expect(shape.paramKeys).toEqual([]); - }); - - test('reports which parameters of an object payload are used', () => { - const shape = describePayload({ +describe('describeParams', () => { + test('dumps each parameter as its type, never its value', () => { + expect(describeParams({ newVizName: 'Quarterly revenue', - liveboardId: '4c8a1b2e-0000-0000-0000-000000000001', - vizId: 'd0a1', - }); - expect(shape.paramCount).toBe(3); - expect(shape.paramKeys).toEqual(['liveboardId', 'newVizName', 'vizId']); - expect(shape.paramShape).toEqual([ - 'liveboardId:string', - 'newVizName:string', - 'vizId:string', - ]); - }); - - test('reports a boolean by its type, not its value', () => { - const shape = describePayload({ runRuntimeFilters: true, isPublic: false }); - expect(shape.paramShape).toEqual(['isPublic:boolean', 'runRuntimeFilters:boolean']); - }); - - test('reports array length and the shape of the first element', () => { - const shape = describePayload({ - runtimeFilters: [ - { columnName: 'Region', operator: RuntimeFilterOp.EQ, values: ['west', 'east'] }, - { columnName: 'Revenue', operator: RuntimeFilterOp.GT, values: [100] }, - ], + rowCount: 10, + runRuntimeFilters: true, + tabId: null, + columns: ['Region', 'Revenue'], + })).toEqual({ + newVizName: 'string', + rowCount: 'number', + runRuntimeFilters: 'boolean', + tabId: 'null', + columns: 'array(2)', }); - expect(shape.paramKeys).toEqual(['runtimeFilters']); - expect(shape.paramShape).toEqual([ - 'runtimeFilters:array(2)', - 'runtimeFilters[]:object(3)', - 'runtimeFilters[].columnName:string', - 'runtimeFilters[].operator:EQ', - 'runtimeFilters[].values:array(2)', - ]); }); - test('reports the member of an enum parameter, by either spelling', () => { - expect( - describePayload({ - filters: [{ column: 'Region', oper: RuntimeFilterOp.IN, values: ['west'] }], - }).paramShape, - ).toContain('filters[].oper:IN'); - - expect( - describePayload({ - filter: { - column: 'Region', - operator: RuntimeFilterOp.BW, - applicability: { level: ApplicabilityLevel.Tab, targetId: 'tab-1' }, - }, - }).paramShape, - ).toEqual( - expect.arrayContaining(['filter.operator:BW', 'filter.applicability.level:TAB']), - ); + test('keeps the member of an enum parameter, by either spelling', () => { + expect(describeParams({ operator: RuntimeFilterOp.EQ })).toEqual({ operator: 'EQ' }); + expect(describeParams({ oper: RuntimeFilterOp.IN })).toEqual({ oper: 'IN' }); }); test('falls back to the type when an enum parameter holds something else', () => { - const shape = describePayload({ - filters: [{ column: 'Region', oper: 'Total Sales > 500', values: ['west'] }], - }); - expect(shape.paramShape).toContain('filters[].oper:string'); - expect(JSON.stringify(shape)).not.toContain('Total Sales'); + expect(describeParams({ operator: 'Total Sales > 500' })).toEqual({ operator: 'string' }); }); - test('does not treat a customer value as an enum just because a sibling key does', () => { - const shape = describePayload({ - oper: RuntimeFilterOp.EQ, - values: ['EQ'], + test('reads the parameters of an array payload from its first element', () => { + expect(describeParams([ + { columnName: 'Region', operator: RuntimeFilterOp.EQ, values: ['west'] }, + ])).toEqual({ + columnName: 'string', + operator: 'EQ', + values: 'array(1)', }); - expect(shape.paramShape).toEqual(['oper:EQ', 'values:array(1)', 'values[]:string']); }); - test('reports empty containers and nulls without walking into them', () => { - const shape = describePayload({ - runtimeFilters: [], - parameters: {}, - vizId: null, + test('redacts a key name that could be customer data', () => { + expect(describeParams({ 'Total Sales': 1, vizId: 'd0a1' })).toEqual({ + [REDACTED_KEY]: 'number', + vizId: 'string', }); - expect(shape.paramShape).toEqual([ - 'parameters:object(0)', - 'runtimeFilters:array(0)', - 'vizId:null', - ]); - expect(shape.shapeTruncated).toBe(false); }); - test('treats a top-level array payload as the parameter list', () => { - const shape = describePayload([ - { columnName: 'Region', values: ['west'] }, - ]); - expect(shape.payloadType).toBe('array'); - expect(shape.paramCount).toBe(1); - expect(shape.paramKeys).toEqual(['columnName', 'values']); - expect(shape.paramShape[0]).toBe('payload:array(1)'); - }); - - test('reports a primitive payload as its type only', () => { - expect(describePayload('answer-guid')).toEqual( - expect.objectContaining({ - hasPayload: true, - payloadType: 'primitive', - paramKeys: [], - paramShape: ['payload:string'], - }), - ); + test('reports nothing for a payload with no parameters', () => { + [undefined, null, {}, [], 'answer-guid', 42].forEach((payload) => { + expect(describeParams(payload)).toEqual({}); + }); }); test('never reports a payload value', () => { - const secrets = ['Region', 'west', 'super-secret-token', 'Quarterly revenue']; - const shape = describePayload({ + const serialized = JSON.stringify(describeParams({ name: 'Quarterly revenue', - token: 'super-secret-token', - filters: [{ columnName: 'Region', values: ['west'] }], - }); - const serialized = JSON.stringify(shape); - secrets.forEach((secret) => { + token: 'secret-token-abc', + columns: ['Region'], + })); + ['Quarterly revenue', 'secret-token-abc', 'Region'].forEach((secret) => { expect(serialized).not.toContain(secret); }); }); - - test('redacts key names that could be customer data', () => { - const shape = describePayload({ - 'Total Sales': 100, - région: 'west', - [`${'a'.repeat(41)}`]: 1, - vizId: 'd0a1', - }); - expect(shape.paramKeys.filter((key: string) => key !== 'vizId')).toEqual([ - REDACTED_KEY, - REDACTED_KEY, - REDACTED_KEY, - ]); - expect(shape.paramShape).toContain('vizId:string'); - expect(shape.paramShape).not.toContain('Total Sales:number'); - }); - - test('summarizes below the depth limit instead of walking the whole payload', () => { - const shape = describePayload({ - a: { b: { c: { d: { e: 'deep' } } } }, - }); - expect(shape.shapeTruncated).toBe(true); - expect(shape.paramShape).toEqual([ - 'a:object(1)', - 'a.b:object(1)', - 'a.b.c:object(1)', - ]); - }); - - test('caps the number of reported key paths', () => { - const wide: Record = {}; - for (let i = 0; i < MAX_SHAPE_PATHS + 10; i += 1) { - wide[`param${i}`] = i; - } - const shape = describePayload(wide); - expect(shape.paramCount).toBe(MAX_SHAPE_PATHS + 10); - expect(shape.paramShape).toHaveLength(MAX_SHAPE_PATHS); - expect(shape.shapeTruncated).toBe(true); - }); - - test('survives a cyclic payload', () => { - const cyclic: any = { vizId: 'd0a1' }; - cyclic.self = cyclic; - expect(() => describePayload(cyclic)).not.toThrow(); - expect(describePayload(cyclic).paramKeys).toEqual(['self', 'vizId']); - }); - - test('survives a payload with a throwing getter', () => { - const hostile = { - get vizId() { - throw new Error('nope'); - }, - }; - expect(describePayload(hostile)).toEqual( - expect.objectContaining({ payloadType: 'unknown' }), - ); - }); }); describe('getHostEventTelemetryProps', () => { - test('reports the host event, context, embed component, outcome and duration', () => { - expect( - getHostEventTelemetryProps({ - hostEvent: HostEvent.Pin, - payload: { vizId: 'd0a1' }, - context: ContextType.Liveboard, - embedComponentType: 'LiveboardEmbed', - status: 'success', - durationMs: 412, - route: 'custom-handler', - }), - ).toEqual( - expect.objectContaining({ - hostEvent: HostEvent.Pin, - contextType: ContextType.Liveboard, - embedComponentType: 'LiveboardEmbed', - sdkVersion: version, - paramKeys: ['vizId'], - status: 'success', - durationMs: 412, - route: 'custom-handler', - }), - ); - }); - - test('omits route and errorCode when there is nothing to report', () => { - const props = getHostEventTelemetryProps({ - hostEvent: HostEvent.Reload, - status: 'no-iframe', - durationMs: 1, + test('reports the host event, context, embed component and SDK version', () => { + expect(getHostEventTelemetryProps({ + hostEvent: HostEvent.Pin, + payload: { vizId: 'd0a1' }, + context: ContextType.Liveboard, + embedComponentType: 'LiveboardEmbed', + })).toEqual({ + hostEvent: HostEvent.Pin, + contextType: ContextType.Liveboard, + embedComponentType: 'LiveboardEmbed', + sdkVersion: version, + params: { vizId: 'string' }, + paramKeys: ['vizId'], }); - expect(props.contextType).toBe('none'); - expect(props.embedComponentType).toBe('unknown'); - expect(props.hasPayload).toBe(false); - expect('route' in props).toBe(false); - expect('errorCode' in props).toBe(false); }); -}); -describe('getEmbedEventTelemetryProps', () => { - test('reports the embed event, its payload shape and whether anyone listens', () => { - const props = getEmbedEventTelemetryProps({ - embedEvent: EmbedEvent.Data, - payload: { - status: 'end', - data: { columnNames: ['Region'], rows: [['west', 100]] }, - }, - embedComponentType: 'LiveboardEmbed', - }); - expect(props).toEqual( + test('falls back when context and embed component are unknown', () => { + expect(getHostEventTelemetryProps({ hostEvent: HostEvent.Reload })).toEqual( expect.objectContaining({ - embedEvent: EmbedEvent.Data, - embedComponentType: 'LiveboardEmbed', - eventStatus: 'end', - sdkVersion: version, + contextType: 'none', + embedComponentType: 'unknown', + params: {}, + paramKeys: [], }), ); - expect(props.paramKeys).toEqual(['data', 'status']); - }); - - test('never reports an embed event payload value', () => { - const props = getEmbedEventTelemetryProps({ - embedEvent: EmbedEvent.Data, - payload: { - data: { columnNames: ['Region'], rows: [['west', 100]] }, - answerName: 'Quarterly revenue', - }, - }); - const serialized = JSON.stringify(props); - ['Region', 'west', 'Quarterly revenue'].forEach((value) => { - expect(serialized).not.toContain(value); - }); - }); - - test('caps an embed payload harder than a host event payload', () => { - const wide: Record = {}; - for (let i = 0; i < MAX_SHAPE_PATHS; i += 1) { - wide[`field${i}`] = i; - } - const props = getEmbedEventTelemetryProps({ - embedEvent: EmbedEvent.Data, - payload: wide, - }); - expect(props.paramShape).toHaveLength(MAX_EMBED_SHAPE_PATHS); - expect(props.shapeTruncated).toBe(true); }); }); diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index 0b4aa306d..d339831ad 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -1,249 +1,44 @@ -import { ContextType, EmbedEvent, HostEvent, RuntimeFilterOp } from '../types'; -import { ApplicabilityLevel } from '../embed/hostEventClient/contracts'; -import { getEmbedConfig } from '../embed/embedConfig'; -import { uploadMixpanelEvent } from '../mixpanel-service'; -import { logger } from './logger'; +import isPlainObject from 'lodash/isPlainObject'; +import mapKeys from 'lodash/mapKeys'; +import mapValues from 'lodash/mapValues'; +import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; import { version as sdkVersion } from './sdk-version'; -export const MAX_SHAPE_DEPTH = 3; -export const MAX_SHAPE_PATHS = 40; -export const MAX_EMBED_SHAPE_PATHS = 20; -export const MAX_KEY_LENGTH = 40; export const REDACTED_KEY = 'redactedKey'; -export const IDLE_TIMEOUT = 2000; -export const RESPONSE_WAIT_MS = 5000; - -const ROOT_PATH = 'payload'; -const SAFE_KEY_PATTERN = /^[A-Za-z_$][A-Za-z0-9_$]*$/; /* - * TODO: this hand-maintained map does not scale. Every host event that gains an - * enum parameter has to be added by hand and nothing fails if it is forgotten, - * so an unlisted enum silently degrades to `string`. A generated map, or a - * marker on the contract types that the members can be read back from, would - * keep it honest. Worth finding a better way. + * TODO: hand-maintained, so an enum parameter nobody adds here silently + * reports `string`. Generating it, or reading members off the contract + * types, would be better. */ -const ENUM_VALUED_PARAMS: Record = { +const ENUM_PARAMS: Record = { operator: Object.values(RuntimeFilterOp), oper: Object.values(RuntimeFilterOp), - level: Object.values(ApplicabilityLevel), -}; - -export type HostEventRoute = 'custom-handler' | 'ui-passthrough' | 'legacy'; - -export type HostEventStatus = - | 'success' - | 'error' - | 'timed-out' - | 'render-not-called' - | 'host-event-undefined' - | 'no-iframe'; - -export interface PayloadShape { - hasPayload: boolean; - payloadType: 'none' | 'object' | 'array' | 'primitive' | 'unknown'; - paramCount: number; - paramKeys: string[]; - paramShape: string[]; - shapeTruncated: boolean; -} - -export interface HostEventTelemetryProps extends PayloadShape { - hostEvent: string; - contextType: string; - embedComponentType: string; - sdkVersion: string; - status: HostEventStatus; - durationMs: number; - route?: HostEventRoute; - errorCode?: string; -} - -export interface EmbedEventTelemetryProps extends PayloadShape { - embedEvent: string; - embedComponentType: string; - sdkVersion: string; - eventStatus: string; - handlerCount: number; - canRespond: boolean; - responded: boolean; -} - -export interface ResponseShape { - responseType: PayloadShape['payloadType']; - responseKeys: string[]; - responseShape: string[]; -} - -interface ShapeAccumulator { - paths: string[]; - truncated: boolean; - maxPaths: number; -} - -const EMPTY_SHAPE: PayloadShape = { - hasPayload: false, - payloadType: 'none', - paramCount: 0, - paramKeys: [], - paramShape: [], - shapeTruncated: false, }; -const sanitizeKey = (key: string): string => ( - key.length <= MAX_KEY_LENGTH && SAFE_KEY_PATTERN.test(key) ? key : REDACTED_KEY -); +const IDENTIFIER = /^[A-Za-z_$][A-Za-z0-9_$]{0,39}$/; -const isEnumMember = (key: string, value: string): boolean => ( - ENUM_VALUED_PARAMS[key]?.includes(value) ?? false -); +const paramName = (key: string) => (IDENTIFIER.test(key) ? key : REDACTED_KEY); -const describeLeaf = (value: unknown, key?: string): string => { +const paramType = (value: unknown, key: string) => { if (value === null) { return 'null'; } - if (typeof value === 'string' && key && isEnumMember(key, value)) { - return value; - } - return typeof value; -}; - -const isRecord = (value: unknown): value is Record => ( - typeof value === 'object' && value !== null && !Array.isArray(value) -); - -const walkShape = ( - value: unknown, - path: string, - acc: ShapeAccumulator, - depth: number, - key?: string, -): void => { - if (acc.paths.length >= acc.maxPaths) { - acc.truncated = true; - return; - } - if (Array.isArray(value)) { - acc.paths.push(`${path}:array(${value.length})`); - if (value.length === 0) { - return; - } - if (depth >= MAX_SHAPE_DEPTH) { - acc.truncated = true; - return; - } - walkShape(value[0], `${path}[]`, acc, depth + 1, key); - return; - } - - if (isRecord(value)) { - const keys = Object.keys(value); - acc.paths.push(`${path}:object(${keys.length})`); - if (keys.length === 0) { - return; - } - if (depth >= MAX_SHAPE_DEPTH) { - acc.truncated = true; - return; - } - keys.sort().forEach((childKey) => { - walkShape( - value[childKey], `${path}.${sanitizeKey(childKey)}`, acc, depth + 1, childKey, - ); - }); - return; - } - - acc.paths.push(`${path}:${describeLeaf(value, key)}`); -}; - -export const describePayload = ( - payload: unknown, - maxPaths = MAX_SHAPE_PATHS, -): PayloadShape => { - if (payload === undefined || payload === null) { - return { ...EMPTY_SHAPE }; - } - - try { - const acc: ShapeAccumulator = { paths: [], truncated: false, maxPaths }; - - if (Array.isArray(payload)) { - const firstElement = payload[0]; - walkShape(payload, ROOT_PATH, acc, 0); - return { - hasPayload: payload.length > 0, - payloadType: 'array', - paramCount: payload.length, - paramKeys: isRecord(firstElement) - ? Object.keys(firstElement).map(sanitizeKey).sort() - : [], - paramShape: acc.paths, - shapeTruncated: acc.truncated, - }; - } - - if (isRecord(payload)) { - const keys = Object.keys(payload); - keys.sort().forEach((key) => { - walkShape(payload[key], sanitizeKey(key), acc, 1, key); - }); - return { - hasPayload: keys.length > 0, - payloadType: 'object', - paramCount: keys.length, - paramKeys: keys.map(sanitizeKey), - paramShape: acc.paths, - shapeTruncated: acc.truncated, - }; - } - - return { - ...EMPTY_SHAPE, - hasPayload: true, - payloadType: 'primitive', - paramShape: [`${ROOT_PATH}:${describeLeaf(payload)}`], - }; - } catch (e) { - return { ...EMPTY_SHAPE, payloadType: 'unknown' }; + return `array(${value.length})`; } -}; - -export const describeResponse = ( - payload: unknown, - maxPaths = MAX_SHAPE_PATHS, -): ResponseShape => { - const shape = describePayload(payload, maxPaths); - return { - responseType: shape.payloadType, - responseKeys: shape.paramKeys, - responseShape: shape.paramShape, - }; -}; - -export const isTelemetryEnabled = (): boolean => !getEmbedConfig()?.disableSDKTracking; - -const runWhenIdle = (work: () => void): void => { - const idle = (globalThis as any)?.requestIdleCallback; - if (typeof idle === 'function') { - idle(work, { timeout: IDLE_TIMEOUT }); - return; + if (typeof value === 'string' && ENUM_PARAMS[key]?.includes(value)) { + return value; } - setTimeout(work, 0); + return typeof value; }; -export const reportEvent = (eventId: string, props: Record): void => { - if (!isTelemetryEnabled()) { - return; +export const describeParams = (payload: unknown): Record => { + const params = Array.isArray(payload) ? payload[0] : payload; + if (!isPlainObject(params)) { + return {}; } - runWhenIdle(() => { - try { - uploadMixpanelEvent(eventId, props); - } catch (e) { - logger.debug('Could not report telemetry for', eventId, e); - } - }); + return mapKeys(mapValues(params as object, paramType), (_type, key) => paramName(key)); }; export const getHostEventTelemetryProps = ({ @@ -251,43 +46,19 @@ export const getHostEventTelemetryProps = ({ payload, context, embedComponentType, - status, - durationMs, - route, - errorCode, }: { hostEvent: HostEvent; payload?: unknown; context?: ContextType; embedComponentType?: string; - status: HostEventStatus; - durationMs: number; - route?: HostEventRoute; - errorCode?: string; -}): HostEventTelemetryProps => ({ - hostEvent: String(hostEvent), - contextType: context ? String(context) : 'none', - embedComponentType: embedComponentType || 'unknown', - sdkVersion, - status, - durationMs, - ...(route ? { route } : {}), - ...(errorCode ? { errorCode } : {}), - ...describePayload(payload), -}); - -export const getEmbedEventTelemetryProps = ({ - embedEvent, - payload, - embedComponentType, -}: { - embedEvent: EmbedEvent; - payload?: any; - embedComponentType?: string; -}): Omit => ({ - embedEvent: String(embedEvent), - embedComponentType: embedComponentType || 'unknown', - sdkVersion, - eventStatus: payload?.status ? String(payload.status) : 'none', - ...describePayload(payload, MAX_EMBED_SHAPE_PATHS), -}); +}) => { + const params = describeParams(payload); + return { + hostEvent: String(hostEvent), + contextType: context ? String(context) : 'none', + embedComponentType: embedComponentType || 'unknown', + sdkVersion, + params, + paramKeys: Object.keys(params), + }; +}; From d084b57bd2e80e0a2782026c827dbac6ff3dbc35 Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 15:01:12 +0530 Subject: [PATCH 11/18] fix(telemetry): never let telemetry break the trigger it describes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 Review catch, and it was real. Verified against this branch before the fix: a payload with a throwing getter made `trigger()` reject with `Error: no telemetry for you`, so a host application lost a working host event to a telemetry side effect. Building the properties and uploading them now happen inside one guarded helper, because either half can throw — `describeParams` reads the payload's own properties, and `uploadMixpanelEvent` calls into mixpanel. A failure is logged at debug level and the trigger carries on. This matters more once the upload moves inside the promise chain: a throw there rejects the chain, so a successful trigger would surface to the host application as a failure and be reported as an error at the same time. Two tests, both verified to fail without the guard: a payload that cannot be described, and an upload that throws. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/host-event-telemetry.spec.ts | 26 +++++++++++++++++++++++++ src/embed/ts-embed.ts | 17 +++++++--------- src/utils/hostEventTelemetry.ts | 27 ++++++++++++++++++++------ 3 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/embed/host-event-telemetry.spec.ts b/src/embed/host-event-telemetry.spec.ts index 187a02e01..08fb0da5d 100644 --- a/src/embed/host-event-telemetry.spec.ts +++ b/src/embed/host-event-telemetry.spec.ts @@ -69,6 +69,32 @@ describe('Host event parameter telemetry', () => { ]); }); + test('a payload it cannot describe never breaks the trigger', async () => { + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + const hostile = { + get vizId() { + throw new Error('no telemetry for you'); + }, + }; + + await expect(embed.trigger(HostEvent.DownloadAsCsv, hostile)).resolves.toEqual( + { session: 'ok' }, + ); + }); + + test('a failing upload never breaks the trigger', async () => { + const embed = await renderLiveboard(); + mockUploadMixpanelEvent.mockClear(); + mockUploadMixpanelEvent.mockImplementation(() => { + throw new Error('mixpanel is down'); + }); + + await expect( + embed.trigger(HostEvent.DownloadAsCsv, { vizId: 'd0a1' }), + ).resolves.toEqual({ session: 'ok' }); + }); + test('reports parameter names and enum members, never customer values', async () => { const embed = await renderLiveboard(); mockUploadMixpanelEvent.mockClear(); diff --git a/src/embed/ts-embed.ts b/src/embed/ts-embed.ts index 4e8782956..938c86d82 100644 --- a/src/embed/ts-embed.ts +++ b/src/embed/ts-embed.ts @@ -71,7 +71,7 @@ import { BaseViewConfig, } from '../types'; import { uploadMixpanelEvent, MIXPANEL_EVENT } from '../mixpanel-service'; -import { getHostEventTelemetryProps } from '../utils/hostEventTelemetry'; +import { reportHostEvent } from '../utils/hostEventTelemetry'; import { processEventData, processAuthFailure } from '../utils/processData'; import { version } from '../utils/sdk-version'; import { @@ -1680,15 +1680,12 @@ export class TsEmbed { data: TriggerPayload = {} as any, context?: ContextT, ): Promise> { - uploadMixpanelEvent( - `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${messageType}`, - getHostEventTelemetryProps({ - hostEvent: messageType, - payload: data, - context, - embedComponentType: this.viewConfig?.embedComponentType, - }), - ); + reportHostEvent({ + hostEvent: messageType, + payload: data, + context, + embedComponentType: this.viewConfig?.embedComponentType, + }); if (!this.isRendered) { this.handleError({ diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index d339831ad..4e7494ae0 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -2,6 +2,8 @@ import isPlainObject from 'lodash/isPlainObject'; import mapKeys from 'lodash/mapKeys'; import mapValues from 'lodash/mapValues'; import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; +import { MIXPANEL_EVENT, uploadMixpanelEvent } from '../mixpanel-service'; +import { logger } from './logger'; import { version as sdkVersion } from './sdk-version'; export const REDACTED_KEY = 'redactedKey'; @@ -41,17 +43,19 @@ export const describeParams = (payload: unknown): Record => { return mapKeys(mapValues(params as object, paramType), (_type, key) => paramName(key)); }; +export interface HostEventTelemetryParams { + hostEvent: HostEvent; + payload?: unknown; + context?: ContextType; + embedComponentType?: string; +} + export const getHostEventTelemetryProps = ({ hostEvent, payload, context, embedComponentType, -}: { - hostEvent: HostEvent; - payload?: unknown; - context?: ContextType; - embedComponentType?: string; -}) => { +}: HostEventTelemetryParams) => { const params = describeParams(payload); return { hostEvent: String(hostEvent), @@ -62,3 +66,14 @@ export const getHostEventTelemetryProps = ({ paramKeys: Object.keys(params), }; }; + +export const reportHostEvent = (params: HostEventTelemetryParams): void => { + try { + uploadMixpanelEvent( + `${MIXPANEL_EVENT.VISUAL_SDK_TRIGGER}-${params.hostEvent}`, + getHostEventTelemetryProps(params), + ); + } catch (e) { + logger.debug('Could not report host event telemetry', e); + } +}; From 33dc0e89c8719e4c10fc07c812994f06fe09cb95 Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 15:21:14 +0530 Subject: [PATCH 12/18] refactor(telemetry): drop the key redaction, guard values only SCAL-333657 The rule is about values, not key names: send no value unless it is an SDK enum member. Key names are the SDK's own parameter names, so they go as they are and the identifier check comes out. Two lodash calls become one. Co-Authored-By: Claude Opus 5 (1M context) --- src/utils/hostEventTelemetry.spec.ts | 9 +-------- src/utils/hostEventTelemetry.ts | 9 +-------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/src/utils/hostEventTelemetry.spec.ts b/src/utils/hostEventTelemetry.spec.ts index 71f1dd702..19a0a7c33 100644 --- a/src/utils/hostEventTelemetry.spec.ts +++ b/src/utils/hostEventTelemetry.spec.ts @@ -1,4 +1,4 @@ -import { describeParams, getHostEventTelemetryProps, REDACTED_KEY } from './hostEventTelemetry'; +import { describeParams, getHostEventTelemetryProps } from './hostEventTelemetry'; import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; import { version } from './sdk-version'; @@ -38,13 +38,6 @@ describe('describeParams', () => { }); }); - test('redacts a key name that could be customer data', () => { - expect(describeParams({ 'Total Sales': 1, vizId: 'd0a1' })).toEqual({ - [REDACTED_KEY]: 'number', - vizId: 'string', - }); - }); - test('reports nothing for a payload with no parameters', () => { [undefined, null, {}, [], 'answer-guid', 42].forEach((payload) => { expect(describeParams(payload)).toEqual({}); diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index 4e7494ae0..6d09eb943 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -1,13 +1,10 @@ import isPlainObject from 'lodash/isPlainObject'; -import mapKeys from 'lodash/mapKeys'; import mapValues from 'lodash/mapValues'; import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; import { MIXPANEL_EVENT, uploadMixpanelEvent } from '../mixpanel-service'; import { logger } from './logger'; import { version as sdkVersion } from './sdk-version'; -export const REDACTED_KEY = 'redactedKey'; - /* * TODO: hand-maintained, so an enum parameter nobody adds here silently * reports `string`. Generating it, or reading members off the contract @@ -18,10 +15,6 @@ const ENUM_PARAMS: Record = { oper: Object.values(RuntimeFilterOp), }; -const IDENTIFIER = /^[A-Za-z_$][A-Za-z0-9_$]{0,39}$/; - -const paramName = (key: string) => (IDENTIFIER.test(key) ? key : REDACTED_KEY); - const paramType = (value: unknown, key: string) => { if (value === null) { return 'null'; @@ -40,7 +33,7 @@ export const describeParams = (payload: unknown): Record => { if (!isPlainObject(params)) { return {}; } - return mapKeys(mapValues(params as object, paramType), (_type, key) => paramName(key)); + return mapValues(params as Record, paramType); }; export interface HostEventTelemetryParams { From 0e16e80508b7174893d4a748d80550d05185445f Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 18:34:51 +0530 Subject: [PATCH 13/18] feat(telemetry): report array element types, not just a count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 `columns: 'array(2)'` said how many but not what. It now reports the element types, so `['Region', 'Revenue']` becomes `['string', 'string']` and a mixed array reads `['object', 'null', 'number']`. Capped at ten elements, because a filter can carry a thousand values and sending a thousand copies of `'string'` to Mixpanel would be waste. A nested array reports as `'array'` rather than expanding, so there is still no recursion. That absence of recursion is what makes a circular payload a non-event: only the top level is read, values are turned into type strings, and the payload itself is never handed to Mixpanel, so nothing can walk a cycle. Two tests pin it — an object holding itself, and an array containing itself. Co-Authored-By: Claude Opus 5 (1M context) --- src/embed/host-event-telemetry.spec.ts | 2 +- src/utils/hostEventTelemetry.spec.ts | 36 +++++++++++++++++++++++--- src/utils/hostEventTelemetry.ts | 14 +++++++--- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/embed/host-event-telemetry.spec.ts b/src/embed/host-event-telemetry.spec.ts index 08fb0da5d..a943205e8 100644 --- a/src/embed/host-event-telemetry.spec.ts +++ b/src/embed/host-event-telemetry.spec.ts @@ -109,7 +109,7 @@ describe('Host event parameter telemetry', () => { expect(triggerProps(HostEvent.UpdateRuntimeFilters).params).toEqual({ columnName: 'string', operator: 'EQ', - values: 'array(1)', + values: ['string'], }); }); }); diff --git a/src/utils/hostEventTelemetry.spec.ts b/src/utils/hostEventTelemetry.spec.ts index 19a0a7c33..5a0279a04 100644 --- a/src/utils/hostEventTelemetry.spec.ts +++ b/src/utils/hostEventTelemetry.spec.ts @@ -1,4 +1,8 @@ -import { describeParams, getHostEventTelemetryProps } from './hostEventTelemetry'; +import { + describeParams, + getHostEventTelemetryProps, + MAX_ARRAY_TYPES, +} from './hostEventTelemetry'; import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; import { version } from './sdk-version'; @@ -10,15 +14,24 @@ describe('describeParams', () => { runRuntimeFilters: true, tabId: null, columns: ['Region', 'Revenue'], + points: [{ x: 1 }, null, 7], + empty: [], })).toEqual({ newVizName: 'string', rowCount: 'number', runRuntimeFilters: 'boolean', tabId: 'null', - columns: 'array(2)', + columns: ['string', 'string'], + points: ['object', 'null', 'number'], + empty: [], }); }); + test('caps how many element types it reports for a long array', () => { + const values = Array.from({ length: MAX_ARRAY_TYPES + 5 }, (_v, i) => `value-${i}`); + expect(describeParams({ values }).values).toHaveLength(MAX_ARRAY_TYPES); + }); + test('keeps the member of an enum parameter, by either spelling', () => { expect(describeParams({ operator: RuntimeFilterOp.EQ })).toEqual({ operator: 'EQ' }); expect(describeParams({ oper: RuntimeFilterOp.IN })).toEqual({ oper: 'IN' }); @@ -34,10 +47,27 @@ describe('describeParams', () => { ])).toEqual({ columnName: 'string', operator: 'EQ', - values: 'array(1)', + values: ['string'], }); }); + test('survives a circular payload, because it never recurses', () => { + const circular: any = { vizId: 'd0a1' }; + circular.self = circular; + circular.loop = [circular]; + expect(describeParams(circular)).toEqual({ + vizId: 'string', + self: 'object', + loop: ['object'], + }); + }); + + test('survives an array that contains itself', () => { + const loop: any[] = ['west']; + loop.push(loop); + expect(describeParams({ values: loop })).toEqual({ values: ['string', 'array'] }); + }); + test('reports nothing for a payload with no parameters', () => { [undefined, null, {}, [], 'answer-guid', 42].forEach((payload) => { expect(describeParams(payload)).toEqual({}); diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index 6d09eb943..ffb4ad460 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -15,12 +15,14 @@ const ENUM_PARAMS: Record = { oper: Object.values(RuntimeFilterOp), }; -const paramType = (value: unknown, key: string) => { +export const MAX_ARRAY_TYPES = 10; + +const valueType = (value: unknown, key: string): string => { if (value === null) { return 'null'; } if (Array.isArray(value)) { - return `array(${value.length})`; + return 'array'; } if (typeof value === 'string' && ENUM_PARAMS[key]?.includes(value)) { return value; @@ -28,7 +30,13 @@ const paramType = (value: unknown, key: string) => { return typeof value; }; -export const describeParams = (payload: unknown): Record => { +const paramType = (value: unknown, key: string): string | string[] => ( + Array.isArray(value) + ? value.slice(0, MAX_ARRAY_TYPES).map((item) => valueType(item, key)) + : valueType(value, key) +); + +export const describeParams = (payload: unknown): Record => { const params = Array.isArray(payload) ? payload[0] : payload; if (!isPlainObject(params)) { return {}; From c89a1644fb81fefcdca06748f5ec6aa738556c1b Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 18:58:39 +0530 Subject: [PATCH 14/18] feat(telemetry): describe nested objects, keep the shape of the payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 The type map now mirrors the payload all the way down instead of stopping at the top level: { points: [{ x: 1 }, null, 7] } -> { points: [{ x: 'number' }, 'null', 'number'] } { filter: { column: 'Region', applicability: { level: 'TAB' } } } -> { filter: { column: 'string', applicability: { level: 'TAB' } } } Recursing means cycles have to be handled rather than being impossible, so a value already on the current path reports `circular` and is not followed. The set is per path, not global, so a shared object referenced twice is still described twice. `level` goes back into the enum map: `Applicability.level` is a real enum in filter and parameter payloads, and nesting is what made it reachable. Arrays stay capped at ten elements. There is no depth limit — cycle detection is what guarantees termination — so a pathologically deep payload would cost a large property rather than a hang, and a stack overflow would be swallowed by the guard around the upload. Co-Authored-By: Claude Opus 5 (1M context) --- src/utils/hostEventTelemetry.spec.ts | 27 ++++++++++++++++----- src/utils/hostEventTelemetry.ts | 35 ++++++++++++++++++---------- 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/utils/hostEventTelemetry.spec.ts b/src/utils/hostEventTelemetry.spec.ts index 5a0279a04..ca50f7960 100644 --- a/src/utils/hostEventTelemetry.spec.ts +++ b/src/utils/hostEventTelemetry.spec.ts @@ -4,6 +4,7 @@ import { MAX_ARRAY_TYPES, } from './hostEventTelemetry'; import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; +import { ApplicabilityLevel } from '../embed/hostEventClient/contracts'; import { version } from './sdk-version'; describe('describeParams', () => { @@ -22,11 +23,25 @@ describe('describeParams', () => { runRuntimeFilters: 'boolean', tabId: 'null', columns: ['string', 'string'], - points: ['object', 'null', 'number'], + points: [{ x: 'number' }, 'null', 'number'], empty: [], }); }); + test('describes nested objects all the way down', () => { + expect(describeParams({ + filter: { + column: 'Region', + applicability: { level: ApplicabilityLevel.Tab, targetId: 'tab-1' }, + }, + })).toEqual({ + filter: { + column: 'string', + applicability: { level: 'TAB', targetId: 'string' }, + }, + }); + }); + test('caps how many element types it reports for a long array', () => { const values = Array.from({ length: MAX_ARRAY_TYPES + 5 }, (_v, i) => `value-${i}`); expect(describeParams({ values }).values).toHaveLength(MAX_ARRAY_TYPES); @@ -51,21 +66,21 @@ describe('describeParams', () => { }); }); - test('survives a circular payload, because it never recurses', () => { + test('reports a cycle instead of following it', () => { const circular: any = { vizId: 'd0a1' }; circular.self = circular; circular.loop = [circular]; expect(describeParams(circular)).toEqual({ vizId: 'string', - self: 'object', - loop: ['object'], + self: 'circular', + loop: ['circular'], }); }); - test('survives an array that contains itself', () => { + test('reports a cycle in an array instead of following it', () => { const loop: any[] = ['west']; loop.push(loop); - expect(describeParams({ values: loop })).toEqual({ values: ['string', 'array'] }); + expect(describeParams({ values: loop })).toEqual({ values: ['string', 'circular'] }); }); test('reports nothing for a payload with no parameters', () => { diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index ffb4ad460..fd603d0d7 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -1,6 +1,7 @@ import isPlainObject from 'lodash/isPlainObject'; import mapValues from 'lodash/mapValues'; import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; +import { ApplicabilityLevel } from '../embed/hostEventClient/contracts'; import { MIXPANEL_EVENT, uploadMixpanelEvent } from '../mixpanel-service'; import { logger } from './logger'; import { version as sdkVersion } from './sdk-version'; @@ -13,35 +14,45 @@ import { version as sdkVersion } from './sdk-version'; const ENUM_PARAMS: Record = { operator: Object.values(RuntimeFilterOp), oper: Object.values(RuntimeFilterOp), + level: Object.values(ApplicabilityLevel), }; export const MAX_ARRAY_TYPES = 10; -const valueType = (value: unknown, key: string): string => { +export type ParamTypes = string | ParamTypes[] | { [key: string]: ParamTypes }; + +const describeValue = (value: unknown, key: string, seen: Set): ParamTypes => { if (value === null) { return 'null'; } - if (Array.isArray(value)) { - return 'array'; - } if (typeof value === 'string' && ENUM_PARAMS[key]?.includes(value)) { return value; } + if (Array.isArray(value) || isPlainObject(value)) { + if (seen.has(value)) { + return 'circular'; + } + seen.add(value); + const described = Array.isArray(value) + ? value.slice(0, MAX_ARRAY_TYPES).map((item) => describeValue(item, key, seen)) + : mapValues(value as Record, (item, itemKey) => ( + describeValue(item, itemKey, seen) + )); + seen.delete(value); + return described; + } return typeof value; }; -const paramType = (value: unknown, key: string): string | string[] => ( - Array.isArray(value) - ? value.slice(0, MAX_ARRAY_TYPES).map((item) => valueType(item, key)) - : valueType(value, key) -); - -export const describeParams = (payload: unknown): Record => { +export const describeParams = (payload: unknown): Record => { const params = Array.isArray(payload) ? payload[0] : payload; if (!isPlainObject(params)) { return {}; } - return mapValues(params as Record, paramType); + const seen = new Set([params]); + return mapValues(params as Record, (value, key) => ( + describeValue(value, key, seen) + )); }; export interface HostEventTelemetryParams { From 66ea59564b0c02575c59066b45c19e801dcc66a8 Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 19:32:14 +0530 Subject: [PATCH 15/18] refactor(telemetry): name the cycle guard for what it holds SCAL-333657 `seen` implied every object visited, which is not what it is: entries are removed on the way out, so it only ever holds the objects between the payload and the value being described. Renamed to `ancestors`, with a comment saying why it exists at all. Co-Authored-By: Claude Opus 5 (1M context) --- src/utils/hostEventTelemetry.ts | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index fd603d0d7..b0acef73d 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -21,7 +21,14 @@ export const MAX_ARRAY_TYPES = 10; export type ParamTypes = string | ParamTypes[] | { [key: string]: ParamTypes }; -const describeValue = (value: unknown, key: string, seen: Set): ParamTypes => { +/* + * `ancestors` holds the objects between the payload and `value`. A value that + * is already one of its own ancestors is a cycle, so it is named rather than + * followed; without that, a payload holding itself would recurse forever. + * Entries are removed on the way out, so the same object referenced twice in + * different branches is still described twice. + */ +const describeValue = (value: unknown, key: string, ancestors: Set): ParamTypes => { if (value === null) { return 'null'; } @@ -29,16 +36,16 @@ const describeValue = (value: unknown, key: string, seen: Set): ParamTy return value; } if (Array.isArray(value) || isPlainObject(value)) { - if (seen.has(value)) { + if (ancestors.has(value)) { return 'circular'; } - seen.add(value); + ancestors.add(value); const described = Array.isArray(value) - ? value.slice(0, MAX_ARRAY_TYPES).map((item) => describeValue(item, key, seen)) + ? value.slice(0, MAX_ARRAY_TYPES).map((item) => describeValue(item, key, ancestors)) : mapValues(value as Record, (item, itemKey) => ( - describeValue(item, itemKey, seen) + describeValue(item, itemKey, ancestors) )); - seen.delete(value); + ancestors.delete(value); return described; } return typeof value; @@ -49,9 +56,9 @@ export const describeParams = (payload: unknown): Record => if (!isPlainObject(params)) { return {}; } - const seen = new Set([params]); + const ancestors = new Set([params]); return mapValues(params as Record, (value, key) => ( - describeValue(value, key, seen) + describeValue(value, key, ancestors) )); }; From 497481a0144039abf05a0cf95d720d1b4bf015bd Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 19:39:47 +0530 Subject: [PATCH 16/18] refactor(telemetry): serialise the payload first, drop the cycle guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 `JSON.parse(JSON.stringify(payload))` up front, then walk the copy. The copy cannot contain a cycle, so the ancestors set and the `circular` marker are gone, and functions and undefined values are dropped on the way through instead of needing a case each. What it costs, so the trade is on the record. The round trip is the most expensive thing in this path — a full serialise and parse of the payload on every trigger, where the walk alone allocated nothing. And it throws rather than degrading: a circular payload, a throwing getter or a BigInt now yields no parameters at all, where the guarded walk would have described the parts it could reach. The reason is logged at debug level. An empty parameter map therefore has two meanings — a host event with no parameters, and a payload that could not be serialised. Worth a marker property if that ambiguity ever costs anything. Verified against Node rather than assumed: a function value drops its key, a function inside an array becomes null, undefined drops, a Date becomes a string, Map and Set become empty objects, and circular, throwing getters and BigInt all throw. Co-Authored-By: Claude Opus 5 (1M context) --- src/utils/hostEventTelemetry.spec.ts | 32 +++++++++++++------- src/utils/hostEventTelemetry.ts | 45 +++++++++++++--------------- 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/src/utils/hostEventTelemetry.spec.ts b/src/utils/hostEventTelemetry.spec.ts index ca50f7960..5fa796b7c 100644 --- a/src/utils/hostEventTelemetry.spec.ts +++ b/src/utils/hostEventTelemetry.spec.ts @@ -66,21 +66,33 @@ describe('describeParams', () => { }); }); - test('reports a cycle instead of following it', () => { + test('reports nothing for a payload it cannot serialise', () => { const circular: any = { vizId: 'd0a1' }; circular.self = circular; - circular.loop = [circular]; - expect(describeParams(circular)).toEqual({ - vizId: 'string', - self: 'circular', - loop: ['circular'], - }); - }); + expect(describeParams(circular)).toEqual({}); - test('reports a cycle in an array instead of following it', () => { const loop: any[] = ['west']; loop.push(loop); - expect(describeParams({ values: loop })).toEqual({ values: ['string', 'circular'] }); + expect(describeParams({ values: loop })).toEqual({}); + + const throwing = { + get vizId(): string { + throw new Error('nope'); + }, + }; + expect(describeParams(throwing)).toEqual({}); + }); + + test('drops a function or undefined parameter instead of choking on it', () => { + expect(describeParams({ + vizId: 'd0a1', + callback: (): void => undefined, + missing: undefined, + handlers: [(): void => undefined, 'x'], + })).toEqual({ + vizId: 'string', + handlers: ['null', 'string'], + }); }); test('reports nothing for a payload with no parameters', () => { diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index b0acef73d..4e3ee16fa 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -21,45 +21,40 @@ export const MAX_ARRAY_TYPES = 10; export type ParamTypes = string | ParamTypes[] | { [key: string]: ParamTypes }; -/* - * `ancestors` holds the objects between the payload and `value`. A value that - * is already one of its own ancestors is a cycle, so it is named rather than - * followed; without that, a payload holding itself would recurse forever. - * Entries are removed on the way out, so the same object referenced twice in - * different branches is still described twice. - */ -const describeValue = (value: unknown, key: string, ancestors: Set): ParamTypes => { +const describeValue = (value: unknown, key: string): ParamTypes => { if (value === null) { return 'null'; } if (typeof value === 'string' && ENUM_PARAMS[key]?.includes(value)) { return value; } - if (Array.isArray(value) || isPlainObject(value)) { - if (ancestors.has(value)) { - return 'circular'; - } - ancestors.add(value); - const described = Array.isArray(value) - ? value.slice(0, MAX_ARRAY_TYPES).map((item) => describeValue(item, key, ancestors)) - : mapValues(value as Record, (item, itemKey) => ( - describeValue(item, itemKey, ancestors) - )); - ancestors.delete(value); - return described; + if (Array.isArray(value)) { + return value.slice(0, MAX_ARRAY_TYPES).map((item) => describeValue(item, key)); + } + if (isPlainObject(value)) { + return mapValues(value as Record, describeValue); } return typeof value; }; export const describeParams = (payload: unknown): Record => { - const params = Array.isArray(payload) ? payload[0] : payload; + let params; + try { + /* + * The round trip drops functions and undefined, and cannot produce a + * cycle, so the walk below needs no cycle guard. It throws instead on a + * circular payload, a throwing getter or a BigInt, which is what the + * catch is for: no parameters are reported, and the reason is logged. + */ + params = JSON.parse(JSON.stringify(Array.isArray(payload) ? payload[0] : payload)); + } catch (e) { + logger.debug('Could not describe host event payload', e); + return {}; + } if (!isPlainObject(params)) { return {}; } - const ancestors = new Set([params]); - return mapValues(params as Record, (value, key) => ( - describeValue(value, key, ancestors) - )); + return mapValues(params as Record, describeValue); }; export interface HostEventTelemetryParams { From e4f27f3c7055b3d2a74ea11cf8e98b18e3d34312 Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Fri, 21 Aug 2026 20:03:04 +0530 Subject: [PATCH 17/18] refactor(telemetry): one traversal entry point, and say why it is cycle-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SCAL-333657 The top level went through its own mapValues while everything below it went through describeValue, which did the same thing. Now the clone is handed straight to describeValue. describeValue has no cycle guard, and that is only safe because describeParams serialises first — a clone cannot hold a cycle. It is not exported, so nothing can reach it with a raw payload, and a comment now says so rather than leaving the next reader to work out why the guard went away. Co-Authored-By: Claude Opus 5 (1M context) --- src/utils/hostEventTelemetry.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index 4e3ee16fa..b6c25e2dd 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -21,6 +21,11 @@ export const MAX_ARRAY_TYPES = 10; export type ParamTypes = string | ParamTypes[] | { [key: string]: ParamTypes }; +/* + * Assumes an already-serialised value, which is what makes the absence of a + * cycle guard safe: describeParams clones first, and a clone cannot hold a + * cycle. Do not export this or call it with a raw payload. + */ const describeValue = (value: unknown, key: string): ParamTypes => { if (value === null) { return 'null'; @@ -51,10 +56,7 @@ export const describeParams = (payload: unknown): Record => logger.debug('Could not describe host event payload', e); return {}; } - if (!isPlainObject(params)) { - return {}; - } - return mapValues(params as Record, describeValue); + return isPlainObject(params) ? describeValue(params, '') as Record : {}; }; export interface HostEventTelemetryParams { From daf39072798402610360fc47bc411bab9119d2d0 Mon Sep 17 00:00:00 2001 From: Justin Mathew Date: Wed, 26 Aug 2026 18:23:01 +0530 Subject: [PATCH 18/18] SCAL-333657 : Clean up --- src/utils/hostEventTelemetry.ts | 56 ++++++++++++++++----------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/src/utils/hostEventTelemetry.ts b/src/utils/hostEventTelemetry.ts index b6c25e2dd..020d3abbe 100644 --- a/src/utils/hostEventTelemetry.ts +++ b/src/utils/hostEventTelemetry.ts @@ -1,21 +1,11 @@ -import isPlainObject from 'lodash/isPlainObject'; -import mapValues from 'lodash/mapValues'; import { ContextType, HostEvent, RuntimeFilterOp } from '../types'; -import { ApplicabilityLevel } from '../embed/hostEventClient/contracts'; import { MIXPANEL_EVENT, uploadMixpanelEvent } from '../mixpanel-service'; import { logger } from './logger'; import { version as sdkVersion } from './sdk-version'; -/* - * TODO: hand-maintained, so an enum parameter nobody adds here silently - * reports `string`. Generating it, or reading members off the contract - * types, would be better. - */ -const ENUM_PARAMS: Record = { - operator: Object.values(RuntimeFilterOp), - oper: Object.values(RuntimeFilterOp), - level: Object.values(ApplicabilityLevel), -}; + +// we preserver these field's values +const PRESERVED_FIELDS: Array = ['operator', 'oper', 'level']; export const MAX_ARRAY_TYPES = 10; @@ -26,23 +16,31 @@ export type ParamTypes = string | ParamTypes[] | { [key: string]: ParamTypes }; * cycle guard safe: describeParams clones first, and a clone cannot hold a * cycle. Do not export this or call it with a raw payload. */ -const describeValue = (value: unknown, key: string): ParamTypes => { - if (value === null) { - return 'null'; - } - if (typeof value === 'string' && ENUM_PARAMS[key]?.includes(value)) { - return value; - } - if (Array.isArray(value)) { - return value.slice(0, MAX_ARRAY_TYPES).map((item) => describeValue(item, key)); - } - if (isPlainObject(value)) { - return mapValues(value as Record, describeValue); +const describeValue = (value: unknown): ParamTypes => { + try { + if (value === null) { + return 'null'; + } + + if (Array.isArray(value)) { + return value.slice(0, MAX_ARRAY_TYPES).map((item) => describeValue(item)); + } + + if (typeof value === 'object') { + Object.keys(value).forEach(key => { + if (!PRESERVED_FIELDS.includes(key)) + (value as any)[key] = describeValue((value as any)[key]) + }); + } + + return typeof value; + } catch (e) { + logger.debug('Error parsing type', value); + return 'ErrorParsing' } - return typeof value; }; -export const describeParams = (payload: unknown): Record => { +export const describeParams = (payload: unknown): unknown => { let params; try { /* @@ -51,12 +49,12 @@ export const describeParams = (payload: unknown): Record => * circular payload, a throwing getter or a BigInt, which is what the * catch is for: no parameters are reported, and the reason is logged. */ - params = JSON.parse(JSON.stringify(Array.isArray(payload) ? payload[0] : payload)); + params = JSON.parse(JSON.stringify(payload)); } catch (e) { logger.debug('Could not describe host event payload', e); return {}; } - return isPlainObject(params) ? describeValue(params, '') as Record : {}; + return describeValue(params); }; export interface HostEventTelemetryParams {