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

[Spaces] - basic telemetry #20581

Merged
merged 12 commits into from
Sep 4, 2018
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class BulkUploader {
throw new Error('interval number of milliseconds is required');
}

this._timer = null;
this._timer = null;
this._interval = interval;
this._log = {
debug: message => server.log(['debug', ...LOGGING_TAGS], message),
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/spaces/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,9 @@ export const SPACE_SEARCH_COUNT_THRESHOLD = 8;
* The maximum number of characters allowed in the Space Avatar's initials
*/
export const MAX_SPACE_INITIALS = 2;

/**
* The type name used within the Monitoring index to publish spaces stats.
* @type {string}
*/
export const KIBANA_SPACES_MONITORING_TYPE = 'spaces';
Copy link
Member

Choose a reason for hiding this comment

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

This string isn't just for monitoring, it's used as the property for spaces usage in the "stats" API. To get through the idea that telemetry isn't a Monitoring feature, I think it would be good to simplify this const to just KIBANA_SPACES_TYPE

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you think about KIBANA_SPACES_STATS_TYPE? KIBANA_SPACES_TYPE feels a bit too generic to me, especially when being imported from a "common constants" file, where there isn't any context.

6 changes: 6 additions & 0 deletions x-pack/plugins/spaces/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { initSpacesRequestInterceptors } from './server/lib/space_request_interc
import { createDefaultSpace } from './server/lib/create_default_space';
import { createSpacesService } from './server/lib/create_spaces_service';
import { getActiveSpace } from './server/lib/get_active_space';
import { getSpacesUsageCollector } from './server/lib/get_spaces_usage_collector';
import { wrapError } from './server/lib/errors';
import mappings from './mappings.json';
import { spacesSavedObjectsClientWrapperFactory } from './server/lib/saved_objects_client/saved_objects_client_wrapper_factory';
Expand Down Expand Up @@ -90,5 +91,10 @@ export const spaces = (kibana) => new kibana.Plugin({
initSpacesApi(server);

initSpacesRequestInterceptors(server);

// Register a function with server to manage the collection of usage stats
server.usage.collectorSet.register(getSpacesUsageCollector(server));

await createDefaultSpace(server);
Copy link
Member

Choose a reason for hiding this comment

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

not sure how this line is part of the telemetry additions

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, good catch! Will remove

}
});
75 changes: 75 additions & 0 deletions x-pack/plugins/spaces/server/lib/get_spaces_usage_collector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* 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 { KIBANA_SPACES_MONITORING_TYPE } from '../../common/constants';
import { KIBANA_STATS_TYPE_MONITORING } from '../../../monitoring/common/constants';

/**
*
* @param callCluster
* @param server
* @param {boolean} spacesAvailable
* @param withinDayRange
* @return {ReportingUsageStats}
*/
async function getSpacesUsage(callCluster, server, spacesAvailable) {
if (!spacesAvailable) return {};

const { getSavedObjectsRepository } = server.savedObjects;

const savedObjectsRepository = getSavedObjectsRepository(callCluster);

const { saved_objects: spaces } = await savedObjectsRepository.find({ type: 'space' });

return {
count: spaces.length,
};
}

/*
* @param {Object} server
* @return {Object} kibana usage stats type collection object
*/
export function getSpacesUsageCollector(server) {
const { collectorSet } = server.usage;
return collectorSet.makeUsageCollector({
type: KIBANA_SPACES_MONITORING_TYPE,
fetch: async callCluster => {
const xpackInfo = server.plugins.xpack_main.info;
const config = server.config();
const available = xpackInfo && xpackInfo.isAvailable(); // some form of spaces is available for all valid licenses
const enabled = config.get('xpack.spaces.enabled');
const spacesAvailableAndEnabled = available && enabled;

const usageStats = await getSpacesUsage(callCluster, server, spacesAvailableAndEnabled);

return {
available,
enabled: spacesAvailableAndEnabled, // similar behavior as _xpack API in ES
...usageStats,
};
},

/*
* Format the response data into a model for internal upload
* 1. Make this data part of the "kibana_stats" type
* 2. Organize the payload in the usage.xpack.spaces namespace of the data payload
*/
formatForBulkUpload: result => {
return {
type: KIBANA_STATS_TYPE_MONITORING,
payload: {
usage: {
xpacl: {
Copy link
Member

Choose a reason for hiding this comment

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

xpack

spaces: result
}
}
}
};

}
});
}
149 changes: 149 additions & 0 deletions x-pack/plugins/spaces/server/lib/get_spaces_usage_collector.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
/*
* 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 { getSpacesUsageCollector } from './get_spaces_usage_collector';

function getServerMock(customization) {
class MockUsageCollector {
constructor(_server, { fetch }) {
this.fetch = fetch;
}
}

const getLicenseCheckResults = jest.fn().mockReturnValue({});
const defaultServerMock = {
plugins: {
security: {
isAuthenticated: jest.fn().mockReturnValue(true)
},
xpack_main: {
info: {
isAvailable: jest.fn().mockReturnValue(true),
feature: () => ({
getLicenseCheckResults
}),
license: {
isOneOf: jest.fn().mockReturnValue(false),
getType: jest.fn().mockReturnValue('platinum'),
},
toJSON: () => ({ b: 1 })
}
}
},
expose: () => { },
log: () => { },
config: () => ({
get: key => {
if (key === 'xpack.spaces.enabled') {
return true;
}
}
}),
usage: {
collectorSet: {
makeUsageCollector: options => {
return new MockUsageCollector(this, options);
}
}
},
savedObjects: {
getSavedObjectsRepository: jest.fn(() => {
return {
find() {
return {
saved_objects: ['a', 'b']
};
}
};
})
}
};
return Object.assign(defaultServerMock, customization);
}

test('sets enabled to false when spaces is turned off', async () => {
const mockConfigGet = jest.fn(key => {
if (key === 'xpack.spaces.enabled') {
return false;
} else if (key.indexOf('xpack.spaces') >= 0) {
throw new Error('Unknown config key!');
}
});
const serverMock = getServerMock({ config: () => ({ get: mockConfigGet }) });
const callClusterMock = jest.fn();
const { fetch: getSpacesUsage } = getSpacesUsageCollector(serverMock);
const usageStats = await getSpacesUsage(callClusterMock);
expect(usageStats.enabled).toBe(false);
});

describe('with a basic license', async () => {
let usageStats;
beforeAll(async () => {
const serverWithBasicLicenseMock = getServerMock();
serverWithBasicLicenseMock.plugins.xpack_main.info.license.getType = jest.fn().mockReturnValue('basic');
const callClusterMock = jest.fn(() => Promise.resolve({}));
const { fetch: getSpacesUsage } = getSpacesUsageCollector(serverWithBasicLicenseMock);
usageStats = await getSpacesUsage(callClusterMock);
});

test('sets enabled to true', async () => {
expect(usageStats.enabled).toBe(true);
});

test('sets available to true', async () => {
expect(usageStats.available).toBe(true);
});

test('sets the number of spaces', async () => {
expect(usageStats.count).toBe(2);
});
});

describe('with no license', async () => {
let usageStats;
beforeAll(async () => {
const serverWithNoLicenseMock = getServerMock();
serverWithNoLicenseMock.plugins.xpack_main.info.isAvailable = jest.fn().mockReturnValue(false);
const callClusterMock = jest.fn(() => Promise.resolve({}));
const { fetch: getSpacesUsage } = getSpacesUsageCollector(serverWithNoLicenseMock);
usageStats = await getSpacesUsage(callClusterMock);
});

test('sets enabled to false', async () => {
expect(usageStats.enabled).toBe(false);
});

test('sets available to false', async () => {
expect(usageStats.available).toBe(false);
});

test('does not set the number of spaces', async () => {
expect(usageStats.count).toBeUndefined();
});
});

describe('with platinum license', async () => {
let usageStats;
beforeAll(async () => {
const serverWithPlatinumLicenseMock = getServerMock();
serverWithPlatinumLicenseMock.plugins.xpack_main.info.license.getType = jest.fn().mockReturnValue('platinum');
const callClusterMock = jest.fn(() => Promise.resolve({}));
const { fetch: getSpacesUsage } = getSpacesUsageCollector(serverWithPlatinumLicenseMock);
usageStats = await getSpacesUsage(callClusterMock);
});

test('sets enabled to true', async () => {
expect(usageStats.enabled).toBe(true);
});

test('sets available to true', async () => {
expect(usageStats.available).toBe(true);
});

test('sets the number of spaces', async () => {
expect(usageStats.count).toBe(2);
});
});