From 8afd9f8c34bc07a00228e648ba1ed7839e4fd2ff Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 21 Apr 2023 14:53:07 +0200 Subject: [PATCH 01/11] core-app-api: forward route params to navigate event Signed-off-by: Vincenzo Scamporlino --- .../core-app-api/src/routing/RouteTracker.tsx | 52 ++++++++++++++----- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/packages/core-app-api/src/routing/RouteTracker.tsx b/packages/core-app-api/src/routing/RouteTracker.tsx index f9d8c78fc0..7ac373298c 100644 --- a/packages/core-app-api/src/routing/RouteTracker.tsx +++ b/packages/core-app-api/src/routing/RouteTracker.tsx @@ -19,8 +19,8 @@ import { matchRoutes, useLocation } from 'react-router-dom'; import { useAnalytics, AnalyticsContext, - CommonAnalyticsContext, RouteRef, + AnalyticsEventAttributes, } from '@backstage/core-plugin-api'; import { BackstageRouteObject } from './types'; @@ -31,22 +31,23 @@ import { BackstageRouteObject } from './types'; const getExtensionContext = ( pathname: string, routes: BackstageRouteObject[], -): CommonAnalyticsContext | {} => { +) => { try { // Find matching routes for the given path name. - const matches = matchRoutes(routes, { pathname }) as - | { route: BackstageRouteObject }[] - | null; + const matches = matchRoutes(routes, { pathname }); // Of the matching routes, get the last (e.g. most specific) instance of // the BackstageRouteObject. - const routeObject = matches + + const routeMatch = matches ?.filter(match => match?.route.routeRefs?.size > 0) - .pop()?.route; + .pop(); + + const routeObject = routeMatch?.route; // If there is no route object, then allow inheritance of default context. if (!routeObject) { - return {}; + return undefined; } // If there is a single route ref, return it. @@ -56,13 +57,23 @@ const getExtensionContext = ( routeRef = routeObject.routeRefs.values().next().value; } + const params = Object.entries( + routeMatch?.params || {}, + ).reduce((acc, [key, value]) => { + if (value) { + acc[`routeParams_${key}`] = value; + } + return acc; + }, {}); + return { extension: 'App', pluginId: routeObject.plugin?.getId() || 'root', ...(routeRef ? { routeRef: (routeRef as { id?: string }).id } : {}), + params, }; } catch { - return {}; + return undefined; } }; @@ -73,16 +84,19 @@ const TrackNavigation = ({ pathname, search, hash, + attributes, }: { pathname: string; search: string; hash: string; + attributes?: AnalyticsEventAttributes; }) => { const analytics = useAnalytics(); - useEffect(() => { - analytics.captureEvent('navigate', `${pathname}${search}${hash}`); - }, [analytics, pathname, search, hash]); + analytics.captureEvent('navigate', `${pathname}${search}${hash}`, { + attributes, + }); + }, [analytics, pathname, search, hash, attributes]); return null; }; @@ -98,9 +112,19 @@ export const RouteTracker = ({ }) => { const { pathname, search, hash } = useLocation(); + const { params, ...attributes } = getExtensionContext( + pathname, + routeObjects, + ) || { params: {} }; + return ( - - + + ); }; From d23ce9548df51f0b2f3207a64fb822eb47f45cbf Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 21 Apr 2023 14:53:46 +0200 Subject: [PATCH 02/11] core-plugin-api: useAnalytics clean up Signed-off-by: Vincenzo Scamporlino --- .../core-plugin-api/src/analytics/Tracker.ts | 59 ------------------- .../src/analytics/useAnalytics.tsx | 44 ++++++++------ 2 files changed, 27 insertions(+), 76 deletions(-) delete mode 100644 packages/core-plugin-api/src/analytics/Tracker.ts diff --git a/packages/core-plugin-api/src/analytics/Tracker.ts b/packages/core-plugin-api/src/analytics/Tracker.ts deleted file mode 100644 index 51e99bff14..0000000000 --- a/packages/core-plugin-api/src/analytics/Tracker.ts +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { - AnalyticsApi, - AnalyticsEventAttributes, - AnalyticsTracker, -} from '../apis'; -import { AnalyticsContextValue } from './'; - -export class Tracker implements AnalyticsTracker { - constructor( - private readonly analyticsApi: AnalyticsApi, - private context: AnalyticsContextValue = { - routeRef: 'unknown', - pluginId: 'root', - extension: 'App', - }, - ) {} - - setContext(context: AnalyticsContextValue) { - this.context = context; - } - - captureEvent( - action: string, - subject: string, - { - value, - attributes, - }: { value?: number; attributes?: AnalyticsEventAttributes } = {}, - ) { - try { - this.analyticsApi.captureEvent({ - action, - subject, - value, - attributes, - context: this.context, - }); - } catch (e) { - // eslint-disable-next-line no-console - console.warn('Error during analytics event capture. %o', e); - } - } -} diff --git a/packages/core-plugin-api/src/analytics/useAnalytics.tsx b/packages/core-plugin-api/src/analytics/useAnalytics.tsx index 5a047e5a63..29017c167f 100644 --- a/packages/core-plugin-api/src/analytics/useAnalytics.tsx +++ b/packages/core-plugin-api/src/analytics/useAnalytics.tsx @@ -20,9 +20,9 @@ import { AnalyticsTracker, AnalyticsApi, useApi, + AnalyticsEventAttributes, } from '../apis'; -import { useRef } from 'react'; -import { Tracker } from './Tracker'; +import { useMemo } from 'react'; function useAnalyticsApi(): AnalyticsApi { try { @@ -38,22 +38,32 @@ function useAnalyticsApi(): AnalyticsApi { * @public */ export function useAnalytics(): AnalyticsTracker { - const trackerRef = useRef(null); const context = useAnalyticsContext(); - // Our goal is to make this API truly optional for any/all consuming code - // (including tests). This hook runs last to ensure hook order is, as much as - // possible, maintained. const analyticsApi = useAnalyticsApi(); - function getTracker(): Tracker { - if (trackerRef.current === null) { - trackerRef.current = new Tracker(analyticsApi); - } - return trackerRef.current; - } - - const tracker = getTracker(); - tracker.setContext(context); - - return tracker; + return useMemo(() => { + return { + captureEvent( + action: string, + subject: string, + { + value, + attributes, + }: { value?: number; attributes?: AnalyticsEventAttributes } = {}, + ) { + try { + analyticsApi.captureEvent({ + action, + subject, + value, + attributes, + context, + }); + } catch (e) { + // eslint-disable-next-line no-console + console.warn('Error during analytics event capture. %o', e); + } + }, + }; + }, [analyticsApi, context]); } From 3f5d3255b5fd116d0e445419256fd1603f0cdf36 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 21 Apr 2023 19:55:00 +0200 Subject: [PATCH 03/11] core-app-api: remove prefix Signed-off-by: Vincenzo Scamporlino --- packages/core-app-api/src/routing/RouteTracker.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core-app-api/src/routing/RouteTracker.tsx b/packages/core-app-api/src/routing/RouteTracker.tsx index 7ac373298c..5ec858b28e 100644 --- a/packages/core-app-api/src/routing/RouteTracker.tsx +++ b/packages/core-app-api/src/routing/RouteTracker.tsx @@ -60,8 +60,8 @@ const getExtensionContext = ( const params = Object.entries( routeMatch?.params || {}, ).reduce((acc, [key, value]) => { - if (value) { - acc[`routeParams_${key}`] = value; + if (value !== undefined) { + acc[key] = value; } return acc; }, {}); From c89437db899a21efec72236c17303750af3393de Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 21 Apr 2023 20:00:41 +0200 Subject: [PATCH 04/11] changesets Signed-off-by: Vincenzo Scamporlino --- .changeset/green-cheetahs-cover.md | 5 +++++ .changeset/mean-seas-tan.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/green-cheetahs-cover.md create mode 100644 .changeset/mean-seas-tan.md diff --git a/.changeset/green-cheetahs-cover.md b/.changeset/green-cheetahs-cover.md new file mode 100644 index 0000000000..f89ebdfedd --- /dev/null +++ b/.changeset/green-cheetahs-cover.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': minor +--- + +RouteTracker now forwards the route parameters as attributes of the navigate event diff --git a/.changeset/mean-seas-tan.md b/.changeset/mean-seas-tan.md new file mode 100644 index 0000000000..9d99016f45 --- /dev/null +++ b/.changeset/mean-seas-tan.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-plugin-api': patch +--- + +Minor improvements to `useAnalytics` hook From c9c70d75992fb81175ca84873d99bc10703c3801 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 26 Apr 2023 13:46:15 +0200 Subject: [PATCH 05/11] docs: clarify navigate event Signed-off-by: Vincenzo Scamporlino --- docs/plugins/analytics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/plugins/analytics.md b/docs/plugins/analytics.md index e949f7beb4..0c7e0d009d 100644 --- a/docs/plugins/analytics.md +++ b/docs/plugins/analytics.md @@ -54,7 +54,7 @@ installed, may be captured. | Action | Subject | Other Notes | | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `navigate` | The URL of the page that was navigated to. | | +| `navigate` | The URL of the page that was navigated to. | the parameters of the current route will be included as attributes | | `click` | The text of the link that was clicked on. | The `to` attribute represents the URL clicked to. | | `create` | The `name` of the software being created; if no `name` property is requested by the given Software Template, then the string `new {templateName}` is used instead. | The context holds an `entityRef`, set to the template's ref (e.g. `template:default/template-name`). | | `search` | The search term entered in any search bar component. | The context holds `searchTypes`, representing `types` constraining the search. The `value` represents the total number of search results for the query. This may not be visible if the permission framework is being used. | From d4a5dd089023450678dba3c64144eef27a1d5ba7 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 26 Apr 2023 14:00:28 +0200 Subject: [PATCH 06/11] core-app-api: improve changeset Signed-off-by: Vincenzo Scamporlino --- .changeset/green-cheetahs-cover.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/green-cheetahs-cover.md b/.changeset/green-cheetahs-cover.md index f89ebdfedd..2633a5af01 100644 --- a/.changeset/green-cheetahs-cover.md +++ b/.changeset/green-cheetahs-cover.md @@ -2,4 +2,4 @@ '@backstage/core-app-api': minor --- -RouteTracker now forwards the route parameters as attributes of the navigate event +The analytics' `navigate` event will now include the route parameters as attributes of the navigate event From f9e4c279e2eb03e5b55a3a4f814e81748eefdaa6 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 26 Apr 2023 22:16:43 +0200 Subject: [PATCH 07/11] Revert "core-plugin-api: useAnalytics clean up" This reverts commit d7d3e471e1c480cd9ad171840833c551a8af268f. Signed-off-by: Vincenzo Scamporlino --- .changeset/mean-seas-tan.md | 5 -- .../core-plugin-api/src/analytics/Tracker.ts | 59 +++++++++++++++++++ .../src/analytics/useAnalytics.tsx | 44 ++++++-------- 3 files changed, 76 insertions(+), 32 deletions(-) delete mode 100644 .changeset/mean-seas-tan.md create mode 100644 packages/core-plugin-api/src/analytics/Tracker.ts diff --git a/.changeset/mean-seas-tan.md b/.changeset/mean-seas-tan.md deleted file mode 100644 index 9d99016f45..0000000000 --- a/.changeset/mean-seas-tan.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/core-plugin-api': patch ---- - -Minor improvements to `useAnalytics` hook diff --git a/packages/core-plugin-api/src/analytics/Tracker.ts b/packages/core-plugin-api/src/analytics/Tracker.ts new file mode 100644 index 0000000000..51e99bff14 --- /dev/null +++ b/packages/core-plugin-api/src/analytics/Tracker.ts @@ -0,0 +1,59 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + AnalyticsApi, + AnalyticsEventAttributes, + AnalyticsTracker, +} from '../apis'; +import { AnalyticsContextValue } from './'; + +export class Tracker implements AnalyticsTracker { + constructor( + private readonly analyticsApi: AnalyticsApi, + private context: AnalyticsContextValue = { + routeRef: 'unknown', + pluginId: 'root', + extension: 'App', + }, + ) {} + + setContext(context: AnalyticsContextValue) { + this.context = context; + } + + captureEvent( + action: string, + subject: string, + { + value, + attributes, + }: { value?: number; attributes?: AnalyticsEventAttributes } = {}, + ) { + try { + this.analyticsApi.captureEvent({ + action, + subject, + value, + attributes, + context: this.context, + }); + } catch (e) { + // eslint-disable-next-line no-console + console.warn('Error during analytics event capture. %o', e); + } + } +} diff --git a/packages/core-plugin-api/src/analytics/useAnalytics.tsx b/packages/core-plugin-api/src/analytics/useAnalytics.tsx index 29017c167f..5a047e5a63 100644 --- a/packages/core-plugin-api/src/analytics/useAnalytics.tsx +++ b/packages/core-plugin-api/src/analytics/useAnalytics.tsx @@ -20,9 +20,9 @@ import { AnalyticsTracker, AnalyticsApi, useApi, - AnalyticsEventAttributes, } from '../apis'; -import { useMemo } from 'react'; +import { useRef } from 'react'; +import { Tracker } from './Tracker'; function useAnalyticsApi(): AnalyticsApi { try { @@ -38,32 +38,22 @@ function useAnalyticsApi(): AnalyticsApi { * @public */ export function useAnalytics(): AnalyticsTracker { + const trackerRef = useRef(null); const context = useAnalyticsContext(); + // Our goal is to make this API truly optional for any/all consuming code + // (including tests). This hook runs last to ensure hook order is, as much as + // possible, maintained. const analyticsApi = useAnalyticsApi(); - return useMemo(() => { - return { - captureEvent( - action: string, - subject: string, - { - value, - attributes, - }: { value?: number; attributes?: AnalyticsEventAttributes } = {}, - ) { - try { - analyticsApi.captureEvent({ - action, - subject, - value, - attributes, - context, - }); - } catch (e) { - // eslint-disable-next-line no-console - console.warn('Error during analytics event capture. %o', e); - } - }, - }; - }, [analyticsApi, context]); + function getTracker(): Tracker { + if (trackerRef.current === null) { + trackerRef.current = new Tracker(analyticsApi); + } + return trackerRef.current; + } + + const tracker = getTracker(); + tracker.setContext(context); + + return tracker; } From bf2c56797ec72d2288188e88237ea8acfe51d947 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 26 Apr 2023 22:21:37 +0200 Subject: [PATCH 08/11] core-plugin-api: add clarifying comment Signed-off-by: Vincenzo Scamporlino --- packages/core-plugin-api/src/analytics/useAnalytics.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core-plugin-api/src/analytics/useAnalytics.tsx b/packages/core-plugin-api/src/analytics/useAnalytics.tsx index 5a047e5a63..50cec8f790 100644 --- a/packages/core-plugin-api/src/analytics/useAnalytics.tsx +++ b/packages/core-plugin-api/src/analytics/useAnalytics.tsx @@ -53,6 +53,8 @@ export function useAnalytics(): AnalyticsTracker { } const tracker = getTracker(); + // this is not ideal, but it allows to memoize the tracker + // without explicitly set the context as dependency. tracker.setContext(context); return tracker; From 19348c5990b73d2989c0e831925784de91cad355 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 26 Apr 2023 23:08:56 +0200 Subject: [PATCH 09/11] core-app-api: add route params test Signed-off-by: Vincenzo Scamporlino --- .../src/routing/RouteTracker.test.tsx | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 packages/core-app-api/src/routing/RouteTracker.test.tsx diff --git a/packages/core-app-api/src/routing/RouteTracker.test.tsx b/packages/core-app-api/src/routing/RouteTracker.test.tsx new file mode 100644 index 0000000000..9472b523aa --- /dev/null +++ b/packages/core-app-api/src/routing/RouteTracker.test.tsx @@ -0,0 +1,115 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { TestApiProvider } from '@backstage/test-utils'; +import React from 'react'; +import { BackstageRouteObject } from './types'; +import { fireEvent, render, waitFor } from '@testing-library/react'; +import { RouteTracker } from './RouteTracker'; +import { Link, MemoryRouter, Route, Routes } from 'react-router-dom'; +import { + AnalyticsApi, + analyticsApiRef, + createRouteRef, +} from '@backstage/core-plugin-api'; + +describe('RouteTracker', () => { + const routeRef1 = createRouteRef({ + id: 'route1', + }); + const routeRef2 = createRouteRef({ + id: 'route2', + }); + + const routeObjects: BackstageRouteObject[] = [ + { + path: '/path/:p1/:p2', + element: go, + routeRefs: new Set([routeRef1]), + caseSensitive: false, + }, + { + path: '/path2/:param', + element:
hi there
, + routeRefs: new Set([routeRef2]), + caseSensitive: false, + }, + ]; + + const mockedAnalytics: jest.Mocked = { + captureEvent: jest.fn(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should capture the navigate event on load', async () => { + render( + + + + + , + ); + + expect(mockedAnalytics.captureEvent).toHaveBeenCalledWith({ + action: 'navigate', + attributes: { + p1: 'foo', + p2: 'bar', + }, + context: { + extension: 'App', + pluginId: 'root', + routeRef: 'route1', + }, + subject: '/path/foo/bar', + value: undefined, + }); + }); + + it('should capture the navigate event on route change', async () => { + const { getByText } = render( + + + + + + {routeObjects.map(({ routeRefs, ...props }) => ( + + ))} + + + , + ); + + fireEvent.click(getByText('go')); + + expect(mockedAnalytics.captureEvent).toHaveBeenCalledWith({ + action: 'navigate', + attributes: { + param: 'hello', + }, + context: { + extension: 'App', + pluginId: 'root', + routeRef: 'route2', + }, + subject: '/path2/hello', + value: undefined, + }); + }); +}); From e755aa0240af7822d0da0170af1e8f3b89047c08 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 27 Apr 2023 15:09:13 +0200 Subject: [PATCH 10/11] Update docs/plugins/analytics.md Co-authored-by: Eric Peterson Signed-off-by: Vincenzo Scamporlino --- docs/plugins/analytics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/plugins/analytics.md b/docs/plugins/analytics.md index 0c7e0d009d..fa9aed7fe5 100644 --- a/docs/plugins/analytics.md +++ b/docs/plugins/analytics.md @@ -54,7 +54,7 @@ installed, may be captured. | Action | Subject | Other Notes | | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `navigate` | The URL of the page that was navigated to. | the parameters of the current route will be included as attributes | +| `navigate` | The URL of the page that was navigated to. | The parameters of the current route will be included as attributes | | `click` | The text of the link that was clicked on. | The `to` attribute represents the URL clicked to. | | `create` | The `name` of the software being created; if no `name` property is requested by the given Software Template, then the string `new {templateName}` is used instead. | The context holds an `entityRef`, set to the template's ref (e.g. `template:default/template-name`). | | `search` | The search term entered in any search bar component. | The context holds `searchTypes`, representing `types` constraining the search. The `value` represents the total number of search results for the query. This may not be visible if the permission framework is being used. | From 46f0dcef7a379b776d42572344cb8872947223bf Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 27 Apr 2023 15:12:10 +0200 Subject: [PATCH 11/11] core-app-api: remove unused method Signed-off-by: Vincenzo Scamporlino --- packages/core-app-api/src/routing/RouteTracker.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-app-api/src/routing/RouteTracker.test.tsx b/packages/core-app-api/src/routing/RouteTracker.test.tsx index 9472b523aa..6896b60169 100644 --- a/packages/core-app-api/src/routing/RouteTracker.test.tsx +++ b/packages/core-app-api/src/routing/RouteTracker.test.tsx @@ -16,7 +16,7 @@ import { TestApiProvider } from '@backstage/test-utils'; import React from 'react'; import { BackstageRouteObject } from './types'; -import { fireEvent, render, waitFor } from '@testing-library/react'; +import { fireEvent, render } from '@testing-library/react'; import { RouteTracker } from './RouteTracker'; import { Link, MemoryRouter, Route, Routes } from 'react-router-dom'; import {