From 5ba237a8856716bd1d88f9b4fca224916e378e9a Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Mon, 4 Nov 2019 23:28:54 -0500 Subject: [PATCH] [Telemetry] Remind users about telemetry on each minor version (#49644) resolves #49519 If a user has previously opted out of telemetry, this PR will cause them to be prompted again, when the major or minor version of Kibana changes. Previously, once opted out, they would never get prompted again. --- src/legacy/core_plugins/telemetry/index.ts | 18 +- .../core_plugins/telemetry/mappings.json | 3 + .../server/get_telemetry_opt_in.test.ts | 214 ++++++++++++++++++ .../telemetry/server/get_telemetry_opt_in.ts | 66 +++++- .../core_plugins/telemetry/server/index.ts | 2 +- .../core_plugins/telemetry/server/plugin.ts | 11 +- .../telemetry/server/routes/index.ts | 9 +- .../telemetry/server/routes/opt_in.ts | 31 ++- .../api_integration/apis/telemetry/index.js | 1 + .../api_integration/apis/telemetry/opt_in.ts | 63 ++++++ 10 files changed, 397 insertions(+), 21 deletions(-) create mode 100644 src/legacy/core_plugins/telemetry/server/get_telemetry_opt_in.test.ts create mode 100644 x-pack/test/api_integration/apis/telemetry/opt_in.ts diff --git a/src/legacy/core_plugins/telemetry/index.ts b/src/legacy/core_plugins/telemetry/index.ts index 3271373449eb..4b6566415f3e 100644 --- a/src/legacy/core_plugins/telemetry/index.ts +++ b/src/legacy/core_plugins/telemetry/index.ts @@ -48,6 +48,9 @@ const telemetry = (kibana: any) => { // `config` is used internally and not intended to be set config: Joi.string().default(Joi.ref('$defaultConfigPath')), banner: Joi.boolean().default(true), + lastVersionChecked: Joi.string() + .allow('') + .default(''), url: Joi.when('$dev', { is: true, then: Joi.string().default( @@ -77,7 +80,8 @@ const telemetry = (kibana: any) => { }, }, async replaceInjectedVars(originalInjectedVars: any, request: any) { - const telemetryOptedIn = await getTelemetryOptIn(request); + const currentKibanaVersion = getCurrentKibanaVersion(request.server); + const telemetryOptedIn = await getTelemetryOptIn({ request, currentKibanaVersion }); return { ...originalInjectedVars, @@ -97,7 +101,13 @@ const telemetry = (kibana: any) => { mappings, }, init(server: Server) { - const initializerContext = {} as PluginInitializerContext; + const initializerContext = { + env: { + packageInfo: { + version: getCurrentKibanaVersion(server), + }, + }, + } as PluginInitializerContext; const coreSetup = ({ http: { server }, @@ -116,3 +126,7 @@ const telemetry = (kibana: any) => { // eslint-disable-next-line import/no-default-export export default telemetry; + +function getCurrentKibanaVersion(server: Server): string { + return server.config().get('pkg.version'); +} diff --git a/src/legacy/core_plugins/telemetry/mappings.json b/src/legacy/core_plugins/telemetry/mappings.json index d83f7f596763..1245ef88f589 100644 --- a/src/legacy/core_plugins/telemetry/mappings.json +++ b/src/legacy/core_plugins/telemetry/mappings.json @@ -3,6 +3,9 @@ "properties": { "enabled": { "type": "boolean" + }, + "lastVersionChecked": { + "type": "keyword" } } } diff --git a/src/legacy/core_plugins/telemetry/server/get_telemetry_opt_in.test.ts b/src/legacy/core_plugins/telemetry/server/get_telemetry_opt_in.test.ts new file mode 100644 index 000000000000..67ad3aaae427 --- /dev/null +++ b/src/legacy/core_plugins/telemetry/server/get_telemetry_opt_in.test.ts @@ -0,0 +1,214 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { getTelemetryOptIn } from './get_telemetry_opt_in'; + +describe('get_telemetry_opt_in', () => { + it('returns false when request path is not /app*', async () => { + const params = getCallGetTelemetryOptInParams({ + requestPath: '/foo/bar', + }); + + const result = await callGetTelemetryOptIn(params); + + expect(result).toBe(false); + }); + + it('returns null when saved object not found', async () => { + const params = getCallGetTelemetryOptInParams({ + savedObjectNotFound: true, + }); + + const result = await callGetTelemetryOptIn(params); + + expect(result).toBe(null); + }); + + it('returns false when saved object forbidden', async () => { + const params = getCallGetTelemetryOptInParams({ + savedObjectForbidden: true, + }); + + const result = await callGetTelemetryOptIn(params); + + expect(result).toBe(false); + }); + + it('throws an error on unexpected saved object error', async () => { + const params = getCallGetTelemetryOptInParams({ + savedObjectOtherError: true, + }); + + let threw = false; + try { + await callGetTelemetryOptIn(params); + } catch (err) { + threw = true; + expect(err.message).toBe(SavedObjectOtherErrorMessage); + } + + expect(threw).toBe(true); + }); + + it('returns null if enabled is null or undefined', async () => { + for (const enabled of [null, undefined]) { + const params = getCallGetTelemetryOptInParams({ + enabled, + }); + + const result = await callGetTelemetryOptIn(params); + + expect(result).toBe(null); + } + }); + + it('returns true when enabled is true', async () => { + const params = getCallGetTelemetryOptInParams({ + enabled: true, + }); + + const result = await callGetTelemetryOptIn(params); + + expect(result).toBe(true); + }); + + // build a table of tests with version checks, with results for enabled false + type VersionCheckTable = Array>; + + const EnabledFalseVersionChecks: VersionCheckTable = [ + { lastVersionChecked: '8.0.0', currentKibanaVersion: '8.0.0', result: false }, + { lastVersionChecked: '8.0.0', currentKibanaVersion: '8.0.1', result: false }, + { lastVersionChecked: '8.0.1', currentKibanaVersion: '8.0.0', result: false }, + { lastVersionChecked: '8.0.0', currentKibanaVersion: '8.1.0', result: null }, + { lastVersionChecked: '8.0.0', currentKibanaVersion: '9.0.0', result: null }, + { lastVersionChecked: '8.0.0', currentKibanaVersion: '7.0.0', result: false }, + { lastVersionChecked: '8.1.0', currentKibanaVersion: '8.0.0', result: false }, + { lastVersionChecked: '8.0.0-X', currentKibanaVersion: '8.0.0', result: false }, + { lastVersionChecked: '8.0.0', currentKibanaVersion: '8.0.0-X', result: false }, + { lastVersionChecked: null, currentKibanaVersion: '8.0.0', result: null }, + { lastVersionChecked: undefined, currentKibanaVersion: '8.0.0', result: null }, + { lastVersionChecked: 5, currentKibanaVersion: '8.0.0', result: null }, + { lastVersionChecked: '8.0.0', currentKibanaVersion: 'beta', result: null }, + { lastVersionChecked: 'beta', currentKibanaVersion: '8.0.0', result: null }, + { lastVersionChecked: 'beta', currentKibanaVersion: 'beta', result: false }, + { lastVersionChecked: 'BETA', currentKibanaVersion: 'beta', result: null }, + ].map(el => ({ ...el, enabled: false })); + + // build a table of tests with version checks, with results for enabled true/null/undefined + const EnabledTrueVersionChecks: VersionCheckTable = EnabledFalseVersionChecks.map(el => ({ + ...el, + enabled: true, + result: true, + })); + + const EnabledNullVersionChecks: VersionCheckTable = EnabledFalseVersionChecks.map(el => ({ + ...el, + enabled: null, + result: null, + })); + + const EnabledUndefinedVersionChecks: VersionCheckTable = EnabledFalseVersionChecks.map(el => ({ + ...el, + enabled: undefined, + result: null, + })); + + const AllVersionChecks = [ + ...EnabledFalseVersionChecks, + ...EnabledTrueVersionChecks, + ...EnabledNullVersionChecks, + ...EnabledUndefinedVersionChecks, + ]; + + test.each(AllVersionChecks)( + 'returns expected result for version check with %j', + async (params: Partial) => { + const result = await callGetTelemetryOptIn({ ...DefaultParams, ...params }); + expect(result).toBe(params.result); + } + ); +}); + +interface CallGetTelemetryOptInParams { + requestPath: string; + savedObjectNotFound: boolean; + savedObjectForbidden: boolean; + savedObjectOtherError: boolean; + enabled: boolean | null | undefined; + lastVersionChecked?: any; // should be a string, but test with non-strings + currentKibanaVersion: string; + result?: boolean | null; +} + +const DefaultParams = { + requestPath: '/app/something', + savedObjectNotFound: false, + savedObjectForbidden: false, + savedObjectOtherError: false, + enabled: true, + lastVersionChecked: '8.0.0', + currentKibanaVersion: '8.0.0', +}; + +function getCallGetTelemetryOptInParams( + overrides: Partial +): CallGetTelemetryOptInParams { + return { ...DefaultParams, ...overrides }; +} + +async function callGetTelemetryOptIn(params: CallGetTelemetryOptInParams): Promise { + const { currentKibanaVersion } = params; + const request = getMockRequest(params); + return await getTelemetryOptIn({ request, currentKibanaVersion }); +} + +function getMockRequest(params: CallGetTelemetryOptInParams): any { + return { + path: params.requestPath, + getSavedObjectsClient() { + return getMockSavedObjectsClient(params); + }, + }; +} + +const SavedObjectNotFoundMessage = 'savedObjectNotFound'; +const SavedObjectForbiddenMessage = 'savedObjectForbidden'; +const SavedObjectOtherErrorMessage = 'savedObjectOtherError'; + +function getMockSavedObjectsClient(params: CallGetTelemetryOptInParams) { + return { + async get(type: string, id: string) { + if (params.savedObjectNotFound) throw new Error(SavedObjectNotFoundMessage); + if (params.savedObjectForbidden) throw new Error(SavedObjectForbiddenMessage); + if (params.savedObjectOtherError) throw new Error(SavedObjectOtherErrorMessage); + + const enabled = params.enabled; + const lastVersionChecked = params.lastVersionChecked; + return { attributes: { enabled, lastVersionChecked } }; + }, + errors: { + isNotFoundError(error: any) { + return error.message === SavedObjectNotFoundMessage; + }, + isForbiddenError(error: any) { + return error.message === SavedObjectForbiddenMessage; + }, + }, + }; +} diff --git a/src/legacy/core_plugins/telemetry/server/get_telemetry_opt_in.ts b/src/legacy/core_plugins/telemetry/server/get_telemetry_opt_in.ts index 9b365d6dd7ae..c8bd4a4b6dfb 100644 --- a/src/legacy/core_plugins/telemetry/server/get_telemetry_opt_in.ts +++ b/src/legacy/core_plugins/telemetry/server/get_telemetry_opt_in.ts @@ -17,7 +17,21 @@ * under the License. */ -export async function getTelemetryOptIn(request: any) { +import semver from 'semver'; + +import { SavedObjectAttributes } from './routes/opt_in'; + +interface GetTelemetryOptIn { + request: any; + currentKibanaVersion: string; +} + +// Returns whether telemetry has been opt'ed into or not. +// Returns null not set, meaning Kibana should prompt in the UI. +export async function getTelemetryOptIn({ + request, + currentKibanaVersion, +}: GetTelemetryOptIn): Promise { const isRequestingApplication = request.path.startsWith('/app'); // Prevent interstitial screens (such as the space selector) from prompting for telemetry @@ -27,9 +41,9 @@ export async function getTelemetryOptIn(request: any) { const savedObjectsClient = request.getSavedObjectsClient(); + let savedObject; try { - const { attributes } = await savedObjectsClient.get('telemetry', 'telemetry'); - return attributes.enabled; + savedObject = await savedObjectsClient.get('telemetry', 'telemetry'); } catch (error) { if (savedObjectsClient.errors.isNotFoundError(error)) { return null; @@ -43,4 +57,50 @@ export async function getTelemetryOptIn(request: any) { throw error; } + + const { attributes }: { attributes: SavedObjectAttributes } = savedObject; + + // if enabled is already null, return null + if (attributes.enabled == null) return null; + + const enabled = !!attributes.enabled; + + // if enabled is true, return it + if (enabled === true) return enabled; + + // Additional check if they've already opted out (enabled: false): + // - if the Kibana version has changed by at least a minor version, + // return null to re-prompt. + + const lastKibanaVersion = attributes.lastVersionChecked; + + // if the last kibana version isn't set, or is somehow not a string, return null + if (typeof lastKibanaVersion !== 'string') return null; + + // if version hasn't changed, just return enabled value + if (lastKibanaVersion === currentKibanaVersion) return enabled; + + const lastSemver = parseSemver(lastKibanaVersion); + const currentSemver = parseSemver(currentKibanaVersion); + + // if either version is invalid, return null + if (lastSemver == null || currentSemver == null) return null; + + // actual major/minor version comparison, for cases when to return null + if (currentSemver.major > lastSemver.major) return null; + if (currentSemver.major === lastSemver.major) { + if (currentSemver.minor > lastSemver.minor) return null; + } + + // current version X.Y is not greater than last version X.Y, return enabled + return enabled; +} + +function parseSemver(version: string): semver.SemVer | null { + // semver functions both return nulls AND throw exceptions: "it depends!" + try { + return semver.parse(version); + } catch (err) { + return null; + } } diff --git a/src/legacy/core_plugins/telemetry/server/index.ts b/src/legacy/core_plugins/telemetry/server/index.ts index b8ae5fc231fb..aa13fab9a5f8 100644 --- a/src/legacy/core_plugins/telemetry/server/index.ts +++ b/src/legacy/core_plugins/telemetry/server/index.ts @@ -25,5 +25,5 @@ export { getTelemetryOptIn } from './get_telemetry_opt_in'; export { telemetryCollectionManager } from './collection_manager'; export const telemetryPlugin = (initializerContext: PluginInitializerContext) => - new TelemetryPlugin(); + new TelemetryPlugin(initializerContext); export { constants }; diff --git a/src/legacy/core_plugins/telemetry/server/plugin.ts b/src/legacy/core_plugins/telemetry/server/plugin.ts index 70de51b2abe9..a5f0f1234799 100644 --- a/src/legacy/core_plugins/telemetry/server/plugin.ts +++ b/src/legacy/core_plugins/telemetry/server/plugin.ts @@ -17,14 +17,21 @@ * under the License. */ -import { CoreSetup } from 'src/core/server'; +import { CoreSetup, PluginInitializerContext } from 'src/core/server'; import { registerRoutes } from './routes'; import { telemetryCollectionManager } from './collection_manager'; import { getStats } from './telemetry_collection'; export class TelemetryPlugin { + private readonly currentKibanaVersion: string; + + constructor(initializerContext: PluginInitializerContext) { + this.currentKibanaVersion = initializerContext.env.packageInfo.version; + } + public setup(core: CoreSetup) { + const currentKibanaVersion = this.currentKibanaVersion; telemetryCollectionManager.setStatsGetter(getStats, 'local'); - registerRoutes(core); + registerRoutes({ core, currentKibanaVersion }); } } diff --git a/src/legacy/core_plugins/telemetry/server/routes/index.ts b/src/legacy/core_plugins/telemetry/server/routes/index.ts index 12ba541d699f..2eb6bf95b4f4 100644 --- a/src/legacy/core_plugins/telemetry/server/routes/index.ts +++ b/src/legacy/core_plugins/telemetry/server/routes/index.ts @@ -21,7 +21,12 @@ import { CoreSetup } from 'src/core/server'; import { registerOptInRoutes } from './opt_in'; import { registerTelemetryDataRoutes } from './telemetry_stats'; -export function registerRoutes(core: CoreSetup) { - registerOptInRoutes(core); +interface RegisterRoutesParams { + core: CoreSetup; + currentKibanaVersion: string; +} + +export function registerRoutes({ core, currentKibanaVersion }: RegisterRoutesParams) { + registerOptInRoutes({ core, currentKibanaVersion }); registerTelemetryDataRoutes(core); } diff --git a/src/legacy/core_plugins/telemetry/server/routes/opt_in.ts b/src/legacy/core_plugins/telemetry/server/routes/opt_in.ts index aabc0259f08f..3a7194890b57 100644 --- a/src/legacy/core_plugins/telemetry/server/routes/opt_in.ts +++ b/src/legacy/core_plugins/telemetry/server/routes/opt_in.ts @@ -21,7 +21,17 @@ import Joi from 'joi'; import { boomify } from 'boom'; import { CoreSetup } from 'src/core/server'; -export function registerOptInRoutes(core: CoreSetup) { +interface RegisterOptInRoutesParams { + core: CoreSetup; + currentKibanaVersion: string; +} + +export interface SavedObjectAttributes { + enabled?: boolean; + lastVersionChecked: string; +} + +export function registerOptInRoutes({ core, currentKibanaVersion }: RegisterOptInRoutesParams) { const { server } = core.http as any; server.route({ @@ -36,17 +46,16 @@ export function registerOptInRoutes(core: CoreSetup) { }, handler: async (req: any, h: any) => { const savedObjectsClient = req.getSavedObjectsClient(); + const savedObject: SavedObjectAttributes = { + enabled: req.payload.enabled, + lastVersionChecked: currentKibanaVersion, + }; + const options = { + id: 'telemetry', + overwrite: true, + }; try { - await savedObjectsClient.create( - 'telemetry', - { - enabled: req.payload.enabled, - }, - { - id: 'telemetry', - overwrite: true, - } - ); + await savedObjectsClient.create('telemetry', savedObject, options); } catch (err) { return boomify(err); } diff --git a/x-pack/test/api_integration/apis/telemetry/index.js b/x-pack/test/api_integration/apis/telemetry/index.js index d941cda9e3fa..6f794d56ae71 100644 --- a/x-pack/test/api_integration/apis/telemetry/index.js +++ b/x-pack/test/api_integration/apis/telemetry/index.js @@ -8,5 +8,6 @@ export default function ({ loadTestFile }) { describe('Telemetry', () => { loadTestFile(require.resolve('./telemetry')); loadTestFile(require.resolve('./telemetry_local')); + loadTestFile(require.resolve('./opt_in')); }); } diff --git a/x-pack/test/api_integration/apis/telemetry/opt_in.ts b/x-pack/test/api_integration/apis/telemetry/opt_in.ts new file mode 100644 index 000000000000..d2ad2d773d69 --- /dev/null +++ b/x-pack/test/api_integration/apis/telemetry/opt_in.ts @@ -0,0 +1,63 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import expect from '@kbn/expect'; + +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function optInTest({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const kibanaServer = getService('kibanaServer'); + + describe('/api/telemetry/v2/optIn API', () => { + let kibanaVersion: any; + + before(async () => { + const kibanaVersionAccessor = kibanaServer.version; + kibanaVersion = await kibanaVersionAccessor.get(); + + expect(typeof kibanaVersion).to.eql('string'); + expect(kibanaVersion.length).to.be.greaterThan(0); + }); + + it('should support sending false', async () => { + await postTelemetryV2Optin(supertest, false, 200); + const { enabled, lastVersionChecked } = await getSavedObjectAttributes(supertest); + expect(enabled).to.be(false); + expect(lastVersionChecked).to.be(kibanaVersion); + }); + + it('should support sending true', async () => { + await postTelemetryV2Optin(supertest, true, 200); + const { enabled, lastVersionChecked } = await getSavedObjectAttributes(supertest); + expect(enabled).to.be(true); + expect(lastVersionChecked).to.be(kibanaVersion); + }); + + it('should not support sending null', async () => { + await postTelemetryV2Optin(supertest, null, 400); + }); + + it('should not support sending junk', async () => { + await postTelemetryV2Optin(supertest, 42, 400); + }); + }); +} + +async function postTelemetryV2Optin(supertest: any, value: any, statusCode: number): Promise { + const { body } = await supertest + .post('/api/telemetry/v2/optIn') + .set('kbn-xsrf', 'xxx') + .send({ enabled: value }) + .expect(statusCode); + + return body; +} + +async function getSavedObjectAttributes(supertest: any): Promise { + const { body } = await supertest.get('/api/saved_objects/telemetry/telemetry').expect(200); + return body.attributes; +}