From dffa1892cd61abaf534d937d743637f05086b3d0 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 13 Aug 2021 16:44:15 +0200 Subject: [PATCH] Simplify Analytics API from analytics module POV Signed-off-by: Eric Peterson --- packages/core-plugin-api/api-report.md | 6 +-- .../src/analytics/useAnalytics.test.tsx | 24 +++++----- .../src/analytics/useAnalytics.tsx | 16 ++++++- .../src/apis/definitions/AnalyticsApi.ts | 9 ++-- packages/test-utils/api-report.md | 12 ++--- .../AnalyticsApi/MockAnalyticsApi.test.ts | 13 +++-- .../apis/AnalyticsApi/MockAnalyticsApi.ts | 30 +++++------- .../AnalyticsApi/GoogleAnalytics.test.ts | 47 +++++++++++++------ .../AnalyticsApi/GoogleAnalytics.ts | 29 ++++-------- 9 files changed, 100 insertions(+), 86 deletions(-) diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index 7c833d2b66..2628c13ad1 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -38,11 +38,7 @@ export type AlertMessage = { // // @public export type AnalyticsApi = { - getDecoratedTracker({ - domain, - }: { - domain: AnalyticsDomainValue; - }): AnalyticsTracker; + captureEvent(event: DomainDecoratedAnalyticsEvent): void; }; // Warning: (ae-missing-release-tag) "analyticsApiRef" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) diff --git a/packages/core-plugin-api/src/analytics/useAnalytics.test.tsx b/packages/core-plugin-api/src/analytics/useAnalytics.test.tsx index 33e71aa2e4..d8d6706a9e 100644 --- a/packages/core-plugin-api/src/analytics/useAnalytics.test.tsx +++ b/packages/core-plugin-api/src/analytics/useAnalytics.test.tsx @@ -35,27 +35,27 @@ describe('useAnalytics', () => { }); it('returns tracker from defined analytics api', () => { - const expectedFunction = 'capture function'; - const getDecoratedTracker = jest.fn().mockReturnValue({ - captureEvent: expectedFunction, - }); + const captureEvent = jest.fn(); // Simulate useApi returning a valid tracker. - mocked(useApi).mockReturnValue({ getDecoratedTracker }); + mocked(useApi).mockReturnValue({ captureEvent }); - // The getDecoratedTracker method of the underlying implementation should - // have been called with the domain provided. + // Calling the captureEvent method of the underlying implementation should + // pass along the given event as well as the default domain. const { result } = renderHook(() => useAnalytics()); - expect(getDecoratedTracker).toHaveBeenCalledWith({ + result.current.captureEvent('a verb', 'a noun', 42, { some: 'value' }); + expect(captureEvent).toHaveBeenCalledWith({ + verb: 'a verb', + noun: 'a noun', + value: 42, + context: { + some: 'value', + }, domain: { componentName: 'App', pluginId: 'root', routeRef: 'unknown', }, }); - - // And the returned tracker's captureEvent should have come from the API - // implementation. - expect(result.current.captureEvent).toBe(expectedFunction); }); }); diff --git a/packages/core-plugin-api/src/analytics/useAnalytics.tsx b/packages/core-plugin-api/src/analytics/useAnalytics.tsx index e3dc851321..8643747f35 100644 --- a/packages/core-plugin-api/src/analytics/useAnalytics.tsx +++ b/packages/core-plugin-api/src/analytics/useAnalytics.tsx @@ -24,7 +24,21 @@ import { useApi } from '../apis'; function useTracker(): AnalyticsTracker { const analyticsApi = useApi(analyticsApiRef); const domain = useAnalyticsDomain(); - return analyticsApi.getDecoratedTracker({ domain }); + return { + captureEvent: (verb, noun, value, context) => { + try { + analyticsApi.captureEvent({ + verb, + noun, + value, + context, + domain, + }); + } catch { + // Just don't throw on an instrumentation problem. + } + }, + }; } /** diff --git a/packages/core-plugin-api/src/apis/definitions/AnalyticsApi.ts b/packages/core-plugin-api/src/apis/definitions/AnalyticsApi.ts index 0e5f87b038..4601077835 100644 --- a/packages/core-plugin-api/src/apis/definitions/AnalyticsApi.ts +++ b/packages/core-plugin-api/src/apis/definitions/AnalyticsApi.ts @@ -109,13 +109,10 @@ export type AnalyticsTracker = { */ export type AnalyticsApi = { /** - * Retrieves a tracker decorated with a given analytics domain. + * Primary event handler responsible for compiling and forwarding events to + * an analytics system. */ - getDecoratedTracker({ - domain, - }: { - domain: AnalyticsDomainValue; - }): AnalyticsTracker; + captureEvent(event: DomainDecoratedAnalyticsEvent): void; }; export const analyticsApiRef: ApiRef = createApiRef({ diff --git a/packages/test-utils/api-report.md b/packages/test-utils/api-report.md index 9c6bc4d999..d439efd0f9 100644 --- a/packages/test-utils/api-report.md +++ b/packages/test-utils/api-report.md @@ -4,8 +4,6 @@ ```ts import { AnalyticsApi } from '@backstage/core-plugin-api'; -import { AnalyticsDomainValue } from '@backstage/core-plugin-api'; -import { AnalyticsTracker } from '@backstage/core-plugin-api'; import { ComponentType } from 'react'; import { DomainDecoratedAnalyticsEvent } from '@backstage/core-plugin-api'; import { ErrorApi } from '@backstage/core-plugin-api'; @@ -24,11 +22,13 @@ import { StorageValueChange } from '@backstage/core-plugin-api'; // @public (undocumented) export class MockAnalyticsApi implements AnalyticsApi { // (undocumented) - getDecoratedTracker({ + captureEvent({ + verb, + noun, + value, + context, domain, - }: { - domain: AnalyticsDomainValue; - }): AnalyticsTracker; + }: DomainDecoratedAnalyticsEvent): void; // (undocumented) getEvents(): DomainDecoratedAnalyticsEvent[]; } diff --git a/packages/test-utils/src/testUtils/apis/AnalyticsApi/MockAnalyticsApi.test.ts b/packages/test-utils/src/testUtils/apis/AnalyticsApi/MockAnalyticsApi.test.ts index 4113940072..fdd9478c5d 100644 --- a/packages/test-utils/src/testUtils/apis/AnalyticsApi/MockAnalyticsApi.test.ts +++ b/packages/test-utils/src/testUtils/apis/AnalyticsApi/MockAnalyticsApi.test.ts @@ -23,11 +23,16 @@ describe('MockAnalyticsApi', () => { it('should collect events', () => { const api = new MockAnalyticsApi(); - const tracker = api.getDecoratedTracker({ domain }); - tracker.captureEvent('verb-1', 'noun-1'); - tracker.captureEvent('verb-2', 'noun-2', 42); - tracker.captureEvent('verb-3', 'noun-3', 1337, { some: 'context' }); + api.captureEvent({ verb: 'verb-1', noun: 'noun-1', domain }); + api.captureEvent({ verb: 'verb-2', noun: 'noun-2', value: 42, domain }); + api.captureEvent({ + verb: 'verb-3', + noun: 'noun-3', + value: 1337, + context: { some: 'context' }, + domain, + }); expect(api.getEvents()[0]).toMatchObject({ noun: 'noun-1', diff --git a/packages/test-utils/src/testUtils/apis/AnalyticsApi/MockAnalyticsApi.ts b/packages/test-utils/src/testUtils/apis/AnalyticsApi/MockAnalyticsApi.ts index 7e8e19d438..b90290ff6a 100644 --- a/packages/test-utils/src/testUtils/apis/AnalyticsApi/MockAnalyticsApi.ts +++ b/packages/test-utils/src/testUtils/apis/AnalyticsApi/MockAnalyticsApi.ts @@ -16,30 +16,26 @@ import { AnalyticsApi, - AnalyticsDomainValue, - AnalyticsTracker, DomainDecoratedAnalyticsEvent, } from '@backstage/core-plugin-api'; export class MockAnalyticsApi implements AnalyticsApi { private events: DomainDecoratedAnalyticsEvent[] = []; - getDecoratedTracker({ + captureEvent({ + verb, + noun, + value, + context, domain, - }: { - domain: AnalyticsDomainValue; - }): AnalyticsTracker { - return { - captureEvent: (verb, noun, value, context) => { - this.events.push({ - verb, - noun, - domain, - ...(value !== undefined ? { value } : {}), - ...(context !== undefined ? { context } : {}), - }); - }, - }; + }: DomainDecoratedAnalyticsEvent) { + this.events.push({ + verb, + noun, + domain, + ...(value !== undefined ? { value } : {}), + ...(context !== undefined ? { context } : {}), + }); } getEvents(): DomainDecoratedAnalyticsEvent[] { diff --git a/plugins/analytics-module-ga/src/apis/implementations/AnalyticsApi/GoogleAnalytics.test.ts b/plugins/analytics-module-ga/src/apis/implementations/AnalyticsApi/GoogleAnalytics.test.ts index 220ff5261b..da319d365c 100644 --- a/plugins/analytics-module-ga/src/apis/implementations/AnalyticsApi/GoogleAnalytics.test.ts +++ b/plugins/analytics-module-ga/src/apis/implementations/AnalyticsApi/GoogleAnalytics.test.ts @@ -38,7 +38,7 @@ describe('GoogleAnalytics', () => { it('returns implementation', () => { const api = GoogleAnalytics.fromConfig(basicValidConfig); - expect(api.getDecoratedTracker).toBeDefined(); + expect(api.captureEvent).toBeDefined(); // Initializes GA with tracking ID. expect(ReactGA.testModeAPI.calls[0]).toEqual([ @@ -94,8 +94,11 @@ describe('GoogleAnalytics', () => { it('tracks basic pageview', () => { const api = GoogleAnalytics.fromConfig(basicValidConfig); - const tracker = api.getDecoratedTracker({ domain }); - tracker.captureEvent('navigate', '/'); + api.captureEvent({ + verb: 'navigate', + noun: '/', + domain, + }); const [command, data] = ReactGA.testModeAPI.calls[1]; expect(command).toBe('send'); @@ -107,12 +110,16 @@ describe('GoogleAnalytics', () => { it('tracks basic event', () => { const api = GoogleAnalytics.fromConfig(basicValidConfig); - const tracker = api.getDecoratedTracker({ domain }); const expectedAction = 'click'; const expectedLabel = 'on something'; const expectedValue = 42; - tracker.captureEvent(expectedAction, expectedLabel, expectedValue); + api.captureEvent({ + verb: expectedAction, + noun: expectedLabel, + value: expectedValue, + domain, + }); const [command, data] = ReactGA.testModeAPI.calls[1]; expect(command).toBe('send'); @@ -127,8 +134,11 @@ describe('GoogleAnalytics', () => { it('captures configured custom dimensions/metrics on pageviews', () => { const api = GoogleAnalytics.fromConfig(advancedConfig); - const tracker = api.getDecoratedTracker({ domain }); - tracker.captureEvent('navigate', '/a-page'); + api.captureEvent({ + verb: 'navigate', + noun: '/a-page', + domain, + }); // Expect a set command first. const [setCommand, setData] = ReactGA.testModeAPI.calls[1]; @@ -149,14 +159,19 @@ describe('GoogleAnalytics', () => { it('captures configured custom dimensions/metrics on events', () => { const api = GoogleAnalytics.fromConfig(advancedConfig); - const tracker = api.getDecoratedTracker({ domain }); const expectedAction = 'search'; const expectedLabel = 'some query'; const expectedValue = 5; - tracker.captureEvent(expectedAction, expectedLabel, expectedValue, { - extraDimension: false, - extraMetric: 0, + api.captureEvent({ + verb: expectedAction, + noun: expectedLabel, + value: expectedValue, + context: { + extraDimension: false, + extraMetric: 0, + }, + domain, }); const [command, data] = ReactGA.testModeAPI.calls[1]; @@ -176,10 +191,14 @@ describe('GoogleAnalytics', () => { it('does not pass non-numeric data on metrics', () => { const api = GoogleAnalytics.fromConfig(advancedConfig); - const tracker = api.getDecoratedTracker({ domain }); - tracker.captureEvent('verb', 'noun', undefined, { - extraMetric: 'not a number', + api.captureEvent({ + verb: 'verb', + noun: 'noun', + context: { + extraMetric: 'not a number', + }, + domain, }); const [, data] = ReactGA.testModeAPI.calls[1]; diff --git a/plugins/analytics-module-ga/src/apis/implementations/AnalyticsApi/GoogleAnalytics.ts b/plugins/analytics-module-ga/src/apis/implementations/AnalyticsApi/GoogleAnalytics.ts index d02924d138..e721dc5e01 100644 --- a/plugins/analytics-module-ga/src/apis/implementations/AnalyticsApi/GoogleAnalytics.ts +++ b/plugins/analytics-module-ga/src/apis/implementations/AnalyticsApi/GoogleAnalytics.ts @@ -17,9 +17,9 @@ import ReactGA from 'react-ga'; import { AnalyticsApi, - AnalyticsTracker, AnalyticsDomainValue, AnalyticsEventContext, + DomainDecoratedAnalyticsEvent, } from '@backstage/core-plugin-api'; import { Config } from '@backstage/config'; @@ -92,31 +92,18 @@ export class GoogleAnalytics implements AnalyticsApi { }); } - /** - * Returns the Google Analytics tracker to API consumers. - */ - getDecoratedTracker({ - domain, - }: { - domain: AnalyticsDomainValue; - }): AnalyticsTracker { - return { - captureEvent: (...args) => this.captureEvent(domain, ...args), - }; - } - /** * Primary event capture implementation. Handles core navigate event as a * pageview and the rest as custom events. All custom dimensions/metrics are * applied as they should be (set on pageview, merged object on events). */ - private captureEvent( - domain: AnalyticsDomainValue, - verb: string, - noun: string, - value?: number, - context?: AnalyticsEventContext, - ) { + captureEvent({ + domain, + verb, + noun, + value, + context, + }: DomainDecoratedAnalyticsEvent) { const customMetadata = this.getCustomDimensionMetrics(domain, context); if (verb === 'navigate' && domain?.componentName === 'App') {