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

[Maps] Enh/gis telemetry #29346

Merged
merged 22 commits into from
Feb 5, 2019
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4e3c22c
Add basic framework for maps telemetry gathering
kindsun Jan 18, 2019
3330518
Merge remote-tracking branch 'upstream/master' into enh/gisTelemetry
kindsun Jan 22, 2019
1e67ebc
Update mappings to cover required maps usage metrics
kindsun Jan 24, 2019
06a122a
Update telemetry to harvest from saved maps
kindsun Jan 24, 2019
aef2b52
Consolidate. Clean up
kindsun Jan 24, 2019
baaf22d
Merge remote-tracking branch 'upstream/master' into enh/gisTelemetry
kindsun Jan 28, 2019
475580b
Harvest telemetry daily at midnight using task manager. Retrieve task…
kindsun Jan 29, 2019
bb4e105
Organize, clean up
kindsun Jan 29, 2019
6d0f932
Reorg
kindsun Jan 29, 2019
7579bcd
Merge remote-tracking branch 'upstream/master' into enh/gisTelemetry
kindsun Jan 30, 2019
b0e2152
Add usage collector tests. Add test utils
kindsun Jan 31, 2019
9c01a21
Merge remote-tracking branch 'upstream/master' into enh/gisTelemetry
kindsun Jan 31, 2019
929907e
Add maps telemetry tests to verify values. Multiple structure changes…
kindsun Feb 1, 2019
f6c63d1
Merge remote-tracking branch 'upstream/master' into enh/gisTelemetry
kindsun Feb 1, 2019
e15e00e
Review feedback
kindsun Feb 1, 2019
cf97d4a
Merge remote-tracking branch 'upstream/master' into enh/gisTelemetry
kindsun Feb 4, 2019
0859ca4
More review feedback
kindsun Feb 4, 2019
fdd66bf
Add capture date/time to telemetry
kindsun Feb 4, 2019
d9ab073
Change date to 'type: date'
kindsun Feb 4, 2019
9a8fc37
Review feedback
kindsun Feb 5, 2019
20742b8
Merge remote-tracking branch 'upstream/master' into enh/gisTelemetry
kindsun Feb 5, 2019
7a1aab4
Fix merge issue
kindsun Feb 5, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 31 additions & 23 deletions x-pack/plugins/maps/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import mappings from './mappings.json';
import { checkLicense } from './check_license';
import { watchStatusAndLicenseToInitialize } from
'../../server/lib/watch_status_and_license_to_initialize';
import { initTelemetryCollection } from './server/maps_telemetry';

export function maps(kibana) {

return new kibana.Plugin({
require: ['kibana', 'elasticsearch', 'xpack_main', 'tile_map'],
require: ['kibana', 'elasticsearch', 'xpack_main', 'tile_map', 'task_manager'],
Copy link
Member

Choose a reason for hiding this comment

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

Hi, just to let you know, if someone needed to disable task manager, they would also have to disable the Maps plugin.

You could avoid that by putting the task stuff in x-pack/plugins/oss_telemetry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some offline conversation and some code digging, I've decided to leave this as is for now with the understanding that we may need to rethink this dependency in the future. There shouldn't be a timing issue as the plugin spec requires dependencies be both enabled and initialized (

* @param {Array} [opts.require] the other plugins that this plugin
) however there is the issue of disabling the task manager effectively disabling the map. It may be possible to do a conditional require, however the condition would usually be on xpack.task_manager.enabled which is obtained from the server config at init time, trying to get this info at the point of require feels hacky. This might be a bigger issue of how to elegantly handle conditional requires in a broader way moving forward and could likely be tackled in the plugin spec.

id: 'maps',
configPrefix: 'xpack.maps',
publicDir: resolve(__dirname, 'public'),
Expand All @@ -32,6 +33,11 @@ export function maps(kibana) {
],
home: ['plugins/maps/register_feature'],
styleSheetPaths: `${__dirname}/public/index.scss`,
savedObjectSchemas: {
'maps-telemetry': {
isNamespaceAgnostic: true
}
},
mappings
},
config(Joi) {
Expand All @@ -43,30 +49,32 @@ export function maps(kibana) {
init(server) {
const gisEnabled = server.config().get('xpack.maps.enabled');

if (gisEnabled) {
const thisPlugin = this;
const xpackMainPlugin = server.plugins.xpack_main;
let routesInitialized = false;

watchStatusAndLicenseToInitialize(xpackMainPlugin, thisPlugin,
async license => {
if (license && license.gis && !routesInitialized) {
routesInitialized = true;
initRoutes(server, license.uid);
}
});
if (!gisEnabled) {
server.log(['info', 'maps'], 'Maps app disabled by configuration');
return;
}

xpackMainPlugin.info
.feature(thisPlugin.id)
.registerLicenseCheckResultsGenerator(checkLicense);
const thisPlugin = this;
Copy link
Contributor

Choose a reason for hiding this comment

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

thisPlugin variable is not necessary

const xpackMainPlugin = server.plugins.xpack_main;
let routesInitialized = false;

server.addSavedObjectsToSampleDataset('logs', webLogsSavedObjects);
server.injectUiAppVars('maps', async () => {
return await server.getInjectedUiAppVars('kibana');
watchStatusAndLicenseToInitialize(xpackMainPlugin, thisPlugin,
async license => {
if (license && license.gis && !routesInitialized) {
routesInitialized = true;
initRoutes(server, license.uid);
initTelemetryCollection(server);
Copy link
Member

@tsullivan tsullivan Feb 1, 2019

Choose a reason for hiding this comment

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

It might not be necessary to keep initTelemetryCollection waiting for xpack_info license information, or the routesInitialized

As long as scheduling the built-in task waits for afterPluginsInit, which it does: https://github.com/elastic/kibana/pull/29346/files#diff-25f6c55fe3ca39b275e87048eb946ca8R16 I think you'd be able to call initTelemetryCollection directly from init

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just thinking there was no point in registering for maps telemetry if there are no maps. Just one less thing to poll for the telemetry service. Do you disagree?

Copy link
Member

Choose a reason for hiding this comment

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

The reasoning sounds fine. I'd be a little cautious about the timing of when your afterPluginsInit declaration. If that happens too late in the server init, the Kibana server may have already processed all the callbacks in its queue: https://github.com/elastic/kibana/blob/f0cc432/src/server/plugins/wait_for_plugins_init.js

Looks like there would be an error thrown if that happens, but to be safe, I'd put that in up-front setup logic

}
});
} else {
server.log(['info', 'maps'], 'Maps app disabled by configuration');
}

xpackMainPlugin.info
.feature(thisPlugin.id)
.registerLicenseCheckResultsGenerator(checkLicense);

server.addSavedObjectsToSampleDataset('logs', webLogsSavedObjects);
server.injectUiAppVars('maps', async () => {
return await server.getInjectedUiAppVars('kibana');
});
}
});
}
}
47 changes: 46 additions & 1 deletion x-pack/plugins/maps/mappings.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,50 @@
"type": "text"
}
}
},
"maps-telemetry": {
"properties": {
"mapsTotalCount": {
"type": "long"
},
"attributesPerMap": {
"properties": {
"dataSourcesCount": {
"properties": {
"min": {
"type": "long"
},
"max": {
"type": "long"
},
"avg": {
"type": "long"
}
}
},
"layersCount": {
"properties": {
"min": {
"type": "long"
},
"max": {
"type": "long"
},
"avg": {
"type": "long"
}
}
},
"layerTypesCount": {
"dynamic": "true",
"properties": {}
},
"emsVectorLayersCount": {
"dynamic": "true",
"properties": {}
}
}
}
}
}
}
}
7 changes: 7 additions & 0 deletions x-pack/plugins/maps/server/maps_telemetry/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* 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 { initTelemetryCollection } from './maps_usage_collector';
105 changes: 105 additions & 0 deletions x-pack/plugins/maps/server/maps_telemetry/maps_telemetry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* 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 _ from 'lodash';

function getSavedObjectsClient(server, callCluster) {
const { SavedObjectsClient, getSavedObjectsRepository } = server.savedObjects;
const internalRepository = getSavedObjectsRepository(callCluster);
return new SavedObjectsClient(internalRepository);
}

function getUniqueLayerCounts(layerCountsList, mapsCount) {
const uniqueLayerTypes = _.uniq(_.flatten(
layerCountsList.map(lTypes => Object.keys(lTypes))));

return uniqueLayerTypes.reduce((accu, type) => {
const typeCounts = layerCountsList.reduce((accu, tCounts) => {
tCounts[type] && accu.push(tCounts[type]);
return accu;
}, []);
const typeCountsSum = _.sum(typeCounts);
accu[type] = {
min: typeCounts.length ? _.min(typeCounts) : 0,
max: typeCounts.length ? _.max(typeCounts) : 0,
avg: typeCountsSum ? typeCountsSum / mapsCount : 0
};
return accu;
}, {});
}

export function buildMapsTelemetry(savedObjects) {
const savedMaps = savedObjects
.map(savedMapObject =>
JSON.parse(savedMapObject.attributes.layerListJSON));
const mapsCount = savedMaps.length;

const dataSourcesCount = savedMaps.map(lList => {
const sourceIdList = lList.map(layer => layer.sourceDescriptor.id);
return _.uniq(sourceIdList).length;
});

const layersCount = savedMaps.map(lList => lList.length);
const layerTypesCount = savedMaps.map(lList => _.countBy(lList, 'type'));

// Count of EMS Vector layers used
const emsLayersCount = savedMaps.map(lList => _(lList)
.countBy(layer => {
const isEmsFile = _.get(layer, 'sourceDescriptor.type') === 'EMS_FILE';
return isEmsFile && _.get(layer, 'sourceDescriptor.id');
})
.pick((val, key) => key !== 'false')
.value());

const dataSourcesCountSum = _.sum(dataSourcesCount);
const layersCountSum = _.sum(layersCount);
const mapsTelem = {
// Total count of maps
mapsTotalCount: mapsCount,
attributesPerMap: {
// Count of data sources per map
dataSourcesCount: {
min: dataSourcesCount.length ? _.min(dataSourcesCount) : 0,
max: dataSourcesCount.length ? _.max(dataSourcesCount) : 0,
avg: dataSourcesCountSum ? layersCountSum / mapsCount : 0
},
// Total count of layers per map
layersCount: {
min: layersCount.length ? _.min(layersCount) : 0,
max: layersCount.length ? _.max(layersCount) : 0,
avg: layersCountSum ? layersCountSum / mapsCount : 0
},
// Count of layers by type
layerTypesCount: {
...getUniqueLayerCounts(layerTypesCount, mapsCount)
},
// Count of layer by EMS region
emsVectorLayersCount: {
...getUniqueLayerCounts(emsLayersCount, mapsCount)
}
}
};
return mapsTelem;
}

async function getSavedObjects(savedObjectsClient) {
const gisMapsSavedObject = await savedObjectsClient.find({
type: 'gis-map'
Copy link
Contributor

Choose a reason for hiding this comment

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

needs to be map

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice catch!

});
return _.get(gisMapsSavedObject, 'saved_objects');
}

export async function getMapsTelemetry(server, callCluster) {
const savedObjectsClient = getSavedObjectsClient(server, callCluster);
const savedObjects = await getSavedObjects(savedObjectsClient);
const mapsTelemetry = buildMapsTelemetry(savedObjects);

return await savedObjectsClient.create('maps-telemetry',
mapsTelemetry, {
id: 'maps-telemetry',
overwrite: true,
});
}
88 changes: 88 additions & 0 deletions x-pack/plugins/maps/server/maps_telemetry/maps_telemetry.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* 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 * as savedObjectsPayload from
'./test_resources/sample_saved_objects.json';
import { buildMapsTelemetry } from './maps_telemetry';

describe('buildMapsTelemetry', () => {

test('returns zeroed telemetry data when there are no saved objects',
async () => {

const gisMaps = [];
const result = buildMapsTelemetry(gisMaps);

expect(result).toMatchObject({
"attributesPerMap": {
"dataSourcesCount": {
"avg": 0,
"max": 0,
"min": 0
},
"emsVectorLayersCount": {},
"layerTypesCount": {},
"layersCount": {
"avg": 0,
"max": 0,
"min": 0
}
},
"mapsTotalCount": 0
});
});

test('returns expected telemetry data from saved objects', async () => {

const gisMaps = savedObjectsPayload.saved_objects;
const result = buildMapsTelemetry(gisMaps);

expect(result).toMatchObject({
"attributesPerMap": {
"dataSourcesCount": {
"avg": 2.6666666666666665,
"max": 3,
"min": 2
},
"emsVectorLayersCount": {
"canada_provinces": {
"avg": 0.3333333333333333,
"max": 1,
"min": 1
},
"france_departments": {
"avg": 0.3333333333333333,
"max": 1,
"min": 1
},
"italy_provinces": {
"avg": 0.3333333333333333,
"max": 1,
"min": 1
}
},
"layerTypesCount": {
"TILE": {
"avg": 1,
"max": 1,
"min": 1
},
"VECTOR": {
"avg": 1.6666666666666667,
"max": 2,
"min": 1
}
},
"layersCount": {
"avg": 2.6666666666666665,
"max": 3,
"min": 2
}
},
"mapsTotalCount": 3
});
});
});
59 changes: 59 additions & 0 deletions x-pack/plugins/maps/server/maps_telemetry/maps_usage_collector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* 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 _ from 'lodash';
import { TASK_ID, scheduleTask, registerMapsTelemetryTask } from './telemetry_task';

export function initTelemetryCollection(server) {
const { taskManager } = server;

registerMapsTelemetryTask(taskManager);
scheduleTask(server, taskManager);
registerMapsUsageCollector(server);
}

export function buildCollectorObj(server) {
return {
type: 'maps',
fetch: async () => {
let docs;
try {
({ docs } = await server.taskManager.fetch({
query: {
bool: {
filter: {
term: {
_id: TASK_ID
Copy link
Member

Choose a reason for hiding this comment

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

taskManager really needs a get method :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that'd be nice 👍

}
}
}
}
}));
} catch (err) {
const errMessage = err && err.message ? err.message : err.toString();
/*
* The usage service WILL to try to fetch from this collector before the task manager has been initialized, because the task manager
* has to wait for all plugins to initialize first.
* It's fine to ignore it as next time around it will be initialized (or it will throw a different type of error)
*/
if (errMessage.indexOf('NotInitialized') >= 0) {
docs = {};
} else {
throw err;
}
}

return _.get(docs, '[0].state.stats');
},
};
}

export function registerMapsUsageCollector(server) {
const { usage } = server;
const collectorObj = buildCollectorObj(server);
const mapsUsageCollector = usage.collectorSet.makeUsageCollector(collectorObj);
usage.collectorSet.register(mapsUsageCollector);
}
Loading