Skip to content

Commit

Permalink
Merge pull request #833 from NordicSemiconductor/enhance/telemetry
Browse files Browse the repository at this point in the history
Refactor telemetry code
  • Loading branch information
datenreisender authored Jan 23, 2024
2 parents 7872558 + 35e2d56 commit 2e604e3
Show file tree
Hide file tree
Showing 25 changed files with 494 additions and 478 deletions.
12 changes: 6 additions & 6 deletions nrfutil/sandbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import path from 'path';
import treeKill from 'tree-kill';

import describeError from '../src/logging/describeError';
import telemetry from '../src/telemetry/telemetry';
import { isDevelopment } from '../src/utils/environment';
import usageData from '../src/utils/usageData';
import { versionToInstall } from './moduleVersion';
import { getNrfutilLogger } from './nrfutilLogger';
import {
Expand Down Expand Up @@ -313,7 +313,7 @@ export class NrfutilSandbox {
}

error.message = error.message.replaceAll('Error: ', '');
usageData.sendErrorReport(
telemetry.sendErrorReport(
`${
pid && this.logLevel === 'trace' ? `[PID:${pid}] ` : ''
}${describeError(error)}`
Expand All @@ -332,7 +332,7 @@ export class NrfutilSandbox {
) =>
new Promise<void>((resolve, reject) => {
let aborting = false;
usageData.sendUsageData(`running nrfutil ${this.module}`, {
telemetry.sendUsageData(`running nrfutil ${this.module}`, {
args,
exec: command,
});
Expand Down Expand Up @@ -494,7 +494,7 @@ export class NrfutilSandbox {
}

error.message = error.message.replaceAll('Error: ', '');
usageData.sendErrorReport(
telemetry.sendErrorReport(
`${
pid && this.logLevel === 'trace' ? `[PID:${pid}] ` : ''
}${describeError(error)}`
Expand All @@ -513,7 +513,7 @@ export class NrfutilSandbox {
) =>
new Promise<void>((resolve, reject) => {
let aborting = false;
usageData.sendUsageData(`running nrfutil ${this.module}`, {
telemetry.sendUsageData(`running nrfutil ${this.module}`, {
args,
exec: command,
});
Expand Down Expand Up @@ -611,7 +611,7 @@ export class NrfutilSandbox {
closedHandlers.forEach(callback => callback());
})
.catch(error => {
usageData.sendErrorReport(describeError(error));
telemetry.sendErrorReport(describeError(error));
running = false;
closedHandlers.forEach(callback => callback(error));
});
Expand Down
13 changes: 11 additions & 2 deletions src/App/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
*/

import React from 'react';
import { screen } from '@testing-library/react';
import { act, screen } from '@testing-library/react';

import { OFFICIAL } from '../../ipc/sources';
import packageJsonFromShared from '../../package.json';
import render from '../../test/testrenderer';
import App, { Pane } from './App';
Expand All @@ -26,6 +27,12 @@ jest.mock('../utils/packageJson', () => ({
packageJson: () => packageJsonFromShared,
}));

const appDetails = Promise.resolve({ source: OFFICIAL });
jest.mock('../utils/appDetails', () => ({
__esModule: true,
default: () => appDetails,
}));

const renderApp = (panes: Pane[]) => {
const dummyReducer = (s = null) => s;
const dummyNode = <div />;
Expand All @@ -50,9 +57,11 @@ const anotherPane = {
};

describe('App', () => {
it('automatically gets an About pane attached', () => {
it('automatically gets an About pane attached', async () => {
renderApp([aPane, anotherPane]);

expect(screen.getByText('About')).toBeInTheDocument();

await act(() => appDetails);
});
});
6 changes: 3 additions & 3 deletions src/App/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ import FlashMessages from '../FlashMessage/FlashMessage';
import LogViewer from '../Log/LogViewer';
import logger from '../logging';
import NavBar from '../NavBar/NavBar';
import telemetry from '../telemetry/telemetry';
import classNames from '../utils/classNames';
import {
getPersistedCurrentPane,
getPersistedLogVisible,
} from '../utils/persistentStore';
import usageData from '../utils/usageData';
import useHotKey from '../utils/useHotKey';
import {
currentPane as currentPaneSelector,
Expand Down Expand Up @@ -87,7 +87,7 @@ const ConnectedApp: FC<ConnectedAppProps> = ({
if (!initApp.current) {
logger.initialise();
setNrfutilLogger(logger);
usageData.setLogger(logger);
telemetry.setLogger(logger);
initApp.current = true;
}

Expand All @@ -110,7 +110,7 @@ const ConnectedApp: FC<ConnectedAppProps> = ({
}, [allPanes]);

useEffect(() => {
usageData.sendPageView(paneName.current[currentPane]);
telemetry.sendPageView(paneName.current[currentPane]);
}, [currentPane]);

useEffect(() => {
Expand Down
13 changes: 13 additions & 0 deletions src/Device/DeviceSelector/DeviceSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,26 @@
import React from 'react';
import { act, fireEvent, screen, waitFor } from '@testing-library/react';

import { OFFICIAL } from '../../../ipc/sources';
import packageJsonFromShared from '../../../package.json';
import render from '../../../test/testrenderer';
import { addDevice, Device, removeDevice } from '../deviceSlice';
import { jprogDeviceSetup } from '../jprogOperations';
import DeviceSelector from './DeviceSelector';

jest.mock('../../../nrfutil/device/device');

jest.mock('../../utils/packageJson', () => ({
isLauncher: () => false,
packageJson: () => packageJsonFromShared,
}));

const appDetails = Promise.resolve({ source: OFFICIAL });
jest.mock('../../utils/appDetails', () => ({
__esModule: true,
default: () => appDetails,
}));

const DEVICE_SERIAL_NUMBER = '000000001';

const testDevice: Device = {
Expand Down
12 changes: 6 additions & 6 deletions src/Device/DeviceSelector/DeviceSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import { useDispatch, useSelector } from 'react-redux';
import { NrfutilDeviceLib } from '../../../nrfutil/device';
import { DeviceTraits } from '../../../nrfutil/device/common';
import logger from '../../logging';
import usageData from '../../utils/usageData';
import { simplifyDeviceForLogging } from '../../utils/usageDataCommon';
import simplifyDevice from '../../telemetry/simplifyDevice';
import telemetry from '../../telemetry/telemetry';
import useHotKey from '../../utils/useHotKey';
import {
clearWaitForDevice,
Expand Down Expand Up @@ -71,9 +71,9 @@ export default ({
const doDeselectDevice = useCallback(
(device?: Device) => {
if (device) {
usageData.sendUsageData(
telemetry.sendUsageData(
'device deselected ',
simplifyDeviceForLogging(device)
simplifyDevice(device)
);
}

Expand Down Expand Up @@ -120,8 +120,8 @@ export default ({
dispatch(setSelectedDeviceInfo(deviceInfo));
onDeviceSelected(device, autoReselected);

usageData.sendUsageData('device selected', {
device: simplifyDeviceForLogging(device),
telemetry.sendUsageData('device selected', {
device: simplifyDevice(device),
deviceInfo,
});

Expand Down
8 changes: 4 additions & 4 deletions src/Device/deviceLister.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import { DeviceTraits, NrfutilDevice } from '../../nrfutil/device/common';
import NrfutilDeviceLib from '../../nrfutil/device/device';
import logger from '../logging';
import type { AppThunk, RootState } from '../store';
import usageData from '../utils/usageData';
import { simplifyDeviceForLogging } from '../utils/usageDataCommon';
import simplifyDevice from '../telemetry/simplifyDevice';
import telemetry from '../telemetry/telemetry';
import {
clearWaitForDevice,
clearWaitForDeviceTimeout,
Expand Down Expand Up @@ -180,9 +180,9 @@ export const startWatchingDevices =

const action = async (device: Device) => {
if (hasValidDeviceTraits(device.traits, deviceListing)) {
usageData.sendUsageData(
telemetry.sendUsageData(
'device connected',
simplifyDeviceForLogging(device)
simplifyDevice(device)
);
if (
!getState().device.devices.find(
Expand Down
4 changes: 2 additions & 2 deletions src/ErrorBoundary/ErrorBoundary.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import { generateSystemReport } from '../utils/systemReport';
import ErrorBoundary from './ErrorBoundary';

jest.mock('../utils/systemReport');
jest.mock('../utils/usageData', () => ({
...jest.requireActual('../utils/usageData'),
jest.mock('../telemetry/telemetry', () => ({
...jest.requireActual('../telemetry/telemetry'),
sendErrorReport: jest.fn(),
isEnabled: () => true,
}));
Expand Down
13 changes: 5 additions & 8 deletions src/ErrorBoundary/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,17 @@ import { Device } from '../Device/deviceSlice';
import FactoryResetButton from '../FactoryReset/FactoryResetButton';
import { CollapsibleGroup } from '../SidePanel/Group';
import Spinner from '../Spinner/Spinner';
import telemetry from '../telemetry/telemetry';
import { openUrl } from '../utils/open';
import { packageJson } from '../utils/packageJson';
import { getAppSpecificStore as store } from '../utils/persistentStore';
import { generateSystemReport } from '../utils/systemReport';
import usageData from '../utils/usageData';
import bugIcon from './bug.svg';

import './error-boundary.scss';

const sendGAEvent = (error: string) => {
if (!usageData.isEnabled()) {
return;
}

usageData.sendErrorReport(error);
const sendErrorReport = (error: string) => {
telemetry.sendErrorReport(error);
};

interface Props {
Expand Down Expand Up @@ -67,9 +63,10 @@ class ErrorBoundary extends React.Component<
componentDidCatch(error: Error) {
const { devices, selectedDevice, selectedSerialNumber, sendUsageData } =
this.props;

sendUsageData != null
? sendUsageData(error.message)
: sendGAEvent(error.message);
: sendErrorReport(error.message);

generateSystemReport(
new Date().toISOString().replace(/:/g, '-'),
Expand Down
6 changes: 3 additions & 3 deletions src/Parsers/shellParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ export type XTerminalShellParser = ReturnType<

export const xTerminalShellParserWrapper = (terminal: Terminal) => ({
getTerminalData: () => {
let out = '';
let result = '';
for (let i = 0; i <= terminal.buffer.active.cursorY; i += 1) {
const line = terminal.buffer.active.getLine(i);
if (typeof line !== 'undefined') {
out += `\r\n${line.translateToString()}`;
result += `\r\n${line.translateToString()}`;
}
}

return out.trim();
return result.trim();
},
clear: () => terminal.clear(),
getLastLine: () => {
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ export {
export { openUrl, openFile } from './utils/open';
export { default as systemReport } from './utils/systemReport';

export { default as usageData } from './utils/usageData';
export { type Metadata as UsageDataMetadata } from './utils/usageDataCommon';
export { default as usageData } from './telemetry/telemetry';
export { type default as UsageDataMetadata } from './telemetry/TelemetryMetadata';
export { default as classNames } from './utils/classNames';
export { truncateMiddle } from './utils/truncateMiddle';

Expand Down
9 changes: 9 additions & 0 deletions src/telemetry/TelemetryMetadata.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright (c) 2023 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: LicenseRef-Nordic-4-Clause
*/

export default interface TelemetryMetadata {
[key: string]: unknown;
}
82 changes: 82 additions & 0 deletions src/telemetry/TelemetrySender.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Copyright (c) 2023 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: LicenseRef-Nordic-4-Clause
*/

import si from 'systeminformation';
import { Logger } from 'winston';

import {
deleteHasUserAgreedToTelemetry,
getHasUserAgreedToTelemetry,
persistHasUserAgreedToTelemetry,
} from '../utils/persistentStore';
import TelemetryMetadata from './TelemetryMetadata';

type MaybePromise<T> = T | Promise<T>;

export default abstract class TelemetrySender {
readonly INSTRUMENTATION_KEY = '4b8b1a39-37c7-479e-a684-d4763c7c647c';

logger?: Logger;
setLogger = (logger: Logger) => {
this.logger = logger;
};

isTelemetryAllowedForCurrentApp = false;
allowTelemetryForCurrentApp = () => {
this.isTelemetryAllowedForCurrentApp = true;
};

getIsSendingTelemetry = () =>
this.isTelemetryAllowedForCurrentApp &&
getHasUserAgreedToTelemetry() === true;

/**
* @deprecated Use `getIsSendingTelemetry` instead
* @returns {boolean} If telemetry is enabled
*/
isEnabled() {
const isSendingTelemetry = this.getIsSendingTelemetry();
this.logger?.debug(`Telemetry is ${isSendingTelemetry}`);

return isSendingTelemetry;
}

async enable() {
persistHasUserAgreedToTelemetry(true);
this.sendUsageData('Telemetry Opt-In');

const { platform, arch } = await si.osInfo();
this.sendUsageData('Report OS info', { platform, arch });

this.logger?.debug('Usage data has been enabled');
}

disable() {
persistHasUserAgreedToTelemetry(false);
this.sendMinimalUsageData('Telemetry Opt-Out');
this.logger?.debug('Usage data has been disabled');
}

reset() {
deleteHasUserAgreedToTelemetry();
this.sendMinimalUsageData('Telemetry Opt-Reset');
this.logger?.debug('Usage data setting has been reset');
}

sendMinimalUsageData(action: string) {
this.sendUsageData(action, { removeAllMetadata: true });
}

abstract sendUsageData(
action: string,
metadata?: TelemetryMetadata
): MaybePromise<void>;
abstract sendPageView(pageName: string): MaybePromise<void>;
abstract sendMetric(name: string, average: number): MaybePromise<void>;
abstract sendTrace(message: string): MaybePromise<void>;
abstract sendErrorReport(error: Error): MaybePromise<void>;
abstract flush(): MaybePromise<void>;
}
Loading

0 comments on commit 2e604e3

Please sign in to comment.