Skip to content

Commit 9fb2d7f

Browse files
Aaron Caldwellelasticmachine
andauthored
[7.5] [Maps] Fix regression preventing maps telemetry from populating & 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 <elasticmachine@users.noreply.github.com> * Revise test for 7.5 (pre-NP changes) compat * Linting Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent 195508d commit 9fb2d7f

File tree

7 files changed

+38
-320
lines changed

7 files changed

+38
-320
lines changed

x-pack/legacy/plugins/maps/common/constants.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export const EMS_TILES_VECTOR_TILE_PATH = 'ems/tiles/vector/tile';
2222
export const MAP_SAVED_OBJECT_TYPE = 'map';
2323
export const APP_ID = 'maps';
2424
export const APP_ICON = 'gisApp';
25+
export const TELEMETRY_TYPE = 'maps-telemetry';
2526

2627
export const MAP_APP_PATH = `app/${APP_ID}`;
2728
export const GIS_API_PATH = `api/${APP_ID}`;

x-pack/legacy/plugins/maps/server/maps_telemetry/maps_telemetry.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55
*/
66

77
import _ from 'lodash';
8-
import { EMS_FILE, MAP_SAVED_OBJECT_TYPE } from '../../common/constants';
8+
import { EMS_FILE, MAP_SAVED_OBJECT_TYPE, TELEMETRY_TYPE } from '../../common/constants';
99

10-
function getSavedObjectsClient(server, callCluster) {
10+
function getSavedObjectsClient(server) {
1111
const { SavedObjectsClient, getSavedObjectsRepository } = server.savedObjects;
12+
const callCluster = server.plugins.elasticsearch.getCluster('admin').callWithInternalUser;
1213
const internalRepository = getSavedObjectsRepository(callCluster);
1314
return new SavedObjectsClient(internalRepository);
1415
}
@@ -62,7 +63,7 @@ export function buildMapsTelemetry(savedObjects) {
6263
// Total count of maps
6364
mapsTotalCount: mapsCount,
6465
// Time of capture
65-
timeCaptured: new Date(),
66+
timeCaptured: new Date().toISOString(),
6667
attributesPerMap: {
6768
// Count of data sources per map
6869
dataSourcesCount: {
@@ -101,8 +102,8 @@ export async function getMapsTelemetry(server, callCluster) {
101102
const savedObjects = await getSavedObjects(savedObjectsClient);
102103
const mapsTelemetry = buildMapsTelemetry(savedObjects);
103104

104-
return await savedObjectsClient.create('maps-telemetry', mapsTelemetry, {
105-
id: 'maps-telemetry',
105+
return await savedObjectsClient.create(TELEMETRY_TYPE, mapsTelemetry, {
106+
id: TELEMETRY_TYPE,
106107
overwrite: true,
107108
});
108109
}

x-pack/legacy/plugins/maps/server/maps_telemetry/maps_usage_collector.js

Lines changed: 7 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -4,85 +4,15 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

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

1010
export function initTelemetryCollection(server) {
11-
registerMapsTelemetryTask(server);
12-
scheduleTask(server);
13-
registerMapsUsageCollector(server);
14-
}
15-
16-
async function isTaskManagerReady(server) {
17-
const result = await fetch(server);
18-
return result !== null;
19-
}
20-
21-
async function fetch(server) {
22-
let docs;
23-
const taskManager = server.plugins.task_manager;
24-
25-
if (!taskManager) {
26-
return null;
27-
}
28-
29-
try {
30-
({ docs } = await taskManager.fetch({
31-
query: {
32-
bool: {
33-
filter: {
34-
term: {
35-
_id: `task:${TASK_ID}`,
36-
},
37-
},
38-
},
39-
},
40-
}));
41-
} catch (err) {
42-
const errMessage = err && err.message ? err.message : err.toString();
43-
/*
44-
* The usage service WILL to try to fetch from this collector before the task manager has been initialized, because the task manager
45-
* has to wait for all plugins to initialize first.
46-
* It's fine to ignore it as next time around it will be initialized (or it will throw a different type of error)
47-
*/
48-
if (errMessage.indexOf('NotInitialized') >= 0) {
49-
return null;
50-
} else {
51-
throw err;
52-
}
53-
}
54-
55-
return docs;
56-
}
57-
58-
export function buildCollectorObj(server) {
59-
let isCollectorReady = false;
60-
async function determineIfTaskManagerIsReady() {
61-
let isReady = false;
62-
try {
63-
isReady = await isTaskManagerReady(server);
64-
} catch (err) {} // eslint-disable-line
65-
66-
if (isReady) {
67-
isCollectorReady = true;
68-
} else {
69-
setTimeout(determineIfTaskManagerIsReady, 500);
70-
}
71-
}
72-
determineIfTaskManagerIsReady();
73-
74-
return {
75-
type: 'maps',
76-
isReady: () => isCollectorReady,
77-
fetch: async () => {
78-
const docs = await fetch(server);
79-
return _.get(docs, '[0].state.stats');
80-
},
81-
};
82-
}
11+
const mapsUsageCollector = server.usage.collectorSet.makeUsageCollector({
12+
type: TELEMETRY_TYPE,
13+
isReady: () => true,
14+
fetch: async () => await getMapsTelemetry(server),
15+
});
8316

84-
export function registerMapsUsageCollector(server) {
85-
const collectorObj = buildCollectorObj(server);
86-
const mapsUsageCollector = server.usage.collectorSet.makeUsageCollector(collectorObj);
8717
server.usage.collectorSet.register(mapsUsageCollector);
8818
}

x-pack/legacy/plugins/maps/server/maps_telemetry/maps_usage_collector.test.js

Lines changed: 20 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -4,60 +4,35 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import sinon from 'sinon';
8-
import { getMockCallWithInternal, getMockKbnServer, getMockTaskFetch } from '../test_utils';
9-
import { buildCollectorObj } from './maps_usage_collector';
7+
import { initTelemetryCollection } from './maps_usage_collector';
108

119
describe('buildCollectorObj#fetch', () => {
12-
let mockKbnServer;
10+
let server;
11+
let makeUsageCollectorStub;
12+
let registerStub;
1313

1414
beforeEach(() => {
15-
mockKbnServer = getMockKbnServer();
16-
});
17-
18-
test('can return empty stats', async () => {
19-
const { type, fetch } = buildCollectorObj(mockKbnServer);
20-
expect(type).toBe('maps');
21-
const fetchResult = await fetch();
22-
expect(fetchResult).toEqual({});
23-
});
24-
25-
test('provides known stats', async () => {
26-
const mockTaskFetch = getMockTaskFetch([
27-
{
28-
state: {
29-
runs: 2,
30-
stats: { wombat_sightings: { total: 712, max: 84, min: 7, avg: 63 } },
15+
makeUsageCollectorStub = jest.fn();
16+
registerStub = jest.fn();
17+
server = {
18+
usage: {
19+
collectorSet: {
20+
makeUsageCollector: makeUsageCollectorStub,
21+
register: registerStub,
3122
},
3223
},
33-
]);
34-
mockKbnServer = getMockKbnServer(getMockCallWithInternal(), mockTaskFetch);
35-
36-
const { type, fetch } = buildCollectorObj(mockKbnServer);
37-
expect(type).toBe('maps');
38-
const fetchResult = await fetch();
39-
expect(fetchResult).toEqual({ wombat_sightings: { total: 712, max: 84, min: 7, avg: 63 } });
24+
};
4025
});
4126

42-
describe('Error handling', () => {
43-
test('Silently handles Task Manager NotInitialized', async () => {
44-
const mockTaskFetch = sinon.stub();
45-
mockTaskFetch.rejects(
46-
new Error('NotInitialized taskManager is still waiting for plugins to load')
47-
);
48-
mockKbnServer = getMockKbnServer(getMockCallWithInternal(), mockTaskFetch);
49-
50-
const { fetch } = buildCollectorObj(mockKbnServer);
51-
await expect(fetch()).resolves.toBe(undefined);
52-
});
53-
// In real life, the CollectorSet calls fetch and handles errors
54-
test('defers the errors', async () => {
55-
const mockTaskFetch = sinon.stub();
56-
mockTaskFetch.rejects(new Error('Sad violin'));
57-
mockKbnServer = getMockKbnServer(getMockCallWithInternal(), mockTaskFetch);
27+
test('makes and registers maps usage collector', async () => {
28+
initTelemetryCollection(server);
5829

59-
const { fetch } = buildCollectorObj(mockKbnServer);
60-
await expect(fetch()).rejects.toMatchObject(new Error('Sad violin'));
30+
expect(registerStub).toHaveBeenCalledTimes(1);
31+
expect(makeUsageCollectorStub).toHaveBeenCalledTimes(1);
32+
expect(makeUsageCollectorStub).toHaveBeenCalledWith({
33+
type: expect.any(String),
34+
isReady: expect.any(Function),
35+
fetch: expect.any(Function),
6136
});
6237
});
6338
});

x-pack/legacy/plugins/maps/server/maps_telemetry/telemetry_task.js

Lines changed: 0 additions & 125 deletions
This file was deleted.

0 commit comments

Comments
 (0)