Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block requests to ui_metric API if telemetry is disabled #35268

Merged
merged 12 commits into from
Apr 25, 2019
32 changes: 24 additions & 8 deletions src/legacy/core_plugins/ui_metric/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,29 @@ 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.

### 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

Expand All @@ -32,8 +47,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

Expand All @@ -58,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.
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?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this todo is no longer needed as you have already addressed it on line 37

20 changes: 20 additions & 0 deletions src/legacy/core_plugins/ui_metric/common/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,26 @@
* under the License.
*/

import { resolve } from 'path';
import { Legacy } from '../../../../kibana';
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: any) {
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) {
init(server: Legacy.Server) {
registerUserActionRoute(server);
registerUiMetricUsageCollector(server);
}
},
});
}
55 changes: 55 additions & 0 deletions src/legacy/core_plugins/ui_metric/public/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* 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';
// @ts-ignore
import { uiModules } from 'ui/modules';
import { getCanTrackUiMetrics } from 'ui/ui_metric';
import { API_BASE_PATH } from '../common';

let _http: any;

uiModules.get('kibana').run(($http: any) => {
_http = $http;
});

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

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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
28 changes: 28 additions & 0 deletions src/legacy/ui/public/ui_metric/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* 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 _canTrackUiMetrics = false;

export function setCanTrackUiMetrics(flag: boolean) {
_canTrackUiMetrics = flag;
}

export function getCanTrackUiMetrics(): boolean {
return _canTrackUiMetrics;
}
7 changes: 0 additions & 7 deletions x-pack/common/ui_metric/index.ts

This file was deleted.

11 changes: 0 additions & 11 deletions x-pack/common/ui_metric/ui_metric.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ jest.mock('ui/index_patterns', () => {
return { INDEX_PATTERN_ILLEGAL_CHARACTERS_VISIBLE };
});

jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({
trackUiMetric: jest.fn(),
}));

describe('<AutoFollowPatternList />', () => {
let server;
let find;
Expand All @@ -37,8 +41,7 @@ describe('<AutoFollowPatternList />', () => {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ jest.mock('ui/index_patterns', () => {
return { INDEX_PATTERN_ILLEGAL_CHARACTERS_VISIBLE };
});

jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({
trackUiMetric: jest.fn(),
}));

describe('<FollowerIndicesList />', () => {
let server;
let find;
Expand All @@ -34,8 +38,7 @@ describe('<FollowerIndicesList />', () => {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ jest.mock('ui/index_patterns', () => {
return { INDEX_PATTERN_ILLEGAL_CHARACTERS_VISIBLE };
});

jest.mock('../../../../../src/legacy/core_plugins/ui_metric/public', () => ({
trackUiMetric: jest.fn(),
}));

const testBedOptions = {
memoryRouter: {
initialEntries: [`${BASE_PATH}/follower_indices`],
Expand All @@ -49,8 +53,7 @@ describe('<CrossClusterReplicationHome />', () => {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { createUiMetricUri } from '../../../../../common/ui_metric';
import { trackUiMetric as 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);
}

/**
Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugins/index_lifecycle_management/public/services/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { get } from 'lodash';

import { createUiMetricUri } from '../../../../common/ui_metric';
import { trackUiMetric as track } from '../../../../../src/legacy/core_plugins/ui_metric/public';

import {
UIM_APP_NAME,
Expand All @@ -29,11 +29,8 @@ import {
defaultHotPhase,
} from '../store/defaults';

import { getHttpClient } from './api';

export function trackUiMetric(metricType, httpClient = getHttpClient()) {
const uiMetricUri = createUiMetricUri(UIM_APP_NAME, metricType);
httpClient.post(uiMetricUri);
export function trackUiMetric(metricType) {
track(UIM_APP_NAME, metricType);
}

export function getUiMetricsForPhases(phases) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
{
Expand Down
Loading