Review feedback.

Co-authored-by: Camila Belo <camilaibs@gmail.com>
Co-authored-by: Fredrik Adelöw <freben@gmail.com>
Signed-off-by: Eric Peterson <ericpeterson@spotify.com>

packages/core-app-api/src/routing/RouteTracker.tsx

@@ -40,15 +40,15 @@ const getExtensionContext = (
 ): CommonAnalyticsContext | {} => {
   try {
     // Find matching routes for the given path name.
-    const matches = matchRoutes(routes, { pathname });
+    const matches = matchRoutes(routes, { pathname }) as
+      | { route: BackstageRouteObject }[]
+      | null;
 
     // Of the matching routes, get the last (e.g. most specific) instance of
     // the BackstageRouteObject.
     const routeObject = matches
-      ?.filter(
-        match => (match?.route as BackstageRouteObject).routeRefs?.size > 0,
-      )
-      .pop()?.route as BackstageRouteObject;
+      ?.filter(match => match?.route.routeRefs?.size > 0)
+      .pop()?.route;
 
     // If there is no route object, then allow inheritance of default context.
     if (!routeObject) {

packages/core-components/src/components/Link/Link.test.tsx

@@ -15,13 +15,12 @@
  */
 
 import React from 'react';
-import { render, fireEvent } from '@testing-library/react';
+import { render, fireEvent, waitFor } from '@testing-library/react';
 import { MockAnalyticsApi, wrapInTestApp } from '@backstage/test-utils';
 import { ApiProvider, ApiRegistry } from '@backstage/core-app-api';
 import { analyticsApiRef } from '@backstage/core-plugin-api';
 import { isExternalUri, Link } from './Link';
 import { Route, Routes } from 'react-router';
-import { act } from 'react-dom/test-utils';
 
 describe('<Link />', () => {
   it('navigates using react-router', async () => {
@@ -36,10 +35,10 @@ describe('<Link />', () => {
       ),
     );
     expect(() => getByText(testString)).toThrow();
-    await act(async () => {
-      fireEvent.click(getByText(linkText));
+    fireEvent.click(getByText(linkText));
+    await waitFor(() => {
+      expect(getByText(testString)).toBeInTheDocument();
     });
-    expect(getByText(testString)).toBeInTheDocument();
   });
 
   it('captures click using analytics api', async () => {
@@ -57,18 +56,18 @@ describe('<Link />', () => {
       ),
     );
 
-    await act(async () => {
-      fireEvent.click(getByText(linkText));
-    });
+    fireEvent.click(getByText(linkText));
 
     // Analytics event should have been fired.
-    expect(analyticsApi.getEvents()[0]).toMatchObject({
-      action: 'click',
-      subject: '/test',
-    });
+    await waitFor(() => {
+      expect(analyticsApi.getEvents()[0]).toMatchObject({
+        action: 'click',
+        subject: '/test',
+      });
 
-    // Custom onClick handler should have still been fired too.
-    expect(customOnClick).toHaveBeenCalled();
+      // Custom onClick handler should have still been fired too.
+      expect(customOnClick).toHaveBeenCalled();
+    });
   });
 
   describe('isExternalUri', () => {

packages/core-components/src/components/Link/Link.tsx

@@ -47,9 +47,7 @@ const ActualLink = React.forwardRef<any, LinkProps>(
     const newWindow = external && !!/^https?:/.exec(to);
 
     const handleClick = (event: React.MouseEvent<any, MouseEvent>) => {
-      if (onClick !== undefined) {
-        onClick(event);
-      }
+      onClick?.(event);
       analytics.captureEvent('click', to);
     };
 

packages/core-plugin-api/api-report.md

@@ -53,17 +53,15 @@ export const AnalyticsContext: ({
   attributes,
   children,
 }: {
-  attributes: AnalyticsContextValue;
+  attributes: Partial<AnalyticsContextValue>;
   children: ReactNode;
 }) => JSX.Element;
 
-// Warning: (ae-forgotten-export) The symbol "AnyAnalyticsContext" needs to be exported by the entry point index.d.ts
 // Warning: (ae-missing-release-tag) "AnalyticsContextValue" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
 // @public
-export type AnalyticsContextValue = Partial<
-  CommonAnalyticsContext & AnyAnalyticsContext
->;
+export type AnalyticsContextValue = CommonAnalyticsContext &
+  AnyAnalyticsContext;
 
 // Warning: (ae-missing-release-tag) "AnalyticsEvent" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
@@ -97,6 +95,13 @@ export type AnalyticsTracker = {
   ) => void;
 };
 
+// Warning: (ae-missing-release-tag) "AnyAnalyticsContext" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
+//
+// @public
+export type AnyAnalyticsContext = {
+  [param in string]: string | boolean | number | undefined;
+};
+
 // Warning: (ae-missing-release-tag) "AnyApiFactory" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
 // @public (undocumented)

packages/core-plugin-api/src/analytics/AnalyticsContext.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import React, { createContext, ReactNode, useContext } from 'react';
+import React, { createContext, ReactNode, useContext, useMemo } from 'react';
 import { AnalyticsContextValue } from './types';
 
 const AnalyticsReactContext = createContext<AnalyticsContextValue>({
@@ -43,14 +43,17 @@ export const AnalyticsContext = ({
   attributes,
   children,
 }: {
-  attributes: AnalyticsContextValue;
+  attributes: Partial<AnalyticsContextValue>;
   children: ReactNode;
 }) => {
   const parentValues = useAnalyticsContext();
-  const combinedValue = {
-    ...parentValues,
-    ...attributes,
-  };
+  const combinedValue = useMemo(
+    () => ({
+      ...parentValues,
+      ...attributes,
+    }),
+    [parentValues, attributes],
+  );
 
   return (
     <AnalyticsReactContext.Provider value={combinedValue}>

packages/core-plugin-api/src/analytics/Tracker.ts

@@ -14,13 +14,21 @@
  * limitations under the License.
  */
 
-import { AnalyticsApi, AnalyticsEventAttributes } from '../apis';
+import {
+  AnalyticsApi,
+  AnalyticsEventAttributes,
+  AnalyticsTracker,
+} from '../apis';
 import { AnalyticsContextValue } from './';
 
-export class Tracker {
+export class Tracker implements AnalyticsTracker {
   constructor(
     private readonly analyticsApi: AnalyticsApi,
-    private context: AnalyticsContextValue = {},
+    private context: AnalyticsContextValue = {
+      routeRef: 'unknown',
+      pluginId: 'root',
+      extension: 'App',
+    },
   ) {}
 
   setContext(context: AnalyticsContextValue) {

packages/core-plugin-api/src/analytics/index.ts

@@ -15,5 +15,9 @@
  */
 
 export { AnalyticsContext } from './AnalyticsContext';
-export type { AnalyticsContextValue, CommonAnalyticsContext } from './types';
+export type {
+  AnalyticsContextValue,
+  AnyAnalyticsContext,
+  CommonAnalyticsContext,
+} from './types';
 export { useAnalytics } from './useAnalytics';

packages/core-plugin-api/src/analytics/types.ts

@@ -19,17 +19,17 @@
  */
 export type CommonAnalyticsContext = {
   /**
-   * The associated plugin.
+   * The nearest known parent plugin where the event was captured.
    */
   pluginId: string;
 
   /**
-   * The ID of the associated route ref.
+   * The ID of the routeRef that was active when the event was captured.
    */
   routeRef: string;
 
   /**
-   * The name of the associated extension.
+   * The nearest known parent extension where the event was captured.
    */
   extension: string;
 };
@@ -37,13 +37,12 @@ export type CommonAnalyticsContext = {
 /**
  * Allow arbitrary scalar values as context attributes too.
  */
-type AnyAnalyticsContext = {
+export type AnyAnalyticsContext = {
   [param in string]: string | boolean | number | undefined;
 };
 
 /**
  * Analytics context envelope.
  */
-export type AnalyticsContextValue = Partial<
-  CommonAnalyticsContext & AnyAnalyticsContext
->;
+export type AnalyticsContextValue = CommonAnalyticsContext &
+  AnyAnalyticsContext;

packages/core-plugin-api/src/analytics/useAnalytics.tsx

@@ -20,11 +20,21 @@ import { useRef } from 'react';
 import { Tracker } from './Tracker';
 
 function useTracker(): AnalyticsTracker {
+  const trackerRef = useRef<Tracker | null>(null);
   const analyticsApi = useApi(analyticsApiRef);
-  const tracker = useRef(new Tracker(analyticsApi));
   const context = useAnalyticsContext();
-  tracker.current.setContext(context);
-  return tracker.current;
+
+  function getTracker(): Tracker {
+    if (trackerRef.current === null) {
+      trackerRef.current = new Tracker(analyticsApi);
+    }
+    return trackerRef.current;
+  }
+
+  const tracker = getTracker();
+  tracker.setContext(context);
+
+  return tracker;
 }
 
 /**

packages/core-plugin-api/src/extensions/extensions.test.tsx

@@ -126,7 +126,7 @@ describe('extensions', () => {
             );
           },
         },
-        data: { 'core.mountpoint': { id: 'some-ref' } },
+        data: { 'core.mountPoint': { id: 'some-ref' } },
       }),
     );
 

packages/core-plugin-api/src/extensions/extensions.tsx

@@ -133,6 +133,9 @@ export function createReactExtension<
       const Result: any = (props: any) => {
         const app = useApp();
         const { Progress } = app.getComponents();
+        const mountPoint = data?.['core.mountPoint'] as
+          | { id?: string }
+          | undefined;
 
         return (
           <Suspense fallback={<Progress />}>
@@ -140,13 +143,8 @@ export function createReactExtension<
               <AnalyticsContext
                 attributes={{
                   pluginId: plugin.getId(),
-                  ...(options.name ? { extension: options.name } : {}),
-                  ...(data['core.mountpoint']
-                    ? {
-                        routeRef: (data['core.mountpoint'] as { id?: string })
-                          .id,
-                      }
-                    : {}),
+                  ...(options.name && { extension: options.name }),
+                  ...(mountPoint && { routeRef: mountPoint.id }),
                 }}
               >
                 <Component {...props} />

packages/test-utils/src/testUtils/apis/AnalyticsApi/MockAnalyticsApi.test.ts

@@ -19,6 +19,8 @@ import { MockAnalyticsApi } from './MockAnalyticsApi';
 describe('MockAnalyticsApi', () => {
   const context = {
     pluginId: 'some-plugin',
+    routeRef: 'some-route-ref',
+    extension: 'some-extension',
   };
 
   it('should collect events', () => {

plugins/analytics-module-ga/README.md

@@ -8,9 +8,9 @@ This plugin contains no other functionality.
 
 ## Installation
 
-1. Install this plugin in your Backstage app:
+1. Install the plugin package in your Backstage app:
    `cd packages/app && yarn add @backstage/plugin-analytics-module-ga`
-2. Add the API implementation to your App and configure the plugin (see below).
+2. Wire up the API implementation to your App:
 
 ```tsx
 // packages/app/src/apis.ts
@@ -27,7 +27,7 @@ export const apis: AnyApiFactory[] = [
 ];
 ```
 
-## Configuration
+3. Configure the plugin in your `app-config.yaml`:
 
 The following is the minimum configuration required to start sending analytics
 events to GA. All that's needed is your Universal Analytics tracking ID:
@@ -41,11 +41,11 @@ app:
       trackingId: UA-0000000-0
 ```
 
-### Plugin Analytics
+## Configuration
 
 In order to be able to analyze usage of your Backstage instance _by plugin_, we
-strongly recommend configuring at least one custom dimension to capture Plugin
-IDs associated with events, including page views.
+strongly recommend configuring at least one [custom dimension][what-is-a-custom-dimension]
+to capture Plugin IDs associated with events, including page views.
 
 1. First, [configure the custom dimension in GA][configure-custom-dimension].
    Be sure to set the Scope to `hit`, and name it something like `Plugin`. Note
@@ -84,6 +84,10 @@ app:
           index: 2
           source: context
           key: routeRef
+        - type: dimension
+          index: 3
+          source: context
+          key: extension
         - type: metric
           index: 1
           source: attributes
@@ -104,6 +108,9 @@ app:
       debug: true # Logs analytics event to the web console
 ```
 
+You might commonly set the above in an `app-config.local.yaml` file, which is
+normally `gitignore`'d but loaded and merged in when Backstage is bootstrapped.
+
 ## Development
 
 If you would like to contribute improvements to this plugin, the easiest way to
@@ -141,4 +148,5 @@ app:
           key: pluginId
 ```
 
-[configure-custom-dimension]: https://support.google.com/analytics/answer/2709828?hl=en#configuration
+[what-is-a-custom-dimension]: https://support.google.com/analytics/answer/2709828
+[configure-custom-dimension]: https://support.google.com/analytics/answer/2709828#configuration

plugins/analytics-module-ga/package.json

@@ -4,6 +4,7 @@
   "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
+  "private": false,
   "publishConfig": {
     "access": "public",
     "main": "dist/index.esm.js",

plugins/analytics-module-ga/src/apis/implementations/AnalyticsApi/GoogleAnalytics.test.ts

@@ -53,6 +53,7 @@ describe('GoogleAnalytics', () => {
     const context = {
       extension: 'App',
       pluginId: 'some-plugin',
+      routeRef: 'unknown',
       releaseNum: 1337,
     };
     const advancedConfig = new ConfigReader({

plugins/analytics-module-ga/src/apis/implementations/AnalyticsApi/GoogleAnalytics.ts

@@ -23,25 +23,18 @@ import {
 } from '@backstage/core-plugin-api';
 import { Config } from '@backstage/config';
 
-type CDM = {
+type CustomDimensionOrMetricConfig = {
   type: 'dimension' | 'metric';
   index: number;
   source: 'context' | 'attributes';
   key: string;
 };
 
-/**
- * Type guard for emptiness check in array filter.
- */
-function notEmpty<T>(value: T | undefined): value is T {
-  return value !== undefined;
-}
-
 /**
  * Google Analytics API provider for the Backstage Analytics API.
  */
 export class GoogleAnalytics implements AnalyticsApi {
-  private readonly cdmConfig: CDM[];
+  private readonly cdmConfig: CustomDimensionOrMetricConfig[];
 
   /**
    * Instantiate the implementation and initialize ReactGA.
@@ -52,7 +45,7 @@ export class GoogleAnalytics implements AnalyticsApi {
     testMode,
     debug,
   }: {
-    cdmConfig: CDM[];
+    cdmConfig: CustomDimensionOrMetricConfig[];
     trackingId: string;
     testMode: boolean;
     debug: boolean;
@@ -69,19 +62,22 @@ export class GoogleAnalytics implements AnalyticsApi {
   static fromConfig(config: Config) {
     // Get all necessary configuration.
     const trackingId = config.getString('app.analytics.ga.trackingId');
-    const debug = !!config.getOptionalBoolean('app.analytics.ga.debug');
-    const testMode = !!config.getOptionalBoolean('app.analytics.ga.testMode');
+    const debug = config.getOptionalBoolean('app.analytics.ga.debug') ?? false;
+    const testMode =
+      config.getOptionalBoolean('app.analytics.ga.testMode') ?? false;
     const cdmConfig =
       config
         .getOptionalConfigArray('app.analytics.ga.customDimensionsMetrics')
         ?.map(c => {
           return {
-            type: c.getString('type') as CDM['type'],
+            type: c.getString('type') as CustomDimensionOrMetricConfig['type'],
             index: c.getNumber('index'),
-            source: c.getString('source') as CDM['source'],
+            source: c.getString(
+              'source',
+            ) as CustomDimensionOrMetricConfig['source'],
             key: c.getString('key'),
           };
-        }) || [];
+        }) ?? [];
 
     // Return an implementation instance.
     return new GoogleAnalytics({
@@ -106,7 +102,7 @@ export class GoogleAnalytics implements AnalyticsApi {
   }: AnalyticsEvent) {
     const customMetadata = this.getCustomDimensionMetrics(context, attributes);
 
-    if (action === 'navigate' && context?.extension === 'App') {
+    if (action === 'navigate' && context.extension === 'App') {
       // Set any/all custom dimensions.
       if (Object.keys(customMetadata).length) {
         ReactGA.set(customMetadata);
@@ -127,34 +123,32 @@ export class GoogleAnalytics implements AnalyticsApi {
 
   /**
    * Returns an object of dimensions/metrics given an Analytics Context and an
-   * Event Context, e.g. { dimension1: "some value", metric8: 42 }
+   * Event Attributes, e.g. { dimension1: "some value", metric8: 42 }
    */
   private getCustomDimensionMetrics(
     context: AnalyticsContextValue,
     attributes: AnalyticsEventAttributes = {},
   ) {
-    const dataArray = this.cdmConfig
-      .map(cdm => {
-        const value =
-          cdm.source === 'context' ? context[cdm.key] : attributes[cdm.key];
+    const customDimensionsMetrics: { [x: string]: string | number | boolean } =
+      {};
 
-        // Never pass a non-numeric value on a metric.
-        if (cdm.type === 'metric' && typeof value !== 'number') {
-          return undefined;
-        }
+    this.cdmConfig.forEach(config => {
+      const value =
+        config.source === 'context'
+          ? context[config.key]
+          : attributes[config.key];
 
-        return value !== undefined
-          ? {
-              [`${cdm.type}${cdm.index}`]: value,
-            }
-          : undefined;
-      })
-      .filter(notEmpty);
+      // Never pass a non-numeric value on a metric.
+      if (config.type === 'metric' && typeof value !== 'number') {
+        return;
+      }
 
-    return dataArray.length
-      ? dataArray.reduce((result, cd) => {
-          return Object.assign(result, cd);
-        })
-      : {};
+      // Only set defined values.
+      if (value !== undefined) {
+        customDimensionsMetrics[`${config.type}${config.index}`] = value;
+      }
+    });
+
+    return customDimensionsMetrics;
   }
 }