Skip to content

Commit

Permalink
[Reporting/Telemetry] Do not send telemetry if we are in screenshot m…
Browse files Browse the repository at this point in the history
…ode (elastic#100388)

* do not send telemetry if isScreenshotMode

* Implement PR feedback:

* added another Jest test
* move Boolean() to make the opt-in value always boolean

* remove unused import and convert to import type

* fix type issues

* update jest snapshot

* Expanded test coverage

- added plugin functional test
- added jest test to check TelemetrySender behaviour
- exported the localStorage/window value that flags screenshot
  mode

* fix test plugin name in package.json and make sure to opt out of telemetry when the test finishes

* added missing type file to plugin_functional test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
#	src/plugins/telemetry/kibana.json
  • Loading branch information
jloleysens committed Jun 1, 2021
1 parent ff6a5a8 commit 687f2a9
Show file tree
Hide file tree
Showing 22 changed files with 247 additions and 24 deletions.
1 change: 1 addition & 0 deletions src/plugins/screenshot_mode/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export {
getScreenshotMode,
setScreenshotModeEnabled,
setScreenshotModeDisabled,
KBN_SCREENSHOT_MODE_ENABLED_KEY,
} from './get_set_browser_screenshot_mode';

export { KBN_SCREENSHOT_MODE_HEADER } from './constants';
6 changes: 5 additions & 1 deletion src/plugins/screenshot_mode/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ export function plugin() {
return new ScreenshotModePlugin();
}

export { KBN_SCREENSHOT_MODE_HEADER, setScreenshotModeEnabled } from '../common';
export {
KBN_SCREENSHOT_MODE_HEADER,
setScreenshotModeEnabled,
KBN_SCREENSHOT_MODE_ENABLED_KEY,
} from '../common';

export { ScreenshotModePluginSetup } from './types';
6 changes: 5 additions & 1 deletion src/plugins/screenshot_mode/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@

import { ScreenshotModePlugin } from './plugin';

export { setScreenshotModeEnabled, KBN_SCREENSHOT_MODE_HEADER } from '../common';
export {
setScreenshotModeEnabled,
KBN_SCREENSHOT_MODE_HEADER,
KBN_SCREENSHOT_MODE_ENABLED_KEY,
} from '../common';

export {
ScreenshotModeRequestHandlerContext,
Expand Down
10 changes: 2 additions & 8 deletions src/plugins/telemetry/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,7 @@
"version": "kibana",
"server": true,
"ui": true,
"requiredPlugins": [
"telemetryCollectionManager",
"usageCollection"
],
"requiredPlugins": ["telemetryCollectionManager", "screenshotMode", "usageCollection"],
"extraPublicDirs": ["common/constants"],
"requiredBundles": [
"kibanaUtils",
"kibanaReact"
]
"requiredBundles": ["kibanaUtils", "kibanaReact"]
}
3 changes: 3 additions & 0 deletions src/plugins/telemetry/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ import { TelemetryPluginStart, TelemetryPluginSetup, TelemetryPluginConfig } fro
export interface TelemetryServiceMockOptions {
reportOptInStatusChange?: boolean;
currentKibanaVersion?: string;
isScreenshotMode?: boolean;
config?: Partial<TelemetryPluginConfig>;
}

export function mockTelemetryService({
reportOptInStatusChange,
currentKibanaVersion = 'mockKibanaVersion',
isScreenshotMode = false,
config: configOverride = {},
}: TelemetryServiceMockOptions = {}) {
const config = {
Expand All @@ -47,6 +49,7 @@ export function mockTelemetryService({
config,
http: httpServiceMock.createStartContract(),
notifications: notificationServiceMock.createStartContract(),
isScreenshotMode,
currentKibanaVersion,
reportOptInStatusChange,
});
Expand Down
15 changes: 14 additions & 1 deletion src/plugins/telemetry/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import type {
ApplicationStart,
} from 'src/core/public';

import type { ScreenshotModePluginSetup } from 'src/plugins/screenshot_mode/public';

import { TelemetrySender, TelemetryService, TelemetryNotifications } from './services';
import type {
TelemetrySavedObjectAttributes,
Expand All @@ -38,6 +40,8 @@ export interface TelemetryServicePublicApis {
getIsOptedIn: () => boolean | null;
/** Is the user allowed to change the opt-in/out status? **/
userCanChangeSettings: boolean;
/** Can phone-home telemetry calls be made? This depends on whether we have opted-in or if we are rendering a report */
canSendTelemetry: () => boolean;
/** Is the cluster allowed to change the opt-in/out status? **/
getCanChangeOptInStatus: () => boolean;
/** Fetches an unencrypted telemetry payload so we can show it to the user **/
Expand Down Expand Up @@ -76,6 +80,10 @@ export interface TelemetryPluginStart {
};
}

interface TelemetryPluginSetupDependencies {
screenshotMode: ScreenshotModePluginSetup;
}

/**
* Public-exposed configuration
*/
Expand Down Expand Up @@ -113,11 +121,15 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
this.config = initializerContext.config.get();
}

public setup({ http, notifications }: CoreSetup): TelemetryPluginSetup {
public setup(
{ http, notifications }: CoreSetup,
{ screenshotMode }: TelemetryPluginSetupDependencies
): TelemetryPluginSetup {
const config = this.config;
const currentKibanaVersion = this.currentKibanaVersion;
this.telemetryService = new TelemetryService({
config,
isScreenshotMode: screenshotMode.isScreenshotMode(),
http,
notifications,
currentKibanaVersion,
Expand Down Expand Up @@ -181,6 +193,7 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
return {
getIsOptedIn: () => telemetryService.getIsOptedIn(),
setOptIn: (optedIn) => telemetryService.setOptIn(optedIn),
canSendTelemetry: () => telemetryService.canSendTelemetry(),
userCanChangeSettings: telemetryService.userCanChangeSettings,
getCanChangeOptInStatus: () => telemetryService.getCanChangeOptInStatus(),
fetchExample: () => telemetryService.fetchExample(),
Expand Down
19 changes: 19 additions & 0 deletions src/plugins/telemetry/public/services/telemetry_sender.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,16 @@ describe('TelemetrySender', () => {
expect(shouldSendReport).toBe(true);
});

it('returns false if we are in screenshot mode', () => {
const telemetryService = mockTelemetryService({ isScreenshotMode: true });
telemetryService.getIsOptedIn = jest.fn().mockReturnValue(false);
const telemetrySender = new TelemetrySender(telemetryService);
const shouldSendReport = telemetrySender['shouldSendReport']();

expect(telemetryService.getIsOptedIn).toBeCalledTimes(0);
expect(shouldSendReport).toBe(false);
});

describe('sendIfDue', () => {
let originalFetch: typeof window['fetch'];
let mockFetch: jest.Mock<typeof window['fetch']>;
Expand Down Expand Up @@ -151,6 +161,15 @@ describe('TelemetrySender', () => {
expect(mockFetch).toBeCalledTimes(0);
});

it('does not send if we are in screenshot mode', async () => {
const telemetryService = mockTelemetryService({ isScreenshotMode: true });
const telemetrySender = new TelemetrySender(telemetryService);
telemetrySender['isSending'] = false;
await telemetrySender['sendIfDue']();

expect(mockFetch).toBeCalledTimes(0);
});

it('sends report if due', async () => {
const mockTelemetryUrl = 'telemetry_cluster_url';
const mockTelemetryPayload = ['hashed_cluster_usage_data1'];
Expand Down
3 changes: 1 addition & 2 deletions src/plugins/telemetry/public/services/telemetry_sender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ export class TelemetrySender {
};

private shouldSendReport = (): boolean => {
// check if opt-in for telemetry is enabled
if (this.telemetryService.getIsOptedIn()) {
if (this.telemetryService.canSendTelemetry()) {
if (!this.lastReported) {
return true;
}
Expand Down
20 changes: 20 additions & 0 deletions src/plugins/telemetry/public/services/telemetry_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,4 +256,24 @@ describe('TelemetryService', () => {
expect(mockFetch).toHaveBeenCalledTimes(1);
});
});

describe('canSendTelemetry', () => {
it('does not send telemetry if screenshotMode is true', () => {
const telemetryService = mockTelemetryService({
isScreenshotMode: true,
config: { optIn: true },
});

expect(telemetryService.canSendTelemetry()).toBe(false);
});

it('does send telemetry if screenshotMode is false and we are opted in', () => {
const telemetryService = mockTelemetryService({
isScreenshotMode: false,
config: { optIn: true },
});

expect(telemetryService.canSendTelemetry()).toBe(true);
});
});
});
13 changes: 11 additions & 2 deletions src/plugins/telemetry/public/services/telemetry_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ interface TelemetryServiceConstructor {
config: TelemetryPluginConfig;
http: CoreStart['http'];
notifications: CoreStart['notifications'];
isScreenshotMode: boolean;
currentKibanaVersion: string;
reportOptInStatusChange?: boolean;
}
Expand All @@ -27,6 +28,7 @@ export class TelemetryService {
private readonly reportOptInStatusChange: boolean;
private readonly notifications: CoreStart['notifications'];
private readonly defaultConfig: TelemetryPluginConfig;
private readonly isScreenshotMode: boolean;
private updatedConfig?: TelemetryPluginConfig;

/** Current version of Kibana */
Expand All @@ -35,11 +37,13 @@ export class TelemetryService {
constructor({
config,
http,
isScreenshotMode,
notifications,
currentKibanaVersion,
reportOptInStatusChange = true,
}: TelemetryServiceConstructor) {
this.defaultConfig = config;
this.isScreenshotMode = isScreenshotMode;
this.reportOptInStatusChange = reportOptInStatusChange;
this.notifications = notifications;
this.currentKibanaVersion = currentKibanaVersion;
Expand All @@ -63,7 +67,7 @@ export class TelemetryService {

/** Is the cluster opted-in to telemetry **/
public get isOptedIn() {
return this.config.optIn;
return Boolean(this.config.optIn);
}

/** Changes the opt-in status **/
Expand Down Expand Up @@ -122,10 +126,15 @@ export class TelemetryService {
}

/** Is the cluster opted-in to telemetry **/
public getIsOptedIn = () => {
public getIsOptedIn = (): boolean => {
return this.isOptedIn;
};

/** Are there any blockers for sending telemetry */
public canSendTelemetry = (): boolean => {
return !this.isScreenshotMode && this.getIsOptedIn();
};

/** Fetches an unencrypted telemetry payload so we can show it to the user **/
public fetchExample = async () => {
return await this.fetchTelemetry({ unencrypted: true });
Expand Down
14 changes: 5 additions & 9 deletions src/plugins/telemetry/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,13 @@
"declarationMap": true,
"isolatedModules": true
},
"include": [
"public/**/**/*",
"server/**/**/*",
"common/**/*",
"../../../typings/**/*"
],
"include": ["public/**/**/*", "server/**/**/*", "common/**/*", "../../../typings/**/*"],
"references": [
{ "path": "../../core/tsconfig.json" },
{ "path": "../../plugins/usage_collection/tsconfig.json" },
{ "path": "../../plugins/telemetry_collection_manager/tsconfig.json" },
{ "path": "../../plugins/kibana_react/tsconfig.json" },
{ "path": "../../plugins/kibana_utils/tsconfig.json" },
{ "path": "../../plugins/kibana_react/tsconfig.json" }
{ "path": "../../plugins/screenshot_mode/tsconfig.json" },
{ "path": "../../plugins/telemetry_collection_manager/tsconfig.json" },
{ "path": "../../plugins/usage_collection/tsconfig.json" }
]
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
currentKibanaVersion: 'mock_kibana_version',
notifications: coreStart.notifications,
Expand Down Expand Up @@ -66,6 +67,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
Expand Down Expand Up @@ -121,6 +123,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
Expand Down Expand Up @@ -169,6 +172,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
Expand Down Expand Up @@ -208,6 +212,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
Expand Down Expand Up @@ -248,6 +253,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
Expand Down Expand Up @@ -288,6 +294,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
currentKibanaVersion: 'mock_kibana_version',
notifications: coreStart.notifications,
Expand Down Expand Up @@ -328,6 +335,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
Expand Down Expand Up @@ -378,6 +386,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
Expand Down
1 change: 1 addition & 0 deletions test/plugin_functional/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
return {
testFiles: [
require.resolve('./test_suites/usage_collection'),
require.resolve('./test_suites/telemetry'),
require.resolve('./test_suites/core'),
require.resolve('./test_suites/custom_visualizations'),
require.resolve('./test_suites/panel_actions'),
Expand Down
9 changes: 9 additions & 0 deletions test/plugin_functional/plugins/telemetry/kibana.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"id": "telemetryTestPlugin",
"version": "0.0.1",
"kibanaVersion": "kibana",
"configPath": ["telemetryTestPlugin"],
"requiredPlugins": ["telemetry"],
"server": false,
"ui": true
}
14 changes: 14 additions & 0 deletions test/plugin_functional/plugins/telemetry/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "telemetry_test_plugin",
"version": "1.0.0",
"main": "target/test/plugin_functional/plugins/telemetry",
"kibana": {
"version": "kibana",
"templateVersion": "1.0.0"
},
"license": "SSPL-1.0 OR Elastic License 2.0",
"scripts": {
"kbn": "node ../../../../scripts/kbn.js",
"build": "rm -rf './target' && ../../../../node_modules/.bin/tsc"
}
}
Loading

0 comments on commit 687f2a9

Please sign in to comment.