[Reporting/Telemetry] Do not send telemetry if we are in screenshot mode (#100388) (#101076)

* 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
This commit is contained in:
Jean-Louis Leysens 2021-06-01 21:27:05 +02:00 committed by GitHub
parent 313a887d34
commit 09129c8e38
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
22 changed files with 247 additions and 24 deletions

View file

@ -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';

View file

@ -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';

View file

@ -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,

View file

@ -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"]
}

View file

@ -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 = {
@ -47,6 +49,7 @@ export function mockTelemetryService({
config,
http: httpServiceMock.createStartContract(),
notifications: notificationServiceMock.createStartContract(),
isScreenshotMode,
currentKibanaVersion,
reportOptInStatusChange,
});

View file

@ -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,
@ -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 **/
@ -76,6 +80,10 @@ export interface TelemetryPluginStart {
};
}
interface TelemetryPluginSetupDependencies {
screenshotMode: ScreenshotModePluginSetup;
}
/**
* Public-exposed configuration
*/
@ -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,
@ -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(),

View file

@ -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']>;
@ -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'];

View file

@ -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;
}

View file

@ -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);
});
});
});

View file

@ -14,6 +14,7 @@ interface TelemetryServiceConstructor {
config: TelemetryPluginConfig;
http: CoreStart['http'];
notifications: CoreStart['notifications'];
isScreenshotMode: boolean;
currentKibanaVersion: string;
reportOptInStatusChange?: boolean;
}
@ -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 */
@ -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;
@ -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 **/
@ -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 });

View file

@ -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" }
]
}

View file

@ -306,6 +306,7 @@ exports[`TelemetryManagementSectionComponent renders null because allowChangingO
showAppliesSettingMessage={true}
telemetryService={
TelemetryService {
"canSendTelemetry": [Function],
"currentKibanaVersion": "mock_kibana_version",
"defaultConfig": Object {
"allowChangingOptInStatus": false,
@ -350,6 +351,7 @@ exports[`TelemetryManagementSectionComponent renders null because allowChangingO
"post": [MockFunction],
"put": [MockFunction],
},
"isScreenshotMode": false,
"notifications": Object {
"toasts": Object {
"add": [MockFunction],

View file

@ -32,6 +32,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
currentKibanaVersion: 'mock_kibana_version',
notifications: coreStart.notifications,
@ -66,6 +67,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
@ -121,6 +123,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
@ -169,6 +172,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
@ -208,6 +212,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
@ -248,6 +253,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
@ -288,6 +294,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
currentKibanaVersion: 'mock_kibana_version',
notifications: coreStart.notifications,
@ -328,6 +335,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',
@ -378,6 +386,7 @@ describe('TelemetryManagementSectionComponent', () => {
optInStatusUrl: '',
sendUsageFrom: 'browser',
},
isScreenshotMode: false,
reportOptInStatusChange: false,
notifications: coreStart.notifications,
currentKibanaVersion: 'mock_kibana_version',

View file

@ -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'),

View file

@ -0,0 +1,9 @@
{
"id": "telemetryTestPlugin",
"version": "0.0.1",
"kibanaVersion": "kibana",
"configPath": ["telemetryTestPlugin"],
"requiredPlugins": ["telemetry"],
"server": false,
"ui": true
}

View file

@ -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"
}
}

View file

@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { TelemetryTestPlugin } from './plugin';
export const plugin = () => new TelemetryTestPlugin();

View file

@ -0,0 +1,28 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { CoreSetup, Plugin } from 'src/core/public';
import type { TelemetryPluginSetup } from '../../../../../src/plugins/telemetry/public';
interface TelemetryTestPluginSetupDependencies {
telemetry: TelemetryPluginSetup;
}
export class TelemetryTestPlugin implements Plugin {
setup(core: CoreSetup, { telemetry }: TelemetryTestPluginSetupDependencies) {
window._checkCanSendTelemetry = async () => {
await telemetry.telemetryService.setOptIn(true);
return telemetry.telemetryService.canSendTelemetry();
};
window._resetTelemetry = async () => {
await telemetry.telemetryService.setOptIn(false);
};
}
start() {}
}

View file

@ -0,0 +1,10 @@
{
"extends": "../../../../tsconfig.base.json",
"compilerOptions": {
"outDir": "./target",
"skipLibCheck": true
},
"include": ["public/**/*.ts", "types.ts", "../../../../typings/**/*"],
"exclude": [],
"references": [{ "path": "../../../../src/core/tsconfig.json" }]
}

View file

@ -0,0 +1,17 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
declare global {
interface Window {
_checkCanSendTelemetry: () => Promise<boolean>;
_resetTelemetry: () => Promise<void>;
}
}
// Turn this file into a module
export {};

View file

@ -0,0 +1,15 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { PluginFunctionalProviderContext } from '../../services';
export default function ({ loadTestFile }: PluginFunctionalProviderContext) {
describe('telemetry', function () {
loadTestFile(require.resolve('./telemetry'));
});
}

View file

@ -0,0 +1,45 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import expect from '@kbn/expect';
import { PluginFunctionalProviderContext } from '../../services';
import { KBN_SCREENSHOT_MODE_ENABLED_KEY } from '../../../../src/plugins/screenshot_mode/public';
export default function ({ getService, getPageObjects }: PluginFunctionalProviderContext) {
const browser = getService('browser');
const PageObjects = getPageObjects(['common']);
describe('Telemetry service', () => {
const checkCanSendTelemetry = (): Promise<boolean> => {
return browser.executeAsync<boolean>((cb) => {
((window as unknown) as Record<string, () => Promise<boolean>>)
._checkCanSendTelemetry()
.then(cb);
});
};
after(async () => {
await browser.removeLocalStorageItem(KBN_SCREENSHOT_MODE_ENABLED_KEY);
await browser.executeAsync<void>((cb) => {
((window as unknown) as Record<string, () => Promise<boolean>>)
._resetTelemetry()
.then(() => cb());
});
});
it('detects that telemetry cannot be sent in screenshot mode', async () => {
await PageObjects.common.navigateToApp('home');
expect(await checkCanSendTelemetry()).to.be(true);
await browser.setLocalStorageItem(KBN_SCREENSHOT_MODE_ENABLED_KEY, 'true');
await PageObjects.common.navigateToApp('home');
expect(await checkCanSendTelemetry()).to.be(false);
});
});
}