From c9817589427721ce03ae4fac5cdb480258d5f9b6 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Wed, 17 Apr 2019 15:50:42 -0700 Subject: [PATCH 01/11] Block requests to ui_metric API if telemetry is disabled - Export track helper method from ui_metric app. - Remove createUiMetricUri helper. --- .../core_plugins/ui_metric/common/index.ts | 20 +++++++ .../ui_metric/{index.js => index.ts} | 3 + .../core_plugins/ui_metric/public/index.ts | 56 +++++++++++++++++++ .../ui_metric/server/routes/api/ui_metric.ts | 3 +- .../legacy/ui/public/ui_metric/index.ts | 10 +++- x-pack/common/ui_metric/index.ts | 7 --- .../public/app/services/track_ui_metric.js | 6 +- .../public/services/ui_metric.js | 5 +- .../public/services/track_ui_metric.js | 6 +- .../public/services/track_ui_metric.js | 6 +- .../crud_app/services/track_ui_metric.js | 6 +- .../public/hacks/on_session_timeout.js | 4 +- .../public/hacks/check_xpack_info_change.js | 25 +++++---- .../public/services/telemetry_opt_in.js | 5 +- 14 files changed, 119 insertions(+), 43 deletions(-) create mode 100644 src/legacy/core_plugins/ui_metric/common/index.ts rename src/legacy/core_plugins/ui_metric/{index.js => index.ts} (91%) create mode 100644 src/legacy/core_plugins/ui_metric/public/index.ts rename x-pack/common/ui_metric/ui_metric.ts => src/legacy/ui/public/ui_metric/index.ts (54%) delete mode 100644 x-pack/common/ui_metric/index.ts diff --git a/src/legacy/core_plugins/ui_metric/common/index.ts b/src/legacy/core_plugins/ui_metric/common/index.ts new file mode 100644 index 00000000000000..02aa55c30965d2 --- /dev/null +++ b/src/legacy/core_plugins/ui_metric/common/index.ts @@ -0,0 +1,20 @@ +/* + * 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. + */ + +export const API_BASE_PATH = '/api/ui_metric'; diff --git a/src/legacy/core_plugins/ui_metric/index.js b/src/legacy/core_plugins/ui_metric/index.ts similarity index 91% rename from src/legacy/core_plugins/ui_metric/index.js rename to src/legacy/core_plugins/ui_metric/index.ts index aadec215af998c..588cae9fe7a2a0 100644 --- a/src/legacy/core_plugins/ui_metric/index.js +++ b/src/legacy/core_plugins/ui_metric/index.ts @@ -17,6 +17,7 @@ * under the License. */ +import { resolve } from 'path'; import { registerUserActionRoute } from './server/routes/api/ui_metric'; import { registerUiMetricUsageCollector } from './server/usage/index'; @@ -24,9 +25,11 @@ export default function (kibana) { return new kibana.Plugin({ id: 'ui_metric', require: ['kibana', 'elasticsearch'], + publicDir: resolve(__dirname, 'public'), uiExports: { mappings: require('./mappings.json'), + hacks: [ 'plugins/ui_metric' ] }, init: function (server) { diff --git a/src/legacy/core_plugins/ui_metric/public/index.ts b/src/legacy/core_plugins/ui_metric/public/index.ts new file mode 100644 index 00000000000000..5b096db9a20ed4 --- /dev/null +++ b/src/legacy/core_plugins/ui_metric/public/index.ts @@ -0,0 +1,56 @@ +/* + * 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 chrome from 'ui/chrome'; +import { uiModules } from 'ui/modules'; +import { getCanGatherUiMetrics } from 'ui/ui_metric'; +import { API_BASE_PATH } from '../common'; + +const module = uiModules.get('kibana'); + +let _http; + +module.run($http => { + _http = $http; +}); + +module.config(($httpProvider) => { + $httpProvider.interceptors.push(($q) => { + return { + request: config => { + const { url } = config; + const isUiMetricRequest = url.indexOf(`${chrome.getBasePath()}${API_BASE_PATH}`) === 0; + const canGatherUiMetrics = getCanGatherUiMetrics(); + + if (isUiMetricRequest && !canGatherUiMetrics) { + // Block request from going out if the user has disabled telemetry. + return $q.reject('uiMetricsDisallowed'); + } else { + return $q.resolve(config); + } + }, + }; + }); +}); + +export function track(appName: string, actionType: string, httpClient: any = _http) { + const uri = chrome.addBasePath(`${API_BASE_PATH}/${appName}/${actionType}`); + // Silently swallow request failures. + httpClient.post(uri).then(() => {}, () => {}); +} diff --git a/src/legacy/core_plugins/ui_metric/server/routes/api/ui_metric.ts b/src/legacy/core_plugins/ui_metric/server/routes/api/ui_metric.ts index df717ae526f1e4..4bbafbb18f5509 100644 --- a/src/legacy/core_plugins/ui_metric/server/routes/api/ui_metric.ts +++ b/src/legacy/core_plugins/ui_metric/server/routes/api/ui_metric.ts @@ -19,13 +19,14 @@ import Boom from 'boom'; import { Server } from 'hapi'; +import { API_BASE_PATH } from '../../../common'; export const registerUserActionRoute = (server: Server) => { /* * Increment a count on an object representing a specific interaction with the UI. */ server.route({ - path: '/api/ui_metric/{appName}/{metricTypes}', + path: `${API_BASE_PATH}/{appName}/{metricTypes}`, method: 'POST', handler: async (request: any) => { const { appName, metricTypes } = request.params; diff --git a/x-pack/common/ui_metric/ui_metric.ts b/src/legacy/ui/public/ui_metric/index.ts similarity index 54% rename from x-pack/common/ui_metric/ui_metric.ts rename to src/legacy/ui/public/ui_metric/index.ts index 217344e3d97710..449602387a8a9b 100644 --- a/x-pack/common/ui_metric/ui_metric.ts +++ b/src/legacy/ui/public/ui_metric/index.ts @@ -4,8 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import chrome from 'ui/chrome'; +let _canGatherUiMetrics = false; -export function createUiMetricUri(appName: string, actionType: string): string { - return chrome.addBasePath(`/api/ui_metric/${appName}/${actionType}`); +export function setCanGatherUiMetrics(flag: boolean) { + _canGatherUiMetrics = flag; +} + +export function getCanGatherUiMetrics(): boolean { + return _canGatherUiMetrics; } diff --git a/x-pack/common/ui_metric/index.ts b/x-pack/common/ui_metric/index.ts deleted file mode 100644 index 5ba8b7520e7cc2..00000000000000 --- a/x-pack/common/ui_metric/index.ts +++ /dev/null @@ -1,7 +0,0 @@ -/* - * 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. - */ - -export { createUiMetricUri } from './ui_metric'; diff --git a/x-pack/plugins/cross_cluster_replication/public/app/services/track_ui_metric.js b/x-pack/plugins/cross_cluster_replication/public/app/services/track_ui_metric.js index 0cae59fbea5c78..0e70da24d27077 100644 --- a/x-pack/plugins/cross_cluster_replication/public/app/services/track_ui_metric.js +++ b/x-pack/plugins/cross_cluster_replication/public/app/services/track_ui_metric.js @@ -4,13 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ -import { createUiMetricUri } from '../../../../../common/ui_metric'; +import { track } from '../../../../../../src/legacy/core_plugins/ui_metric/public'; import { UIM_APP_NAME } from '../constants'; -import { getHttpClient } from './api'; export function trackUiMetric(actionType) { - const uiMetricUri = createUiMetricUri(UIM_APP_NAME, actionType); - getHttpClient().post(uiMetricUri); + track(UIM_APP_NAME, actionType); } /** diff --git a/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js b/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js index bc9d023ded5e67..18c0aa80e98139 100644 --- a/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js +++ b/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js @@ -6,7 +6,7 @@ import { get } from 'lodash'; -import { createUiMetricUri } from '../../../../common/ui_metric'; +import { track } from '../../../../../src/legacy/core_plugins/ui_metric/public'; import { UIM_APP_NAME, @@ -32,8 +32,7 @@ import { import { getHttpClient } from './api'; export function trackUiMetric(metricType, httpClient = getHttpClient()) { - const uiMetricUri = createUiMetricUri(UIM_APP_NAME, metricType); - httpClient.post(uiMetricUri); + track(UIM_APP_NAME, metricType, httpClient); } export function getUiMetricsForPhases(phases) { diff --git a/x-pack/plugins/index_management/public/services/track_ui_metric.js b/x-pack/plugins/index_management/public/services/track_ui_metric.js index 0e37a3b2a4fda0..9fb1125a2d91dd 100644 --- a/x-pack/plugins/index_management/public/services/track_ui_metric.js +++ b/x-pack/plugins/index_management/public/services/track_ui_metric.js @@ -4,11 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { createUiMetricUri } from '../../../../common/ui_metric'; +import { track } from '../../../../../src/legacy/core_plugins/ui_metric/public'; import { UIM_APP_NAME } from '../../common/constants'; -import { getHttpClient } from './api'; export function trackUiMetric(metricType) { - const uiMetricUri = createUiMetricUri(UIM_APP_NAME, metricType); - getHttpClient().post(uiMetricUri); + track(UIM_APP_NAME, metricType); } diff --git a/x-pack/plugins/remote_clusters/public/services/track_ui_metric.js b/x-pack/plugins/remote_clusters/public/services/track_ui_metric.js index d98815e95ff498..2587a79da8daa7 100644 --- a/x-pack/plugins/remote_clusters/public/services/track_ui_metric.js +++ b/x-pack/plugins/remote_clusters/public/services/track_ui_metric.js @@ -4,13 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ -import { createUiMetricUri } from '../../../../common/ui_metric'; +import { track } from '../../../../../src/legacy/core_plugins/ui_metric/public'; import { UIM_APP_NAME } from '../constants'; -import { getHttpClient } from './api'; export function trackUiMetric(actionType) { - const uiMetricUri = createUiMetricUri(UIM_APP_NAME, actionType); - getHttpClient().post(uiMetricUri); + track(UIM_APP_NAME, actionType); } /** diff --git a/x-pack/plugins/rollup/public/crud_app/services/track_ui_metric.js b/x-pack/plugins/rollup/public/crud_app/services/track_ui_metric.js index c616ca6e13d2a0..1d0d32e54c7344 100644 --- a/x-pack/plugins/rollup/public/crud_app/services/track_ui_metric.js +++ b/x-pack/plugins/rollup/public/crud_app/services/track_ui_metric.js @@ -4,13 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ -import { createUiMetricUri } from '../../../../../common/ui_metric'; +import { track } from '../../../../../../src/legacy/core_plugins/ui_metric/public'; import { UIM_APP_NAME } from '../../../common'; -import { getHttp } from './http_provider'; export function trackUiMetric(actionType) { - const uiMetricUri = createUiMetricUri(UIM_APP_NAME, actionType); - getHttp().post(uiMetricUri); + track(UIM_APP_NAME, actionType); } /** diff --git a/x-pack/plugins/security/public/hacks/on_session_timeout.js b/x-pack/plugins/security/public/hacks/on_session_timeout.js index ba110c5f0fbf22..9aff3f9e6e033e 100644 --- a/x-pack/plugins/security/public/hacks/on_session_timeout.js +++ b/x-pack/plugins/security/public/hacks/on_session_timeout.js @@ -65,7 +65,9 @@ module.config(($httpProvider) => { function interceptorFactory(responseHandler) { return function interceptor(response) { - if (!isUnauthenticated && !isSystemApiRequest(response.config) && sessionTimeout !== null) { + // If another interceptor has rejected a request, then config will be undefined. + const isSystemRequest = response.config ? isSystemApiRequest(response.config) : false; + if (!isUnauthenticated && !isSystemRequest && sessionTimeout !== null) { clearNotifications(); scheduleNotification(); } diff --git a/x-pack/plugins/xpack_main/public/hacks/check_xpack_info_change.js b/x-pack/plugins/xpack_main/public/hacks/check_xpack_info_change.js index ef3efaf99130e0..7d4745dfd7dad3 100644 --- a/x-pack/plugins/xpack_main/public/hacks/check_xpack_info_change.js +++ b/x-pack/plugins/xpack_main/public/hacks/check_xpack_info_change.js @@ -81,18 +81,21 @@ module.factory('checkXPackInfoChange', ($q, Private) => { return handleResponse(response); } - const currentSignature = response.headers('kbn-xpack-sig'); - const cachedSignature = xpackInfoSignature.get(); + // If another interceptor has rejected a request, then headers will be undefined. + if (response.headers) { + const currentSignature = response.headers('kbn-xpack-sig'); + const cachedSignature = xpackInfoSignature.get(); - if (currentSignature && cachedSignature !== currentSignature) { - // Signature from the server differ from the signature of our - // cached info, so we need to refresh it. - // Intentionally swallowing this error - // because nothing catches it and it's an ugly console error. - xpackInfo.refresh().then( - () => notifyIfLicenseIsExpired(), - () => {} - ); + if (currentSignature && cachedSignature !== currentSignature) { + // Signature from the server differ from the signature of our + // cached info, so we need to refresh it. + // Intentionally swallowing this error + // because nothing catches it and it's an ugly console error. + xpackInfo.refresh().then( + () => notifyIfLicenseIsExpired(), + () => {} + ); + } } return handleResponse(response); diff --git a/x-pack/plugins/xpack_main/public/services/telemetry_opt_in.js b/x-pack/plugins/xpack_main/public/services/telemetry_opt_in.js index b4070e4587f87d..4af694e79be5d3 100644 --- a/x-pack/plugins/xpack_main/public/services/telemetry_opt_in.js +++ b/x-pack/plugins/xpack_main/public/services/telemetry_opt_in.js @@ -5,16 +5,19 @@ */ import moment from 'moment'; +import { setCanGatherUiMetrics } from 'ui/ui_metric'; export function TelemetryOptInProvider($injector, chrome) { - const Notifier = $injector.get('Notifier'); const notify = new Notifier(); let currentOptInStatus = $injector.get('telemetryOptedIn'); + setCanGatherUiMetrics(currentOptInStatus); return { getOptIn: () => currentOptInStatus, setOptIn: async (enabled) => { + setCanGatherUiMetrics(enabled); + const $http = $injector.get('$http'); try { From 9312131829473ccc36dd8744bf45d2db156710e4 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Wed, 17 Apr 2019 16:35:22 -0700 Subject: [PATCH 02/11] Remove unnecessary httpClient argument. --- src/legacy/core_plugins/ui_metric/public/index.ts | 4 ++-- .../index_lifecycle_management/public/services/api.js | 10 +++++----- .../public/services/ui_metric.js | 6 ++---- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/legacy/core_plugins/ui_metric/public/index.ts b/src/legacy/core_plugins/ui_metric/public/index.ts index 5b096db9a20ed4..910538337c8bf9 100644 --- a/src/legacy/core_plugins/ui_metric/public/index.ts +++ b/src/legacy/core_plugins/ui_metric/public/index.ts @@ -49,8 +49,8 @@ module.config(($httpProvider) => { }); }); -export function track(appName: string, actionType: string, httpClient: any = _http) { +export function track(appName: string, actionType: string) { const uri = chrome.addBasePath(`${API_BASE_PATH}/${appName}/${actionType}`); // Silently swallow request failures. - httpClient.post(uri).then(() => {}, () => {}); + _http.post(uri).then(() => {}, () => {}); } diff --git a/x-pack/plugins/index_lifecycle_management/public/services/api.js b/x-pack/plugins/index_lifecycle_management/public/services/api.js index df799897ee81e4..e2463ce61d83a5 100644 --- a/x-pack/plugins/index_lifecycle_management/public/services/api.js +++ b/x-pack/plugins/index_lifecycle_management/public/services/api.js @@ -53,7 +53,7 @@ export async function loadPolicies(withIndices, httpClient = getHttpClient()) { export async function deletePolicy(policyName, httpClient = getHttpClient()) { const response = await httpClient.delete(`${apiPrefix}/policies/${encodeURIComponent(policyName)}`); // Only track successful actions. - trackUiMetric(UIM_POLICY_DELETE, httpClient); + trackUiMetric(UIM_POLICY_DELETE); return response.data; } @@ -73,27 +73,27 @@ export async function getAffectedIndices(indexTemplateName, policyName, httpClie export const retryLifecycleForIndex = async (indexNames, httpClient = getHttpClient()) => { const response = await httpClient.post(`${apiPrefix}/index/retry`, { indexNames }); // Only track successful actions. - trackUiMetric(UIM_INDEX_RETRY_STEP, httpClient); + trackUiMetric(UIM_INDEX_RETRY_STEP); return response.data; }; export const removeLifecycleForIndex = async (indexNames, httpClient = getHttpClient()) => { const response = await httpClient.post(`${apiPrefix}/index/remove`, { indexNames }); // Only track successful actions. - trackUiMetric(UIM_POLICY_DETACH_INDEX, httpClient); + trackUiMetric(UIM_POLICY_DETACH_INDEX); return response.data; }; export const addLifecyclePolicyToIndex = async (body, httpClient = getHttpClient()) => { const response = await httpClient.post(`${apiPrefix}/index/add`, body); // Only track successful actions. - trackUiMetric(UIM_POLICY_ATTACH_INDEX, httpClient); + trackUiMetric(UIM_POLICY_ATTACH_INDEX); return response.data; }; export const addLifecyclePolicyToTemplate = async (body, httpClient = getHttpClient()) => { const response = await httpClient.post(`${apiPrefix}/template`, body); // Only track successful actions. - trackUiMetric(UIM_POLICY_ATTACH_INDEX_TEMPLATE, httpClient); + trackUiMetric(UIM_POLICY_ATTACH_INDEX_TEMPLATE); return response.data; }; diff --git a/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js b/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js index 18c0aa80e98139..bd22e38968ea05 100644 --- a/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js +++ b/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js @@ -29,10 +29,8 @@ import { defaultHotPhase, } from '../store/defaults'; -import { getHttpClient } from './api'; - -export function trackUiMetric(metricType, httpClient = getHttpClient()) { - track(UIM_APP_NAME, metricType, httpClient); +export function trackUiMetric(metricType) { + track(UIM_APP_NAME, metricType); } export function getUiMetricsForPhases(phases) { From f78f6b2bf7d8c83df106d0b5d545649b4d191fd5 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Wed, 17 Apr 2019 17:18:13 -0700 Subject: [PATCH 03/11] Fix lint errors and tests. --- src/legacy/core_plugins/ui_metric/index.ts | 9 +++++---- .../core_plugins/ui_metric/public/index.ts | 4 ++-- src/legacy/ui/public/ui_metric/index.ts | 19 ++++++++++++++++--- .../auto_follow_pattern_list.test.js | 7 +++++-- .../follower_indices_list.test.js | 7 +++++-- .../__jest__/client_integration/home.test.js | 7 +++++-- .../remote_clusters_list.test.js | 7 +++++-- .../job_create_review.test.js | 3 +++ .../client_integration/job_list.test.js | 4 ++++ 9 files changed, 50 insertions(+), 17 deletions(-) diff --git a/src/legacy/core_plugins/ui_metric/index.ts b/src/legacy/core_plugins/ui_metric/index.ts index 588cae9fe7a2a0..7d21962657254f 100644 --- a/src/legacy/core_plugins/ui_metric/index.ts +++ b/src/legacy/core_plugins/ui_metric/index.ts @@ -21,7 +21,8 @@ import { resolve } from 'path'; import { registerUserActionRoute } from './server/routes/api/ui_metric'; import { registerUiMetricUsageCollector } from './server/usage/index'; -export default function (kibana) { +// eslint-disable-next-line import/no-default-export +export default function(kibana) { return new kibana.Plugin({ id: 'ui_metric', require: ['kibana', 'elasticsearch'], @@ -29,12 +30,12 @@ export default function (kibana) { uiExports: { mappings: require('./mappings.json'), - hacks: [ 'plugins/ui_metric' ] + hacks: ['plugins/ui_metric'], }, - init: function (server) { + init(server) { registerUserActionRoute(server); registerUiMetricUsageCollector(server); - } + }, }); } diff --git a/src/legacy/core_plugins/ui_metric/public/index.ts b/src/legacy/core_plugins/ui_metric/public/index.ts index 910538337c8bf9..73849e6a67cdf7 100644 --- a/src/legacy/core_plugins/ui_metric/public/index.ts +++ b/src/legacy/core_plugins/ui_metric/public/index.ts @@ -30,8 +30,8 @@ module.run($http => { _http = $http; }); -module.config(($httpProvider) => { - $httpProvider.interceptors.push(($q) => { +module.config($httpProvider => { + $httpProvider.interceptors.push($q => { return { request: config => { const { url } = config; diff --git a/src/legacy/ui/public/ui_metric/index.ts b/src/legacy/ui/public/ui_metric/index.ts index 449602387a8a9b..71457ee65392a9 100644 --- a/src/legacy/ui/public/ui_metric/index.ts +++ b/src/legacy/ui/public/ui_metric/index.ts @@ -1,7 +1,20 @@ /* - * 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. + * 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. */ let _canGatherUiMetrics = false; diff --git a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/auto_follow_pattern_list.test.js b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/auto_follow_pattern_list.test.js index cd0d6d47bc5e66..90268b0515f419 100644 --- a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/auto_follow_pattern_list.test.js +++ b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/auto_follow_pattern_list.test.js @@ -21,6 +21,10 @@ jest.mock('ui/index_patterns', () => { return { INDEX_PATTERN_ILLEGAL_CHARACTERS_VISIBLE }; }); +jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ + track: jest.fn(), +})); + describe('', () => { let server; let find; @@ -37,8 +41,7 @@ describe('', () => { beforeEach(() => { server = sinon.fakeServer.create(); server.respondImmediately = true; - // We make requests to APIs which don't impact the UX, e.g. UI metric telemetry, - // and we can mock them all with a 200 instead of mocking each one individually. + // Mock all HTTP Requests that have not been handled previously server.respondWith([200, {}, '']); // Register helpers to mock Http Requests diff --git a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/follower_indices_list.test.js b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/follower_indices_list.test.js index 7baee35e658352..9c5c2736d35272 100644 --- a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/follower_indices_list.test.js +++ b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/follower_indices_list.test.js @@ -21,6 +21,10 @@ jest.mock('ui/index_patterns', () => { return { INDEX_PATTERN_ILLEGAL_CHARACTERS_VISIBLE }; }); +jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ + track: jest.fn(), +})); + describe('', () => { let server; let find; @@ -34,8 +38,7 @@ describe('', () => { beforeEach(() => { server = sinon.fakeServer.create(); server.respondImmediately = true; - // We make requests to APIs which don't impact the UX, e.g. UI metric telemetry, - // and we can mock them all with a 200 instead of mocking each one individually. + // Mock all HTTP Requests that have not been handled previously server.respondWith([200, {}, '']); // Register helpers to mock Http Requests diff --git a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/home.test.js b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/home.test.js index 9e87b013394ddd..2cdfeabd9993cf 100644 --- a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/home.test.js +++ b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/home.test.js @@ -32,6 +32,10 @@ jest.mock('ui/index_patterns', () => { return { INDEX_PATTERN_ILLEGAL_CHARACTERS_VISIBLE }; }); +jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ + track: jest.fn(), +})); + const testBedOptions = { memoryRouter: { initialEntries: [`${BASE_PATH}/follower_indices`], @@ -49,8 +53,7 @@ describe('', () => { beforeEach(() => { server = sinon.fakeServer.create(); server.respondImmediately = true; - // We make requests to APIs which don't impact the UX, e.g. UI metric telemetry, - // and we can mock them all with a 200 instead of mocking each one individually. + // Mock all HTTP Requests that have not been handled previously server.respondWith([200, {}, '']); // Register helpers to mock Http Requests diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js index 4f1ddb4a9a813f..ab5b31c591316f 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js @@ -26,6 +26,10 @@ jest.mock('ui/chrome', () => ({ } })); +jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ + track: jest.fn(), +})); + const testBedOptions = { memoryRouter: { onRouter: (router) => registerRouter(router) @@ -47,8 +51,7 @@ describe('', () => { beforeEach(() => { server = sinon.fakeServer.create(); server.respondImmediately = true; - // We make requests to APIs which don't impact the UX, e.g. UI metric telemetry, - // and we can mock them all with a 200 instead of mocking each one individually. + // Mock all HTTP Requests that have not been handled previously server.respondWith([200, {}, '']); // Register helpers to mock Http Requests diff --git a/x-pack/plugins/rollup/__jest__/client_integration/job_create_review.test.js b/x-pack/plugins/rollup/__jest__/client_integration/job_create_review.test.js index c6629640e1996d..1090ee6079c162 100644 --- a/x-pack/plugins/rollup/__jest__/client_integration/job_create_review.test.js +++ b/x-pack/plugins/rollup/__jest__/client_integration/job_create_review.test.js @@ -20,6 +20,9 @@ jest.mock('ui/chrome', () => ({ jest.mock('lodash/function/debounce', () => fn => fn); +jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ + track: jest.fn(), +})); describe('Create Rollup Job, step 5: Metrics', () => { let server; diff --git a/x-pack/plugins/rollup/__jest__/client_integration/job_list.test.js b/x-pack/plugins/rollup/__jest__/client_integration/job_list.test.js index 5af5be2997b4d2..478de45ff4d1ab 100644 --- a/x-pack/plugins/rollup/__jest__/client_integration/job_list.test.js +++ b/x-pack/plugins/rollup/__jest__/client_integration/job_list.test.js @@ -30,6 +30,10 @@ jest.mock('ui/chrome', () => ({ } })); +jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ + track: jest.fn(), +})); + jest.mock('../../public/crud_app/services', () => { const services = require.requireActual('../../public/crud_app/services'); return { From 8bdf554f55b5a1acc2e0d2d883b737f7eb9753e0 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 18 Apr 2019 08:11:27 -0700 Subject: [PATCH 04/11] Add comments. --- src/legacy/core_plugins/ui_metric/public/index.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/ui_metric/public/index.ts b/src/legacy/core_plugins/ui_metric/public/index.ts index 73849e6a67cdf7..1e6273aef7689b 100644 --- a/src/legacy/core_plugins/ui_metric/public/index.ts +++ b/src/legacy/core_plugins/ui_metric/public/index.ts @@ -51,6 +51,8 @@ module.config($httpProvider => { export function track(appName: string, actionType: string) { const uri = chrome.addBasePath(`${API_BASE_PATH}/${appName}/${actionType}`); - // Silently swallow request failures. - _http.post(uri).then(() => {}, () => {}); + // Silently swallow request failures. Without this empty error handler, Angular will complain + // in the console that the rejected promise isn't being handled. + const emptyHandler = () => {}; + _http.post(uri).then(emptyHandler, emptyHandler); } From d084eaa65e09597e91f14b9f377e375cbe57a65c Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 18 Apr 2019 10:01:04 -0700 Subject: [PATCH 05/11] Rename track to trackUiMetric, and simplify logic to not involve interceptors. - Add support for an array of metric types. - Update README. --- src/legacy/core_plugins/ui_metric/README.md | 22 ++++++++---- .../core_plugins/ui_metric/public/index.ts | 36 +++++-------------- src/legacy/ui/public/ui_metric/index.ts | 10 +++--- .../auto_follow_pattern_list.test.js | 2 +- .../follower_indices_list.test.js | 2 +- .../__jest__/client_integration/home.test.js | 2 +- .../public/app/services/api.js | 4 +-- .../public/app/services/track_ui_metric.js | 2 +- .../public/services/ui_metric.js | 2 +- .../public/store/actions/lifecycle.js | 2 +- .../public/services/track_ui_metric.js | 2 +- .../remote_clusters_list.test.js | 2 +- .../public/services/track_ui_metric.js | 2 +- .../job_create_review.test.js | 2 +- .../client_integration/job_list.test.js | 2 +- .../crud_app/services/track_ui_metric.js | 2 +- .../public/services/telemetry_opt_in.js | 6 ++-- 17 files changed, 45 insertions(+), 57 deletions(-) diff --git a/src/legacy/core_plugins/ui_metric/README.md b/src/legacy/core_plugins/ui_metric/README.md index ad05589d327b8f..15a7f20db6d39a 100644 --- a/src/legacy/core_plugins/ui_metric/README.md +++ b/src/legacy/core_plugins/ui_metric/README.md @@ -16,14 +16,23 @@ the name of a dashboard they've viewed, or the timestamp of the interaction. ## How to use it -To track a user interaction, simply send a `POST` request to `/api/ui_metric/{APP_NAME}/{METRIC_TYPE}`, -where `APP_NAME` and `METRIC_TYPE` are underscore-delimited strings. For example, to track the app -`my_app` and the metric `my_metric`, send a request to `/api/ui_metric/my_app/my_metric`. +To track a user interaction, import the `trackUiMetric` helper function from UI Metric app: + +```js +import { trackUiMetric } from 'relative/path/to/src/legacy/core_plugins/ui_metric/public'; +``` + +Call this function whenever you would like to track a user interaction within your app. The function +accepts two arguments, `appName` and `metricType`. These should be underscore-delimited strings. +For example, to track the `my_metric` metric in the app `my_app` call `trackUiMetric('my_app', 'my_metric)`. That's all you need to do! -To track multiple metrics within a single request, provide multiple metric types separated by -commas, e.g. `/api/ui_metric/my_app/my_metric1,my_metric2,my_metric3`. +To track multiple metrics within a single request, provide an array of metric types, e.g. `trackUiMetric('my_app', ['my_metric1', 'my_metric2', 'my_metric3'])`. + +**NOTE:** When called, this function sends a `POST` request to `/api/ui_metric/{appName}/{metricType}`. +It's important that this request is sent via the `trackUiMetric` function, because it contains special +logic for blocking the request if the user hasn't opted in to telemetry. ### Tracking timed interactions @@ -32,8 +41,7 @@ logic yourself. You'll also need to predefine some buckets into which the UI met For example, if you're timing how long it takes to create a visualization, you may decide to measure interactions that take less than 1 minute, 1-5 minutes, 5-20 minutes, and longer than 20 minutes. To track these interactions, you'd use the timed length of the interaction to determine whether to -hit `/api/ui_metric/visualize/create_vis_1m`, `/api/ui_metric/visualize/create_vis_5m`, -`/api/ui_metric/visualize/create_vis_20m`, etc. +use a `metricType` of `create_vis_1m`, `create_vis_5m`, `create_vis_20m`, or `create_vis_infinity`. ## How it works diff --git a/src/legacy/core_plugins/ui_metric/public/index.ts b/src/legacy/core_plugins/ui_metric/public/index.ts index 1e6273aef7689b..2a21eea7e80595 100644 --- a/src/legacy/core_plugins/ui_metric/public/index.ts +++ b/src/legacy/core_plugins/ui_metric/public/index.ts @@ -19,40 +19,20 @@ import chrome from 'ui/chrome'; import { uiModules } from 'ui/modules'; -import { getCanGatherUiMetrics } from 'ui/ui_metric'; +import { getCanTrackUiMetrics } from 'ui/ui_metric'; import { API_BASE_PATH } from '../common'; -const module = uiModules.get('kibana'); - let _http; -module.run($http => { +uiModules.get('kibana').run($http => { _http = $http; }); -module.config($httpProvider => { - $httpProvider.interceptors.push($q => { - return { - request: config => { - const { url } = config; - const isUiMetricRequest = url.indexOf(`${chrome.getBasePath()}${API_BASE_PATH}`) === 0; - const canGatherUiMetrics = getCanGatherUiMetrics(); - - if (isUiMetricRequest && !canGatherUiMetrics) { - // Block request from going out if the user has disabled telemetry. - return $q.reject('uiMetricsDisallowed'); - } else { - return $q.resolve(config); - } - }, - }; - }); -}); +export function trackUiMetric(appName: string, metricType: string | string[]) { + const metricTypes = Array.isArray(metricType) ? metricType.join(',') : metricType; + const uri = chrome.addBasePath(`${API_BASE_PATH}/${appName}/${metricTypes}`); -export function track(appName: string, actionType: string) { - const uri = chrome.addBasePath(`${API_BASE_PATH}/${appName}/${actionType}`); - // Silently swallow request failures. Without this empty error handler, Angular will complain - // in the console that the rejected promise isn't being handled. - const emptyHandler = () => {}; - _http.post(uri).then(emptyHandler, emptyHandler); + if (getCanTrackUiMetrics()) { + _http.post(uri); + } } diff --git a/src/legacy/ui/public/ui_metric/index.ts b/src/legacy/ui/public/ui_metric/index.ts index 71457ee65392a9..ad43f27201ce4d 100644 --- a/src/legacy/ui/public/ui_metric/index.ts +++ b/src/legacy/ui/public/ui_metric/index.ts @@ -17,12 +17,12 @@ * under the License. */ -let _canGatherUiMetrics = false; +let _canTrackUiMetrics = false; -export function setCanGatherUiMetrics(flag: boolean) { - _canGatherUiMetrics = flag; +export function setCanTrackUiMetrics(flag: boolean) { + _canTrackUiMetrics = flag; } -export function getCanGatherUiMetrics(): boolean { - return _canGatherUiMetrics; +export function getCanTrackUiMetrics(): boolean { + return _canTrackUiMetrics; } diff --git a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/auto_follow_pattern_list.test.js b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/auto_follow_pattern_list.test.js index 90268b0515f419..1aa24df751a37e 100644 --- a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/auto_follow_pattern_list.test.js +++ b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/auto_follow_pattern_list.test.js @@ -22,7 +22,7 @@ jest.mock('ui/index_patterns', () => { }); jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ - track: jest.fn(), + trackUiMetric: jest.fn(), })); describe('', () => { diff --git a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/follower_indices_list.test.js b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/follower_indices_list.test.js index 9c5c2736d35272..045d76fd1b3550 100644 --- a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/follower_indices_list.test.js +++ b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/follower_indices_list.test.js @@ -22,7 +22,7 @@ jest.mock('ui/index_patterns', () => { }); jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ - track: jest.fn(), + trackUiMetric: jest.fn(), })); describe('', () => { diff --git a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/home.test.js b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/home.test.js index 2cdfeabd9993cf..d2f7de30fbd204 100644 --- a/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/home.test.js +++ b/x-pack/plugins/cross_cluster_replication/__jest__/client_integration/home.test.js @@ -33,7 +33,7 @@ jest.mock('ui/index_patterns', () => { }); jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ - track: jest.fn(), + trackUiMetric: jest.fn(), })); const testBedOptions = { diff --git a/x-pack/plugins/cross_cluster_replication/public/app/services/api.js b/x-pack/plugins/cross_cluster_replication/public/app/services/api.js index cfbfcbb61966c8..047dc96cda8cfd 100644 --- a/x-pack/plugins/cross_cluster_replication/public/app/services/api.js +++ b/x-pack/plugins/cross_cluster_replication/public/app/services/api.js @@ -104,7 +104,7 @@ export const createFollowerIndex = (followerIndex) => { uiMetrics.push(UIM_FOLLOWER_INDEX_USE_ADVANCED_OPTIONS); } const request = httpClient.post(`${apiPrefix}/follower_indices`, followerIndex); - return trackUserRequest(request, uiMetrics.join(',')).then(extractData); + return trackUserRequest(request, uiMetrics).then(extractData); }; export const pauseFollowerIndex = (id) => { @@ -138,7 +138,7 @@ export const updateFollowerIndex = (id, followerIndex) => { uiMetrics.push(UIM_FOLLOWER_INDEX_USE_ADVANCED_OPTIONS); } const request = httpClient.put(`${apiPrefix}/follower_indices/${encodeURIComponent(id)}`, followerIndex); - return trackUserRequest(request, uiMetrics.join(',')).then(extractData); + return trackUserRequest(request, uiMetrics).then(extractData); }; /* Stats */ diff --git a/x-pack/plugins/cross_cluster_replication/public/app/services/track_ui_metric.js b/x-pack/plugins/cross_cluster_replication/public/app/services/track_ui_metric.js index 0e70da24d27077..2824644e6f08fe 100644 --- a/x-pack/plugins/cross_cluster_replication/public/app/services/track_ui_metric.js +++ b/x-pack/plugins/cross_cluster_replication/public/app/services/track_ui_metric.js @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { track } from '../../../../../../src/legacy/core_plugins/ui_metric/public'; +import { trackUiMetric as track } from '../../../../../../src/legacy/core_plugins/ui_metric/public'; import { UIM_APP_NAME } from '../constants'; export function trackUiMetric(actionType) { diff --git a/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js b/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js index bd22e38968ea05..408c70636c3d92 100644 --- a/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js +++ b/x-pack/plugins/index_lifecycle_management/public/services/ui_metric.js @@ -6,7 +6,7 @@ import { get } from 'lodash'; -import { track } from '../../../../../src/legacy/core_plugins/ui_metric/public'; +import { trackUiMetric as track } from '../../../../../src/legacy/core_plugins/ui_metric/public'; import { UIM_APP_NAME, diff --git a/x-pack/plugins/index_lifecycle_management/public/store/actions/lifecycle.js b/x-pack/plugins/index_lifecycle_management/public/store/actions/lifecycle.js index 60cb86dd2d86ff..095de055690925 100644 --- a/x-pack/plugins/index_lifecycle_management/public/store/actions/lifecycle.js +++ b/x-pack/plugins/index_lifecycle_management/public/store/actions/lifecycle.js @@ -32,7 +32,7 @@ export const saveLifecyclePolicy = (lifecycle, isNew) => async () => { const uiMetrics = getUiMetricsForPhases(lifecycle.phases); uiMetrics.push(isNew ? UIM_POLICY_CREATE : UIM_POLICY_UPDATE); - trackUiMetric(uiMetrics.join(',')); + trackUiMetric(uiMetrics); const message = i18n.translate('xpack.indexLifecycleMgmt.editPolicy.successfulSaveMessage', { diff --git a/x-pack/plugins/index_management/public/services/track_ui_metric.js b/x-pack/plugins/index_management/public/services/track_ui_metric.js index 9fb1125a2d91dd..443d218ae7b7c2 100644 --- a/x-pack/plugins/index_management/public/services/track_ui_metric.js +++ b/x-pack/plugins/index_management/public/services/track_ui_metric.js @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { track } from '../../../../../src/legacy/core_plugins/ui_metric/public'; +import { trackUiMetric as track } from '../../../../../src/legacy/core_plugins/ui_metric/public'; import { UIM_APP_NAME } from '../../common/constants'; export function trackUiMetric(metricType) { diff --git a/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js b/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js index ab5b31c591316f..ce8558efb9206d 100644 --- a/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js +++ b/x-pack/plugins/remote_clusters/__jest__/client_integration/remote_clusters_list.test.js @@ -27,7 +27,7 @@ jest.mock('ui/chrome', () => ({ })); jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ - track: jest.fn(), + trackUiMetric: jest.fn(), })); const testBedOptions = { diff --git a/x-pack/plugins/remote_clusters/public/services/track_ui_metric.js b/x-pack/plugins/remote_clusters/public/services/track_ui_metric.js index 2587a79da8daa7..178c49d2eb7fc5 100644 --- a/x-pack/plugins/remote_clusters/public/services/track_ui_metric.js +++ b/x-pack/plugins/remote_clusters/public/services/track_ui_metric.js @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { track } from '../../../../../src/legacy/core_plugins/ui_metric/public'; +import { trackUiMetric as track } from '../../../../../src/legacy/core_plugins/ui_metric/public'; import { UIM_APP_NAME } from '../constants'; export function trackUiMetric(actionType) { diff --git a/x-pack/plugins/rollup/__jest__/client_integration/job_create_review.test.js b/x-pack/plugins/rollup/__jest__/client_integration/job_create_review.test.js index 1090ee6079c162..e7d6792d7fe5ef 100644 --- a/x-pack/plugins/rollup/__jest__/client_integration/job_create_review.test.js +++ b/x-pack/plugins/rollup/__jest__/client_integration/job_create_review.test.js @@ -21,7 +21,7 @@ jest.mock('ui/chrome', () => ({ jest.mock('lodash/function/debounce', () => fn => fn); jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ - track: jest.fn(), + trackUiMetric: jest.fn(), })); describe('Create Rollup Job, step 5: Metrics', () => { diff --git a/x-pack/plugins/rollup/__jest__/client_integration/job_list.test.js b/x-pack/plugins/rollup/__jest__/client_integration/job_list.test.js index 478de45ff4d1ab..fb74cb91834b3f 100644 --- a/x-pack/plugins/rollup/__jest__/client_integration/job_list.test.js +++ b/x-pack/plugins/rollup/__jest__/client_integration/job_list.test.js @@ -31,7 +31,7 @@ jest.mock('ui/chrome', () => ({ })); jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({ - track: jest.fn(), + trackUiMetric: jest.fn(), })); jest.mock('../../public/crud_app/services', () => { diff --git a/x-pack/plugins/rollup/public/crud_app/services/track_ui_metric.js b/x-pack/plugins/rollup/public/crud_app/services/track_ui_metric.js index 1d0d32e54c7344..1152135f53bc7f 100644 --- a/x-pack/plugins/rollup/public/crud_app/services/track_ui_metric.js +++ b/x-pack/plugins/rollup/public/crud_app/services/track_ui_metric.js @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { track } from '../../../../../../src/legacy/core_plugins/ui_metric/public'; +import { trackUiMetric as track } from '../../../../../../src/legacy/core_plugins/ui_metric/public'; import { UIM_APP_NAME } from '../../../common'; export function trackUiMetric(actionType) { diff --git a/x-pack/plugins/xpack_main/public/services/telemetry_opt_in.js b/x-pack/plugins/xpack_main/public/services/telemetry_opt_in.js index 4af694e79be5d3..e086702b17a8df 100644 --- a/x-pack/plugins/xpack_main/public/services/telemetry_opt_in.js +++ b/x-pack/plugins/xpack_main/public/services/telemetry_opt_in.js @@ -5,18 +5,18 @@ */ import moment from 'moment'; -import { setCanGatherUiMetrics } from 'ui/ui_metric'; +import { setCanTrackUiMetrics } from 'ui/ui_metric'; export function TelemetryOptInProvider($injector, chrome) { const Notifier = $injector.get('Notifier'); const notify = new Notifier(); let currentOptInStatus = $injector.get('telemetryOptedIn'); - setCanGatherUiMetrics(currentOptInStatus); + setCanTrackUiMetrics(currentOptInStatus); return { getOptIn: () => currentOptInStatus, setOptIn: async (enabled) => { - setCanGatherUiMetrics(enabled); + setCanTrackUiMetrics(enabled); const $http = $injector.get('$http'); From 39e45663952dfe601cecbdeffcb4b345aa7831c8 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 18 Apr 2019 10:05:46 -0700 Subject: [PATCH 06/11] Exit early. --- src/legacy/core_plugins/ui_metric/public/index.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/legacy/core_plugins/ui_metric/public/index.ts b/src/legacy/core_plugins/ui_metric/public/index.ts index 2a21eea7e80595..bacfc0e0219565 100644 --- a/src/legacy/core_plugins/ui_metric/public/index.ts +++ b/src/legacy/core_plugins/ui_metric/public/index.ts @@ -29,10 +29,11 @@ uiModules.get('kibana').run($http => { }); export function trackUiMetric(appName: string, metricType: string | string[]) { + if (!getCanTrackUiMetrics()) { + return; + } + const metricTypes = Array.isArray(metricType) ? metricType.join(',') : metricType; const uri = chrome.addBasePath(`${API_BASE_PATH}/${appName}/${metricTypes}`); - - if (getCanTrackUiMetrics()) { - _http.post(uri); - } + _http.post(uri); } From 83b364e65cf7afa6c403c6e55cd5abe783109465 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 18 Apr 2019 10:09:06 -0700 Subject: [PATCH 07/11] Revert changes to unrelated files. --- .../public/hacks/on_session_timeout.js | 4 +-- .../public/hacks/check_xpack_info_change.js | 25 ++++++++----------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/security/public/hacks/on_session_timeout.js b/x-pack/plugins/security/public/hacks/on_session_timeout.js index 9aff3f9e6e033e..ba110c5f0fbf22 100644 --- a/x-pack/plugins/security/public/hacks/on_session_timeout.js +++ b/x-pack/plugins/security/public/hacks/on_session_timeout.js @@ -65,9 +65,7 @@ module.config(($httpProvider) => { function interceptorFactory(responseHandler) { return function interceptor(response) { - // If another interceptor has rejected a request, then config will be undefined. - const isSystemRequest = response.config ? isSystemApiRequest(response.config) : false; - if (!isUnauthenticated && !isSystemRequest && sessionTimeout !== null) { + if (!isUnauthenticated && !isSystemApiRequest(response.config) && sessionTimeout !== null) { clearNotifications(); scheduleNotification(); } diff --git a/x-pack/plugins/xpack_main/public/hacks/check_xpack_info_change.js b/x-pack/plugins/xpack_main/public/hacks/check_xpack_info_change.js index 7d4745dfd7dad3..ef3efaf99130e0 100644 --- a/x-pack/plugins/xpack_main/public/hacks/check_xpack_info_change.js +++ b/x-pack/plugins/xpack_main/public/hacks/check_xpack_info_change.js @@ -81,21 +81,18 @@ module.factory('checkXPackInfoChange', ($q, Private) => { return handleResponse(response); } - // If another interceptor has rejected a request, then headers will be undefined. - if (response.headers) { - const currentSignature = response.headers('kbn-xpack-sig'); - const cachedSignature = xpackInfoSignature.get(); + const currentSignature = response.headers('kbn-xpack-sig'); + const cachedSignature = xpackInfoSignature.get(); - if (currentSignature && cachedSignature !== currentSignature) { - // Signature from the server differ from the signature of our - // cached info, so we need to refresh it. - // Intentionally swallowing this error - // because nothing catches it and it's an ugly console error. - xpackInfo.refresh().then( - () => notifyIfLicenseIsExpired(), - () => {} - ); - } + if (currentSignature && cachedSignature !== currentSignature) { + // Signature from the server differ from the signature of our + // cached info, so we need to refresh it. + // Intentionally swallowing this error + // because nothing catches it and it's an ugly console error. + xpackInfo.refresh().then( + () => notifyIfLicenseIsExpired(), + () => {} + ); } return handleResponse(response); From 15a468002c69f574145dea50f827164812fa6eff Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 18 Apr 2019 13:00:06 -0700 Subject: [PATCH 08/11] Fix typings. --- src/legacy/core_plugins/ui_metric/index.ts | 5 +++-- src/legacy/core_plugins/ui_metric/public/index.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/legacy/core_plugins/ui_metric/index.ts b/src/legacy/core_plugins/ui_metric/index.ts index 7d21962657254f..34ca346b8756e9 100644 --- a/src/legacy/core_plugins/ui_metric/index.ts +++ b/src/legacy/core_plugins/ui_metric/index.ts @@ -18,11 +18,12 @@ */ import { resolve } from 'path'; +import { Legacy } from '../../../../kibana'; import { registerUserActionRoute } from './server/routes/api/ui_metric'; import { registerUiMetricUsageCollector } from './server/usage/index'; // eslint-disable-next-line import/no-default-export -export default function(kibana) { +export default function(kibana: any) { return new kibana.Plugin({ id: 'ui_metric', require: ['kibana', 'elasticsearch'], @@ -33,7 +34,7 @@ export default function(kibana) { hacks: ['plugins/ui_metric'], }, - init(server) { + init(server: Legacy.Server) { registerUserActionRoute(server); registerUiMetricUsageCollector(server); }, diff --git a/src/legacy/core_plugins/ui_metric/public/index.ts b/src/legacy/core_plugins/ui_metric/public/index.ts index bacfc0e0219565..38b98ffdfc9edb 100644 --- a/src/legacy/core_plugins/ui_metric/public/index.ts +++ b/src/legacy/core_plugins/ui_metric/public/index.ts @@ -18,13 +18,14 @@ */ import chrome from 'ui/chrome'; +// @ts-ignore import { uiModules } from 'ui/modules'; import { getCanTrackUiMetrics } from 'ui/ui_metric'; import { API_BASE_PATH } from '../common'; -let _http; +let _http: any; -uiModules.get('kibana').run($http => { +uiModules.get('kibana').run(($http: any) => { _http = $http; }); From 750f32961699da40bf925ed63c2d1f9a52092646 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 22 Apr 2019 11:07:45 -0700 Subject: [PATCH 09/11] Add guidance and throw error if trackUiMetric is called with a string containing a colon. --- src/legacy/core_plugins/ui_metric/README.md | 10 +++++++++- src/legacy/core_plugins/ui_metric/public/index.ts | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/legacy/core_plugins/ui_metric/README.md b/src/legacy/core_plugins/ui_metric/README.md index 15a7f20db6d39a..5035e3d458bddc 100644 --- a/src/legacy/core_plugins/ui_metric/README.md +++ b/src/legacy/core_plugins/ui_metric/README.md @@ -34,6 +34,12 @@ To track multiple metrics within a single request, provide an array of metric ty It's important that this request is sent via the `trackUiMetric` function, because it contains special logic for blocking the request if the user hasn't opted in to telemetry. +### Disallowed characters + +The colon and comma characters (`,`, `:`) should not be used in app name or metric types. Colons play +a sepcial role in how metrics are stored as saved objects, and the API endpoint uses commas to delimit +multiple metric types in a single API request. + ### Tracking timed interactions If you want to track how long it takes a user to do something, you'll need to implement the timing @@ -66,4 +72,6 @@ These saved objects are automatically consumed by the stats API and surfaced und ``` By storing these metrics and their counts as key-value pairs, we can add more metrics without having -to worry about exceeding the 1000-field soft limit in Elasticsearch. \ No newline at end of file +to worry about exceeding the 1000-field soft limit in Elasticsearch. + +// TODO: Clarify that colons are NOT allowed in metric names or app types possibly reject request in trackUiMetric; disallow commas too? \ No newline at end of file diff --git a/src/legacy/core_plugins/ui_metric/public/index.ts b/src/legacy/core_plugins/ui_metric/public/index.ts index 38b98ffdfc9edb..85175521ae7cb6 100644 --- a/src/legacy/core_plugins/ui_metric/public/index.ts +++ b/src/legacy/core_plugins/ui_metric/public/index.ts @@ -29,11 +29,25 @@ uiModules.get('kibana').run(($http: any) => { _http = $http; }); +function createErrorMessage(subject) { + const message = `trackUiMetric was called with ${subject}, which is not allowed to contain a colon. ` + + `Colons play a special role in how metrics are saved as stored objects`; + return new Error(message); +} + export function trackUiMetric(appName: string, metricType: string | string[]) { if (!getCanTrackUiMetrics()) { return; } + if (appName.includes(':')) { + throw createErrorMessage(`app name '${appName}'`); + } + + if (metricType.includes(':')) { + throw createErrorMessage(`metric type ${metricType}`); + } + const metricTypes = Array.isArray(metricType) ? metricType.join(',') : metricType; const uri = chrome.addBasePath(`${API_BASE_PATH}/${appName}/${metricTypes}`); _http.post(uri); From 80ababd685c713890a066c6abea5e320012716fc Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 22 Apr 2019 14:25:07 -0700 Subject: [PATCH 10/11] Fix lint error. --- src/legacy/core_plugins/ui_metric/public/index.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/ui_metric/public/index.ts b/src/legacy/core_plugins/ui_metric/public/index.ts index 85175521ae7cb6..6bf54943a3ac40 100644 --- a/src/legacy/core_plugins/ui_metric/public/index.ts +++ b/src/legacy/core_plugins/ui_metric/public/index.ts @@ -29,8 +29,9 @@ uiModules.get('kibana').run(($http: any) => { _http = $http; }); -function createErrorMessage(subject) { - const message = `trackUiMetric was called with ${subject}, which is not allowed to contain a colon. ` + +function createErrorMessage(subject: string): any { + const message = + `trackUiMetric was called with ${subject}, which is not allowed to contain a colon. ` + `Colons play a special role in how metrics are saved as stored objects`; return new Error(message); } From 73fa5b1ccf3ed1990a4d649a0f0262bba9a9c38f Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 25 Apr 2019 14:01:32 -0700 Subject: [PATCH 11/11] Remove comment. --- src/legacy/core_plugins/ui_metric/README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/legacy/core_plugins/ui_metric/README.md b/src/legacy/core_plugins/ui_metric/README.md index 5035e3d458bddc..9b78cf600dc4e8 100644 --- a/src/legacy/core_plugins/ui_metric/README.md +++ b/src/legacy/core_plugins/ui_metric/README.md @@ -72,6 +72,4 @@ These saved objects are automatically consumed by the stats API and surfaced und ``` By storing these metrics and their counts as key-value pairs, we can add more metrics without having -to worry about exceeding the 1000-field soft limit in Elasticsearch. - -// TODO: Clarify that colons are NOT allowed in metric names or app types possibly reject request in trackUiMetric; disallow commas too? \ No newline at end of file +to worry about exceeding the 1000-field soft limit in Elasticsearch. \ No newline at end of file