Skip to content

Commit

Permalink
[Monitoring] Fix bug where kibana crashes with collection disabled (e…
Browse files Browse the repository at this point in the history
…lastic#75335)

* Fix bug where kibana crashes with collection disabled

* Do not set this to an empty object by default

* Always create the bulk uploader, and fix this misplaced license check

* Fix type issue

* Add a couple basic tests

* Fix type issue
  • Loading branch information
chrisronline authored Aug 20, 2020
1 parent 020a76b commit 3fb639f
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 26 deletions.
11 changes: 5 additions & 6 deletions x-pack/plugins/monitoring/server/license_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,13 @@ export class LicenseService {
let rawLicense: Readonly<ILicense> | undefined;
let licenseSubscription: Subscription | undefined = license$.subscribe((nextRawLicense) => {
rawLicense = nextRawLicense;
if (!rawLicense?.isAvailable) {
log.warn(
`X-Pack Monitoring Cluster Alerts will not be available: ${rawLicense?.getUnavailableReason()}`
);
}
});

if (!rawLicense?.isAvailable) {
log.warn(
`X-Pack Monitoring Cluster Alerts will not be available: ${rawLicense?.getUnavailableReason()}`
);
}

return {
refresh,
license$,
Expand Down
140 changes: 140 additions & 0 deletions x-pack/plugins/monitoring/server/plugin.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
* 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 { Plugin } from './plugin';
import { combineLatest } from 'rxjs';
// @ts-ignore
import { initBulkUploader } from './kibana_monitoring';
import { AlertsFactory } from './alerts';

jest.mock('rxjs', () => ({
// @ts-ignore
...jest.requireActual('rxjs'),
combineLatest: jest.fn(),
}));

jest.mock('./es_client/instantiate_client', () => ({
instantiateClient: jest.fn(),
}));

jest.mock('./license_service', () => ({
LicenseService: jest.fn().mockImplementation(() => ({
setup: jest.fn().mockImplementation(() => ({
refresh: jest.fn(),
})),
})),
}));

jest.mock('./kibana_monitoring', () => ({
initBulkUploader: jest.fn(),
}));

describe('Monitoring plugin', () => {
const initializerContext = {
logger: {
get: jest.fn().mockImplementation(() => ({
info: jest.fn(),
})),
},
config: {
create: jest.fn().mockImplementation(() => ({
pipe: jest.fn().mockImplementation(() => ({
toPromise: jest.fn(),
})),
})),
legacy: {
globalConfig$: {},
},
},
env: {
packageInfo: {
version: '1.0.0',
},
},
};

const coreSetup = {
http: {
createRouter: jest.fn(),
getServerInfo: jest.fn().mockImplementation(() => ({
port: 5601,
})),
basePath: {
serverBasePath: '',
},
},
uuid: {
getInstanceUuid: jest.fn(),
},
elasticsearch: {
legacy: {
client: {},
createClient: jest.fn(),
},
},
};

const setupPlugins = {
usageCollection: {
getCollectorByType: jest.fn(),
makeStatsCollector: jest.fn(),
registerCollector: jest.fn(),
},
alerts: {
registerType: jest.fn(),
},
};

let config = {};
const defaultConfig = {
ui: {
elasticsearch: {},
},
kibana: {
collection: {
interval: 30000,
},
},
};

beforeEach(() => {
config = defaultConfig;
(combineLatest as jest.Mock).mockImplementation(() => {
return {
pipe: jest.fn().mockImplementation(() => {
return {
toPromise: jest.fn().mockImplementation(() => {
return [config, 2];
}),
};
}),
};
});
});

afterEach(() => {
(setupPlugins.alerts.registerType as jest.Mock).mockReset();
});

it('always create the bulk uploader', async () => {
const setKibanaStatusGetter = jest.fn();
(initBulkUploader as jest.Mock).mockImplementation(() => {
return {
setKibanaStatusGetter,
};
});
const plugin = new Plugin(initializerContext as any);
const contract = await plugin.setup(coreSetup as any, setupPlugins as any);
contract.registerLegacyAPI(null as any);
expect(setKibanaStatusGetter).toHaveBeenCalled();
});

it('should register all alerts', async () => {
const alerts = AlertsFactory.getAll();
const plugin = new Plugin(initializerContext as any);
await plugin.setup(coreSetup as any, setupPlugins as any);
expect(setupPlugins.alerts.registerType).toHaveBeenCalledTimes(alerts.length);
});
});
44 changes: 24 additions & 20 deletions x-pack/plugins/monitoring/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class Plugin {
private licenseService = {} as MonitoringLicenseService;
private monitoringCore = {} as MonitoringCore;
private legacyShimDependencies = {} as LegacyShimDependencies;
private bulkUploader = {} as IBulkUploader;
private bulkUploader: IBulkUploader = {} as IBulkUploader;

constructor(initializerContext: PluginInitializerContext) {
this.initializerContext = initializerContext;
Expand Down Expand Up @@ -152,28 +152,28 @@ export class Plugin {
registerCollectors(plugins.usageCollection, config);
}

// If collection is enabled, create the bulk uploader
// Always create the bulk uploader
const kibanaMonitoringLog = this.getLogger(KIBANA_MONITORING_LOGGING_TAG);
const bulkUploader = (this.bulkUploader = initBulkUploader({
elasticsearch: core.elasticsearch,
config,
log: kibanaMonitoringLog,
kibanaStats: {
uuid: core.uuid.getInstanceUuid(),
name: serverInfo.name,
index: get(legacyConfig, 'kibana.index'),
host: serverInfo.hostname,
locale: i18n.getLocale(),
port: serverInfo.port.toString(),
transport_address: `${serverInfo.hostname}:${serverInfo.port}`,
version: this.initializerContext.env.packageInfo.version,
snapshot: snapshotRegex.test(this.initializerContext.env.packageInfo.version),
},
}));

// If collection is enabled, start it
const kibanaCollectionEnabled = config.kibana.collection.enabled;
if (kibanaCollectionEnabled) {
// Start kibana internal collection
const bulkUploader = (this.bulkUploader = initBulkUploader({
elasticsearch: core.elasticsearch,
config,
log: kibanaMonitoringLog,
kibanaStats: {
uuid: core.uuid.getInstanceUuid(),
name: serverInfo.name,
index: get(legacyConfig, 'kibana.index'),
host: serverInfo.hostname,
locale: i18n.getLocale(),
port: serverInfo.port.toString(),
transport_address: `${serverInfo.hostname}:${serverInfo.port}`,
version: this.initializerContext.env.packageInfo.version,
snapshot: snapshotRegex.test(this.initializerContext.env.packageInfo.version),
},
}));

// Do not use `this.licenseService` as that looks at the monitoring cluster
// whereas we want to check the production cluster here
if (plugins.licensing) {
Expand All @@ -188,6 +188,10 @@ export class Plugin {
bulkUploader.handleNotEnabled();
}
});
} else {
kibanaMonitoringLog.warn(
'Internal collection for Kibana monitoring is disabled due to missing license information.'
);
}
} else {
kibanaMonitoringLog.info(
Expand Down

0 comments on commit 3fb639f

Please sign in to comment.