Skip to content

Commit

Permalink
[7.5] [Maps] Fix regression preventing maps telemetry from populating…
Browse files Browse the repository at this point in the history 
… & remove task manager logic (#52834) (#54055)

* [Maps] Fix regression preventing maps telemetry from populating & remove task manager logic (#52834)

* Remove task logic. Remove server refs and revise for np. Migrate a few files to ts

* Remove unused reference

* Update mappings

* Test usage collector register

* Update api integration tests to include maps now that telemetry is 'normal' (not using task mgr state)

* Update integration test to use stack stats

* Update integration test to look for 'maps-telemetry' instead of 'maps'

* Update jest test to reflect calls to register

* Follow the same pattern as other int tests and test reliable nested attribute

* Back out np-related changes for separate PR

* timeCaptured hasn't changed but for some reason stopped working. Getting iso string fixes issue

* Back out file shuffling for separate PR

* Remove mappings updates (handled in separate PR)

* Review feedback. Move telemetry type constant to constants file

* Consolidate imports

* Linting fix

Co-authored-by: Elastic Machine <[email protected]>

* Revise test for 7.5 (pre-NP changes) compat

* Linting

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
@elasticmachine
Aaron Caldwell and elasticmachine authored Jan 9, 2020
1 parent 195508d commit 9fb2d7f
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 320 deletions.
1 change: 1 addition & 0 deletions x-pack/legacy/plugins/maps/common/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const EMS_TILES_VECTOR_TILE_PATH = 'ems/tiles/vector/tile';
export const MAP_SAVED_OBJECT_TYPE = 'map';
export const APP_ID = 'maps';
export const APP_ICON = 'gisApp';
export const TELEMETRY_TYPE = 'maps-telemetry';

export const MAP_APP_PATH = `app/${APP_ID}`;
export const GIS_API_PATH = `api/${APP_ID}`;
Expand Down
11 changes: 6 additions & 5 deletions x-pack/legacy/plugins/maps/server/maps_telemetry/maps_telemetry.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
*/

import _ from 'lodash';
import { EMS_FILE, MAP_SAVED_OBJECT_TYPE } from '../../common/constants';
import { EMS_FILE, MAP_SAVED_OBJECT_TYPE, TELEMETRY_TYPE } from '../../common/constants';

function getSavedObjectsClient(server, callCluster) {
function getSavedObjectsClient(server) {
const { SavedObjectsClient, getSavedObjectsRepository } = server.savedObjects;
const callCluster = server.plugins.elasticsearch.getCluster('admin').callWithInternalUser;
const internalRepository = getSavedObjectsRepository(callCluster);
return new SavedObjectsClient(internalRepository);
}
Expand Down Expand Up @@ -62,7 +63,7 @@ export function buildMapsTelemetry(savedObjects) {
// Total count of maps
mapsTotalCount: mapsCount,
// Time of capture
timeCaptured: new Date(),
timeCaptured: new Date().toISOString(),
attributesPerMap: {
// Count of data sources per map
dataSourcesCount: {
Expand Down Expand Up @@ -101,8 +102,8 @@ export async function getMapsTelemetry(server, callCluster) {
const savedObjects = await getSavedObjects(savedObjectsClient);
const mapsTelemetry = buildMapsTelemetry(savedObjects);

return await savedObjectsClient.create('maps-telemetry', mapsTelemetry, {
id: 'maps-telemetry',
return await savedObjectsClient.create(TELEMETRY_TYPE, mapsTelemetry, {
id: TELEMETRY_TYPE,
overwrite: true,
});
}
84 changes: 7 additions & 77 deletions x-pack/legacy/plugins/maps/server/maps_telemetry/maps_usage_collector.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,85 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/

import _ from 'lodash';
import { TASK_ID, scheduleTask, registerMapsTelemetryTask } from './telemetry_task';
import { getMapsTelemetry } from './maps_telemetry';
import { TELEMETRY_TYPE } from '../../common/constants';

export function initTelemetryCollection(server) {
registerMapsTelemetryTask(server);
scheduleTask(server);
registerMapsUsageCollector(server);
}

async function isTaskManagerReady(server) {
const result = await fetch(server);
return result !== null;
}

async function fetch(server) {
let docs;
const taskManager = server.plugins.task_manager;

if (!taskManager) {
return null;
}

try {
({ docs } = await taskManager.fetch({
query: {
bool: {
filter: {
term: {
_id: `task:${TASK_ID}`,
},
},
},
},
}));
} 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) {
return null;
} else {
throw err;
}
}

return docs;
}

export function buildCollectorObj(server) {
let isCollectorReady = false;
async function determineIfTaskManagerIsReady() {
let isReady = false;
try {
isReady = await isTaskManagerReady(server);
} catch (err) {} // eslint-disable-line

if (isReady) {
isCollectorReady = true;
} else {
setTimeout(determineIfTaskManagerIsReady, 500);
}
}
determineIfTaskManagerIsReady();

return {
type: 'maps',
isReady: () => isCollectorReady,
fetch: async () => {
const docs = await fetch(server);
return _.get(docs, '[0].state.stats');
},
};
}
const mapsUsageCollector = server.usage.collectorSet.makeUsageCollector({
type: TELEMETRY_TYPE,
isReady: () => true,
fetch: async () => await getMapsTelemetry(server),
});

export function registerMapsUsageCollector(server) {
const collectorObj = buildCollectorObj(server);
const mapsUsageCollector = server.usage.collectorSet.makeUsageCollector(collectorObj);
server.usage.collectorSet.register(mapsUsageCollector);
}
65 changes: 20 additions & 45 deletions x-pack/legacy/plugins/maps/server/maps_telemetry/maps_usage_collector.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,60 +4,35 @@
* you may not use this file except in compliance with the Elastic License.
*/

import sinon from 'sinon';
import { getMockCallWithInternal, getMockKbnServer, getMockTaskFetch } from '../test_utils';
import { buildCollectorObj } from './maps_usage_collector';
import { initTelemetryCollection } from './maps_usage_collector';

describe('buildCollectorObj#fetch', () => {
let mockKbnServer;
let server;
let makeUsageCollectorStub;
let registerStub;

beforeEach(() => {
mockKbnServer = getMockKbnServer();
});

test('can return empty stats', async () => {
const { type, fetch } = buildCollectorObj(mockKbnServer);
expect(type).toBe('maps');
const fetchResult = await fetch();
expect(fetchResult).toEqual({});
});

test('provides known stats', async () => {
const mockTaskFetch = getMockTaskFetch([
{
state: {
runs: 2,
stats: { wombat_sightings: { total: 712, max: 84, min: 7, avg: 63 } },
makeUsageCollectorStub = jest.fn();
registerStub = jest.fn();
server = {
usage: {
collectorSet: {
makeUsageCollector: makeUsageCollectorStub,
register: registerStub,
},
},
]);
mockKbnServer = getMockKbnServer(getMockCallWithInternal(), mockTaskFetch);

const { type, fetch } = buildCollectorObj(mockKbnServer);
expect(type).toBe('maps');
const fetchResult = await fetch();
expect(fetchResult).toEqual({ wombat_sightings: { total: 712, max: 84, min: 7, avg: 63 } });
};
});

describe('Error handling', () => {
test('Silently handles Task Manager NotInitialized', async () => {
const mockTaskFetch = sinon.stub();
mockTaskFetch.rejects(
new Error('NotInitialized taskManager is still waiting for plugins to load')
);
mockKbnServer = getMockKbnServer(getMockCallWithInternal(), mockTaskFetch);

const { fetch } = buildCollectorObj(mockKbnServer);
await expect(fetch()).resolves.toBe(undefined);
});
// In real life, the CollectorSet calls fetch and handles errors
test('defers the errors', async () => {
const mockTaskFetch = sinon.stub();
mockTaskFetch.rejects(new Error('Sad violin'));
mockKbnServer = getMockKbnServer(getMockCallWithInternal(), mockTaskFetch);
test('makes and registers maps usage collector', async () => {
initTelemetryCollection(server);

const { fetch } = buildCollectorObj(mockKbnServer);
await expect(fetch()).rejects.toMatchObject(new Error('Sad violin'));
expect(registerStub).toHaveBeenCalledTimes(1);
expect(makeUsageCollectorStub).toHaveBeenCalledTimes(1);
expect(makeUsageCollectorStub).toHaveBeenCalledWith({
type: expect.any(String),
isReady: expect.any(Function),
fetch: expect.any(Function),
});
});
});
125 changes: 0 additions & 125 deletions x-pack/legacy/plugins/maps/server/maps_telemetry/telemetry_task.js
View file

This file was deleted.

Loading

0 comments on commit 9fb2d7f

Please sign in to comment.