Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: remove ReactGA and migrate to GA4 for tracking #2071

Merged
merged 7 commits into from
Dec 26, 2023
Merged

feat: remove ReactGA and migrate to GA4 for tracking #2071

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
47ac437
feat: remove ReactGA and migrate to GA4
EshaanAgg Dec 25, 2023
23098d7
fix types and implementation
EshaanAgg Dec 26, 2023
daacac3
update tests
EshaanAgg Dec 26, 2023
6a48640
fix leaking of window type
EshaanAgg Dec 26, 2023
92d8377
Merge branch 'main' into b3
EshaanAgg Dec 26, 2023
ada2383
remove logging utils
EshaanAgg Dec 26, 2023
a5aa8c7
Merge branch 'main' into b3
EshaanAgg Dec 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/jaeger-ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@
"react": "^18.2.0",
"react-circular-progressbar": "^2.1.0",
"react-dom": "^18.2.0",
"react-ga": "^3.3.1",
"react-helmet": "^6.1.0",
"react-icons": "^4.10.1",
"react-is": "^18.2.0",
Expand Down
71 changes: 36 additions & 35 deletions packages/jaeger-ui/src/utils/tracking/ga.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import ReactGA from 'react-ga';
import * as GA from './ga';
import * as utils from './utils';
import { getVersionInfo, getAppEnvironment } from '../constants';
import { getAppEnvironment } from '../constants';

jest.mock('./conv-raven-to-ga', () => () => ({
category: 'jaeger/a',
Expand All @@ -35,7 +34,6 @@ function getStr(len) {
}

describe('google analytics tracking', () => {
let calls;
let tracking;

beforeAll(() => {
Expand All @@ -54,16 +52,18 @@ describe('google analytics tracking', () => {
});

beforeEach(() => {
getVersionInfo.mockReturnValue('{}');
calls = ReactGA.testModeAPI.calls;
calls.length = 0;
jest.useFakeTimers();
// Set an arbitrary date so that we can test the date-based dimension
jest.setSystemTime(new Date('2023-01-01'));
window.dataLayer = [];
});

describe('init', () => {
it('check init function (no cookies)', () => {
tracking.init();
expect(calls).toEqual([
['create', 'UA-123456', 'auto'],
expect(window.dataLayer).toEqual([
['js', new Date()],
['config', 'UA-123456'],
[
'set',
{
Expand All @@ -78,8 +78,9 @@ describe('google analytics tracking', () => {
it('check init function (no cookies)', () => {
document.cookie = 'page=1;';
tracking.init();
expect(calls).toEqual([
['create', 'UA-123456', 'auto'],
expect(window.dataLayer).toEqual([
['js', new Date()],
['config', 'UA-123456'],
[
'set',
{
Expand All @@ -96,33 +97,33 @@ describe('google analytics tracking', () => {
describe('trackPageView', () => {
it('tracks a page view', () => {
tracking.trackPageView('a', 'b');
expect(calls).toEqual([['send', { hitType: 'pageview', page: 'ab' }]]);
expect(window.dataLayer).toEqual([['event', 'page_view', { page_path: 'ab' }]]);
});

it('ignores search when it is falsy', () => {
tracking.trackPageView('a');
expect(calls).toEqual([['send', { hitType: 'pageview', page: 'a' }]]);
expect(window.dataLayer).toEqual([['event', 'page_view', { page_path: 'a' }]]);
});
});

describe('trackError', () => {
it('tracks an error', () => {
tracking.trackError('a');
expect(calls).toEqual([
['send', { hitType: 'exception', exDescription: expect.any(String), exFatal: false }],
expect(window.dataLayer).toEqual([
['event', 'exception', { description: expect.any(String), fatal: false }],
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
]);
});

it('ensures "jaeger" is prepended', () => {
tracking.trackError('a');
expect(calls).toEqual([['send', { hitType: 'exception', exDescription: 'jaeger/a', exFatal: false }]]);
expect(window.dataLayer).toEqual([['event', 'exception', { description: 'jaeger/a', fatal: false }]]);
});

it('truncates if needed', () => {
const str = `jaeger/${getStr(200)}`;
tracking.trackError(str);
expect(calls).toEqual([
['send', { hitType: 'exception', exDescription: str.slice(0, 149), exFatal: false }],
expect(window.dataLayer).toEqual([
['event', 'exception', { description: str.slice(0, 149), fatal: false }],
]);
});
});
Expand All @@ -132,13 +133,12 @@ describe('google analytics tracking', () => {
const category = 'jaeger/some-category';
const action = 'some-action';
tracking.trackEvent(category, action);
expect(calls).toEqual([
expect(window.dataLayer).toEqual([
[
'send',
'event',
'some-action',
{
hitType: 'event',
eventCategory: category,
eventAction: action,
event_category: category,
},
],
]);
Expand All @@ -148,33 +148,32 @@ describe('google analytics tracking', () => {
const category = 'some-category';
const action = 'some-action';
tracking.trackEvent(category, action);
expect(calls).toEqual([
['send', { hitType: 'event', eventCategory: `jaeger/${category}`, eventAction: action }],
]);
expect(window.dataLayer).toEqual([['event', 'some-action', { event_category: `jaeger/${category}` }]]);
});

it('truncates values, if needed', () => {
const str = `jaeger/${getStr(600)}`;
tracking.trackEvent(str, str, str);
expect(calls).toEqual([
expect(window.dataLayer).toEqual([
[
'send',
'event',
str.slice(0, 499),
{
hitType: 'event',
eventCategory: str.slice(0, 149),
eventAction: str.slice(0, 499),
eventLabel: str.slice(0, 499),
event_category: str.slice(0, 149),
event_label: str.slice(0, 499),
},
],
]);
});
});

it('converting raven-js errors', () => {
window.onunhandledrejection({ reason: new Error('abc') });
expect(calls).toEqual([
['send', { hitType: 'exception', exDescription: expect.any(String), exFatal: false }],
['send', { hitType: 'event', eventCategory: expect.any(String), eventAction: expect.any(String) }],
window.onunhandledrejection({
reason: new Error('abc'),
});
expect(window.dataLayer).toEqual([
['event', 'exception', { description: expect.any(String), fatal: false }],
['event', expect.any(String), { event_category: expect.any(String) }],
]);
});

Expand Down Expand Up @@ -209,10 +208,12 @@ describe('google analytics tracking', () => {
it('isDebugMode = true', () => {
// eslint-disable-next-line no-import-assign
utils.logTrackingCalls = jest.fn();

trackingDebug.init();
trackingDebug.trackError();
trackingDebug.trackEvent('jaeger/some-category', 'some-action');
trackingDebug.trackPageView('a', 'b');

expect(utils.logTrackingCalls).toHaveBeenCalledTimes(4);
});
});
Expand Down
67 changes: 59 additions & 8 deletions packages/jaeger-ui/src/utils/tracking/ga.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

import _get from 'lodash/get';
import ReactGA from 'react-ga';
import Raven, { RavenOptions, RavenTransportOptions } from 'raven-js';

import convRavenToGa from './conv-raven-to-ga';
Expand All @@ -24,6 +23,40 @@ import { logTrackingCalls } from './utils';
import { getAppEnvironment, shouldDebugGoogleAnalytics } from '../constants';
import parseQuery from '../parseQuery';

// Modify the `window` object to have an additional attribute `dataLayer`
// This is required by the gtag.js script to work

// eslint-disable-next-line @typescript-eslint/naming-convention
export interface WindowWithGATracking extends Window {
dataLayer: (string | object)[][] | undefined;
}

declare let window: WindowWithGATracking;

// Function to add a new event to the Google Analytics dataLayer
const gtag = (...args: (string | object)[]) => {
if (window !== undefined)
if (window.dataLayer !== undefined) {
EshaanAgg marked this conversation as resolved.
Show resolved Hide resolved
window.dataLayer.push(args);
}
};

// Function to initialize the Google Analytics script
const initGA = (GA_MEASUREMENT_ID: string) => {
const gtagUrl = 'https://www.googletagmanager.com/gtag/js';

// Load the script asynchronously
const script = document.createElement('script');
script.async = true;
script.src = `${gtagUrl}?id=${GA_MEASUREMENT_ID}`;
document.body.appendChild(script);

// Initialize the dataLayer and send initial configuration data
window.dataLayer = window.dataLayer || [];
gtag('js', new Date());
gtag('config', GA_MEASUREMENT_ID);
};

const isTruish = (value?: string | string[]) => {
return Boolean(value) && value !== '0' && value !== 'false';
};
Expand Down Expand Up @@ -56,7 +89,12 @@ const GA: IWebAnalyticsFunc = (config: Config, versionShort: string, versionLong
msg = `jaeger/${msg}`;
}
msg = msg.slice(0, 149);
ReactGA.exception({ description: msg, fatal: false });

gtag('event', 'exception', {
description: msg,
fatal: false,
});

if (isDebugMode) {
logTrackingCalls();
}
Expand Down Expand Up @@ -90,7 +128,13 @@ const GA: IWebAnalyticsFunc = (config: Config, versionShort: string, versionLong
if (value != null) {
event.value = Math.round(value);
}
ReactGA.event(event);

gtag('event', event.action, {
event_category: event.category,
...(event.label && { event_label: event.label }),
...(event.value && { event_value: event.value }),
});

if (isDebugMode) {
logTrackingCalls();
}
Expand All @@ -107,18 +151,23 @@ const GA: IWebAnalyticsFunc = (config: Config, versionShort: string, versionLong
return;
}

const gaConfig = { testMode: isTest || isDebugMode, titleCase: false, debug: true };
ReactGA.initialize(gaID || 'debug-mode', gaConfig);
ReactGA.set({
initGA(gaID || 'debug-mode');

gtag('set', {
appId: 'github.com/jaegertracing/jaeger-ui',
appName: 'Jaeger UI',
appVersion: versionLong,
});

if (cookiesToDimensions !== undefined) {
(cookiesToDimensions as unknown as Array<{ cookie: string; dimension: string }>).forEach(
({ cookie, dimension }: { cookie: string; dimension: string }) => {
const match = ` ${document.cookie}`.match(new RegExp(`[; ]${cookie}=([^\\s;]*)`));
if (match) ReactGA.set({ [dimension]: match[1] });
if (match) {
gtag('set', {
[dimension]: match[1],
});
}
// eslint-disable-next-line no-console
else console.warn(`${cookie} not present in cookies, could not set dimension: ${dimension}`);
}
Expand Down Expand Up @@ -152,7 +201,9 @@ const GA: IWebAnalyticsFunc = (config: Config, versionShort: string, versionLong

const trackPageView = (pathname: string, search: string | TNil) => {
const pagePath = search ? `${pathname}${search}` : pathname;
ReactGA.pageview(pagePath);
gtag('event', 'page_view', {
page_path: pagePath,
});
if (isDebugMode) {
logTrackingCalls();
}
Expand Down
13 changes: 6 additions & 7 deletions packages/jaeger-ui/src/utils/tracking/utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import ReactGA from 'react-ga';
import { WindowWithGATracking } from './ga';

declare let window: WindowWithGATracking;

// eslint-disable-next-line import/prefer-default-export
export const logTrackingCalls = () => {
Copy link
Member

Choose a reason for hiding this comment

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

can we move this function inside ga.tsx (and tests)? I don't see it being used outside, so odd to expose as public, especially since the dataLayer contact is for GA only.

Copy link
Member

Choose a reason for hiding this comment

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

in fact, if feels like it could be combined with the gtag() function instead of always calling it explicitly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aight. The combining idea sounds nice. I'll check it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have combined the logging with the gtag function itself in the latest implementation. This WindowWithGATracking is now only defined and used in ga.tsx

const calls = ReactGA.testModeAPI.calls;
for (let i = 0; i < calls.length; i++) {
// eslint-disable-next-line no-console
console.log('[react-ga]', ...calls[i]);
}
calls.length = 0;
// eslint-disable-next-line no-console
window.dataLayer?.forEach(callArgs => console.log('[GA Tracking]', ...callArgs));
window.dataLayer = [];
};
10 changes: 10 additions & 0 deletions packages/jaeger-ui/src/vite-env.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,13 @@

// eslint-disable-next-line spaced-comment
/// <reference types="vite/client" />

declare namespace global {
// eslint-disable-next-line @typescript-eslint/naming-convention
interface Window {
// dataLayer property is used by Google Tag Manager
// It should be only be changed with the help of the custom Interface WindowWithGATracking
// defined in utils/tracking/ga.ts
dataLayer: never;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Could you elaborate why this change is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On second thoughts, the most ideal implementation would be for dataLayer to have the appropriate types within ga.ts and have the type never outside it so that no other module tries to access or modify the same.

This had been my initial reasoning but I think it would be better to remove it considering the way you have advised to handle the types in the previous comment.

5 changes: 0 additions & 5 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8470,11 +8470,6 @@ react-fast-compare@^3.1.1:
resolved "https://registry.yarnpkg.com/react-fast-compare/-/react-fast-compare-3.2.0.tgz#641a9da81b6a6320f270e89724fb45a0b39e43bb"
integrity sha512-rtGImPZ0YyLrscKI9xTpV8psd6I8VAtjKCzQDlzyDvqJA8XOW78TXYQwNRNd8g8JZnDu8q9Fu/1v4HPAVwVdHA==

react-ga@^3.3.1:
version "3.3.1"
resolved "https://registry.yarnpkg.com/react-ga/-/react-ga-3.3.1.tgz#d8e1f4e05ec55ed6ff944dcb14b99011dfaf9504"
integrity sha512-4Vc0W5EvXAXUN/wWyxvsAKDLLgtJ3oLmhYYssx+YzphJpejtOst6cbIHCIyF50Fdxuf5DDKqRYny24yJ2y7GFQ==

react-helmet@^6.1.0:
version "6.1.0"
resolved "https://registry.yarnpkg.com/react-helmet/-/react-helmet-6.1.0.tgz#a750d5165cb13cf213e44747502652e794468726"
Expand Down
Loading