From 6e6e6aeb92935ab7254f03261215d97869ca5ef6 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 8 May 2018 14:33:04 -0700 Subject: [PATCH 1/2] [Monitoring/Telemetry collection] use `typeCollector.registerType` in Reporting --- x-pack/plugins/monitoring/common/constants.js | 5 -- .../get_reporting_usage_collector.js | 44 ------------ .../get_reporting_usage_collector.js | 18 ----- .../server/kibana_monitoring/index.js | 1 - .../lib/get_collector_types_combiner.js | 4 +- .../kibana_monitoring/start_collector_set.js | 2 - x-pack/plugins/reporting/common/constants.js | 8 ++- x-pack/plugins/reporting/index.js | 15 +++- ...ge.js => get_reporting_usage_collector.js} | 72 +++++++++++-------- ... => get_reporting_usage_collector.test.js} | 14 ++-- .../plugins/reporting/server/usage/index.js | 2 +- .../server/routes/api/v1/kibana_stats.js | 7 +- 12 files changed, 78 insertions(+), 114 deletions(-) delete mode 100644 x-pack/plugins/monitoring/server/kibana_monitoring/collectors/__tests__/get_reporting_usage_collector.js delete mode 100644 x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_reporting_usage_collector.js rename x-pack/plugins/reporting/server/usage/{get_usage.js => get_reporting_usage_collector.js} (70%) rename x-pack/plugins/reporting/server/usage/{get_usage.test.js => get_reporting_usage_collector.test.js} (83%) diff --git a/x-pack/plugins/monitoring/common/constants.js b/x-pack/plugins/monitoring/common/constants.js index 6b015234ce63c..e1f1806026b1a 100644 --- a/x-pack/plugins/monitoring/common/constants.js +++ b/x-pack/plugins/monitoring/common/constants.js @@ -33,11 +33,6 @@ export const KIBANA_SETTINGS_TYPE = 'kibana_settings'; * @type {string} */ export const KIBANA_USAGE_TYPE = 'kibana'; -/** - * The type name used within the Monitoring index to publish reporting stats. - * @type {string} - */ -export const KIBANA_REPORTING_TYPE = 'reporting_stats'; /* * Key for the localStorage service diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/__tests__/get_reporting_usage_collector.js b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/__tests__/get_reporting_usage_collector.js deleted file mode 100644 index d4ba6331712de..0000000000000 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/__tests__/get_reporting_usage_collector.js +++ /dev/null @@ -1,44 +0,0 @@ -/* - * 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 expect from 'expect.js'; -import sinon from 'sinon'; -import { getReportingUsageCollector } from '../get_reporting_usage_collector'; -import { callClusterFactory } from '../../../../../xpack_main'; - -describe('getReportingUsageCollector', () => { - let clusterStub; - let serverStub; - let callClusterStub; - - beforeEach(() => { - clusterStub = { callWithInternalUser: sinon.stub().returns(Promise.resolve({})) }; - serverStub = { - plugins: { - elasticsearch: { getCluster: sinon.stub() }, - xpack_main: { - info: { - license: { getType: sinon.stub() }, - isAvailable() { return true; } - } - } - }, - config: () => ({ get: sinon.stub() }), - expose: sinon.stub(), - log: sinon.stub(), - }; - - serverStub.plugins.elasticsearch.getCluster.withArgs('admin').returns(clusterStub); - callClusterStub = callClusterFactory(serverStub).getCallClusterInternal(); - }); - - it('correctly defines reporting collector.', () => { - const reportingCollector = getReportingUsageCollector(serverStub, callClusterStub); - - expect(reportingCollector.type).to.be('reporting_stats'); - expect(reportingCollector.fetch).to.be.a(Function); - }); -}); diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_reporting_usage_collector.js b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_reporting_usage_collector.js deleted file mode 100644 index 7b9fd333a3d9c..0000000000000 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_reporting_usage_collector.js +++ /dev/null @@ -1,18 +0,0 @@ -/* - * 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_REPORTING_TYPE } from '../../../common/constants'; -import { getReportingUsage } from '../../../../reporting'; -import { UsageCollector } from '../'; - -export function getReportingUsageCollector(server, callCluster) { - return new UsageCollector(server, { - type: KIBANA_REPORTING_TYPE, - fetch() { - return getReportingUsage(callCluster, server); - } - }); -} diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/index.js b/x-pack/plugins/monitoring/server/kibana_monitoring/index.js index 3d4afbbb22bdd..a79c7aa2ecc11 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/index.js +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/index.js @@ -6,5 +6,4 @@ export { createCollectorSet } from './create_collector_set'; export { getKibanaUsageCollector } from './collectors/get_kibana_usage_collector'; -export { getReportingUsageCollector } from './collectors/get_reporting_usage_collector'; export { UsageCollector } from './classes/usage_collector'; diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/lib/get_collector_types_combiner.js b/x-pack/plugins/monitoring/server/kibana_monitoring/lib/get_collector_types_combiner.js index a8ce1510915ee..7c0d44699d958 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/lib/get_collector_types_combiner.js +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/lib/get_collector_types_combiner.js @@ -9,8 +9,10 @@ import { KIBANA_STATS_TYPE, KIBANA_SETTINGS_TYPE, KIBANA_USAGE_TYPE, - KIBANA_REPORTING_TYPE, } from '../../../common/constants'; + +import { KIBANA_REPORTING_TYPE } from '../../../../reporting/common/constants'; + import { sourceKibana } from './source_kibana'; /* diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/start_collector_set.js b/x-pack/plugins/monitoring/server/kibana_monitoring/start_collector_set.js index 4f898ef4eb6c8..434b3009aa07c 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/start_collector_set.js +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/start_collector_set.js @@ -9,7 +9,6 @@ import { CollectorSet } from './classes/collector_set'; import { getOpsStatsCollector } from './collectors/get_ops_stats_collector'; import { getSettingsCollector } from './collectors/get_settings_collector'; import { getKibanaUsageCollector } from './collectors/get_kibana_usage_collector'; -import { getReportingUsageCollector } from './collectors/get_reporting_usage_collector'; import { sendBulkPayload } from './lib/send_bulk_payload'; import { getCollectorTypesCombiner } from './lib/get_collector_types_combiner'; @@ -39,7 +38,6 @@ export function startCollectorSet(kbnServer, server, client, _sendBulkPayload = collectorSet.register(getKibanaUsageCollector(server, callCluster)); collectorSet.register(getOpsStatsCollector(server)); collectorSet.register(getSettingsCollector(server)); - collectorSet.register(getReportingUsageCollector(server, callCluster)); // TODO: move this to Reporting init // Startup Kibana cleanly or reconnect to Elasticsearch server.plugins.elasticsearch.status.on('green', () => { diff --git a/x-pack/plugins/reporting/common/constants.js b/x-pack/plugins/reporting/common/constants.js index 53f6e24af5ca0..6376c44042e25 100644 --- a/x-pack/plugins/reporting/common/constants.js +++ b/x-pack/plugins/reporting/common/constants.js @@ -12,4 +12,10 @@ export const API_BASE_URL = '/api/reporting'; export const WHITELISTED_JOB_CONTENT_TYPES = [ 'application/json', 'application/pdf', 'text/csv' ]; -export const UI_SETTINGS_CUSTOM_PDF_LOGO = 'xpackReporting:customPdfLogo'; \ No newline at end of file +export const UI_SETTINGS_CUSTOM_PDF_LOGO = 'xpackReporting:customPdfLogo'; + +/** + * The type name used within the Monitoring index to publish reporting stats. + * @type {string} + */ +export const KIBANA_REPORTING_TYPE = 'reporting_stats'; diff --git a/x-pack/plugins/reporting/index.js b/x-pack/plugins/reporting/index.js index e7dd3c6e3f0d1..4e3813707b4db 100644 --- a/x-pack/plugins/reporting/index.js +++ b/x-pack/plugins/reporting/index.js @@ -18,7 +18,8 @@ import { exportTypesRegistryFactory } from './server/lib/export_types_registry'; import { createBrowserDriverFactory, getDefaultBrowser, getDefaultChromiumSandboxDisabled } from './server/browsers'; import { logConfiguration } from './log_configuration'; -export { getReportingUsage } from './server/usage'; +import { callClusterFactory } from '../xpack_main'; +import { getReportingUsageCollector } from './server/usage'; const kbToBase64Length = (kb) => { return Math.floor((kb * 1024 * 8) / 6); @@ -146,7 +147,8 @@ export const reporting = (kibana) => { validateConfig(config, message => server.log(['reporting', 'warning'], message)); logConfiguration(config, message => server.log(['reporting', 'debug'], message)); - const xpackMainPlugin = server.plugins.xpack_main; + const { xpack_main: xpackMainPlugin, monitoring: monitoringPlugin } = server.plugins; + mirrorPluginStatus(xpackMainPlugin, this); const checkLicense = checkLicenseFactory(exportTypesRegistry); xpackMainPlugin.status.once('green', () => { @@ -155,6 +157,15 @@ export const reporting = (kibana) => { xpackMainPlugin.info.feature(this.id).registerLicenseCheckResultsGenerator(checkLicense); }); + // Register a function to with Monitoring to manage the collection of usage stats + // TODO: take plugin stats collection out of Monitoring: https://github.com/elastic/kibana/issues/18242 + monitoringPlugin.status.once('green', () => { + if (monitoringPlugin.collectorSet) { + const callCluster = callClusterFactory(server).getCallClusterInternal(); // uses callWithInternal as this is for internal collection + monitoringPlugin.collectorSet.register(getReportingUsageCollector(server, callCluster)); + } + }); + server.expose('browserDriverFactory', await createBrowserDriverFactory(server)); server.expose('queue', createQueueFactory(server)); diff --git a/x-pack/plugins/reporting/server/usage/get_usage.js b/x-pack/plugins/reporting/server/usage/get_reporting_usage_collector.js similarity index 70% rename from x-pack/plugins/reporting/server/usage/get_usage.js rename to x-pack/plugins/reporting/server/usage/get_reporting_usage_collector.js index 8bb90eedffa45..ad4dbd6a24da2 100644 --- a/x-pack/plugins/reporting/server/usage/get_usage.js +++ b/x-pack/plugins/reporting/server/usage/get_reporting_usage_collector.js @@ -6,9 +6,9 @@ import { uniq } from 'lodash'; import { getExportTypesHandler } from './get_export_type_handler'; -import { - getReportCountsByParameter, -} from './get_reporting_type_counts'; +import { getReportCountsByParameter } from './get_reporting_type_counts'; +import { KIBANA_REPORTING_TYPE } from '../../common/constants'; +import { UsageCollector } from '../../../monitoring/server/kibana_monitoring'; // TODO: should come from xpack_main /** * @typedef {Object} ReportingUsageStats Almost all of these stats are optional. @@ -110,34 +110,44 @@ async function getReportingUsageWithinRange(callCluster, server, reportingAvaila }; } -export async function getReportingUsage(callCluster, server) { - const xpackInfo = server.plugins.xpack_main.info; - const config = server.config(); - const available = xpackInfo && xpackInfo.isAvailable(); // some form of reporting (csv at least) is available for all valid licenses - const enabled = config.get('xpack.reporting.enabled'); // follow ES behavior: if its not available then its not enabled - const reportingAvailable = available && enabled; - - const statsOverAllTime = await getReportingUsageWithinRange(callCluster, server, reportingAvailable); - const statsOverLast1Day = await getReportingUsageWithinRange(callCluster, server, reportingAvailable, 1); - const statsOverLast7Days = await getReportingUsageWithinRange(callCluster, server, reportingAvailable, 7); - - let browserType; - if (enabled) { - // Allow this to explictly throw an exception if/when this config is deprecated, - // because we shouldn't collect browserType in that case! - browserType = config.get('xpack.reporting.capture.browser.type'); - } +/* + * @param {Object} server + * @param {Function} callCluster - function that uses either callWithRequest or callWithInternal to fetch data from ES + * @return {Object} kibana usage stats type collection object + */ +export function getReportingUsageCollector(server, callCluster) { + return new UsageCollector(server, { + type: KIBANA_REPORTING_TYPE, + fetch: async () => { + const xpackInfo = server.plugins.xpack_main.info; + const config = server.config(); + const available = xpackInfo && xpackInfo.isAvailable(); // some form of reporting (csv at least) is available for all valid licenses + const enabled = config.get('xpack.reporting.enabled'); // follow ES behavior: if its not available then its not enabled + const reportingAvailable = available && enabled; + + const statsOverAllTime = await getReportingUsageWithinRange(callCluster, server, reportingAvailable); + const statsOverLast1Day = await getReportingUsageWithinRange(callCluster, server, reportingAvailable, 1); + const statsOverLast7Days = await getReportingUsageWithinRange(callCluster, server, reportingAvailable, 7); + + let browserType; + if (enabled) { + // Allow this to explictly throw an exception if/when this config is deprecated, + // because we shouldn't collect browserType in that case! + browserType = config.get('xpack.reporting.capture.browser.type'); + } - return { - available, - enabled: available && enabled, // similar behavior as _xpack API in ES - browser_type: browserType, - ...statsOverAllTime, - lastDay: { - ...statsOverLast1Day - }, - last7Days: { - ...statsOverLast7Days + return { + available, + enabled: available && enabled, // similar behavior as _xpack API in ES + browser_type: browserType, + ...statsOverAllTime, + lastDay: { + ...statsOverLast1Day + }, + last7Days: { + ...statsOverLast7Days + } + }; } - }; + }); } diff --git a/x-pack/plugins/reporting/server/usage/get_usage.test.js b/x-pack/plugins/reporting/server/usage/get_reporting_usage_collector.test.js similarity index 83% rename from x-pack/plugins/reporting/server/usage/get_usage.test.js rename to x-pack/plugins/reporting/server/usage/get_reporting_usage_collector.test.js index ab3c773bd79c5..c92643dbcfd36 100644 --- a/x-pack/plugins/reporting/server/usage/get_usage.test.js +++ b/x-pack/plugins/reporting/server/usage/get_reporting_usage_collector.test.js @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ import sinon from 'sinon'; -import { getReportingUsage } from './get_usage'; +import { getReportingUsageCollector } from './get_reporting_usage_collector'; function getServerMock(customization) { const getLicenseCheckResults = sinon.stub().returns({}); @@ -52,7 +52,8 @@ test('sets enabled to false when reporting is turned off', async () => { }); const serverMock = getServerMock({ config: () => ({ get: mockConfigGet }) }); const callClusterMock = jest.fn(); - const usageStats = await getReportingUsage(callClusterMock, serverMock); + const { fetch: getReportingUsage } = getReportingUsageCollector(serverMock, callClusterMock); + const usageStats = await getReportingUsage(); expect(usageStats.enabled).toBe(false); }); @@ -62,7 +63,8 @@ describe('with a basic license', async () => { const serverWithBasicLicenseMock = getServerMock(); serverWithBasicLicenseMock.plugins.xpack_main.info.license.getType = sinon.stub().returns('basic'); const callClusterMock = jest.fn(() => Promise.resolve({})); - usageStats = await getReportingUsage(callClusterMock, serverWithBasicLicenseMock); + const { fetch: getReportingUsage } = getReportingUsageCollector(serverWithBasicLicenseMock, callClusterMock); + usageStats = await getReportingUsage(); }); test('sets enables to true', async () => { @@ -84,7 +86,8 @@ describe('with no license', async () => { const serverWithNoLicenseMock = getServerMock(); serverWithNoLicenseMock.plugins.xpack_main.info.license.getType = sinon.stub().returns('none'); const callClusterMock = jest.fn(() => Promise.resolve({})); - usageStats = await getReportingUsage(callClusterMock, serverWithNoLicenseMock); + const { fetch: getReportingUsage } = getReportingUsageCollector(serverWithNoLicenseMock, callClusterMock); + usageStats = await getReportingUsage(); }); test('sets enables to true', async () => { @@ -106,7 +109,8 @@ describe('with platinum license', async () => { const serverWithPlatinumLicenseMock = getServerMock(); serverWithPlatinumLicenseMock.plugins.xpack_main.info.license.getType = sinon.stub().returns('platinum'); const callClusterMock = jest.fn(() => Promise.resolve({})); - usageStats = await getReportingUsage(callClusterMock, serverWithPlatinumLicenseMock); + const { fetch: getReportingUsage } = getReportingUsageCollector(serverWithPlatinumLicenseMock, callClusterMock); + usageStats = await getReportingUsage(); }); test('sets enables to true', async () => { diff --git a/x-pack/plugins/reporting/server/usage/index.js b/x-pack/plugins/reporting/server/usage/index.js index 37988d9b21c70..91e2a9284550b 100644 --- a/x-pack/plugins/reporting/server/usage/index.js +++ b/x-pack/plugins/reporting/server/usage/index.js @@ -4,4 +4,4 @@ * you may not use this file except in compliance with the Elastic License. */ -export { getReportingUsage } from './get_usage'; +export { getReportingUsageCollector } from './get_reporting_usage_collector'; diff --git a/x-pack/plugins/xpack_main/server/routes/api/v1/kibana_stats.js b/x-pack/plugins/xpack_main/server/routes/api/v1/kibana_stats.js index 6bf8080981daf..ef2691e34309f 100644 --- a/x-pack/plugins/xpack_main/server/routes/api/v1/kibana_stats.js +++ b/x-pack/plugins/xpack_main/server/routes/api/v1/kibana_stats.js @@ -9,7 +9,8 @@ */ import { wrap } from 'boom'; import { callClusterFactory } from '../../../lib/call_cluster_factory'; -import { getKibanaUsageCollector, getReportingUsageCollector } from '../../../../../monitoring/server/kibana_monitoring'; +import { getKibanaUsageCollector } from '../../../../../monitoring/server/kibana_monitoring'; +import { getReportingUsageCollector } from '../../../../../reporting/server/usage'; export function kibanaStatsRoute(server) { server.route({ @@ -22,11 +23,11 @@ export function kibanaStatsRoute(server) { try { const kibanaUsageCollector = getKibanaUsageCollector(server, callCluster); - const reportingCollector = getReportingUsageCollector(server, callCluster); // TODO instead of hardcoding, loop through a set of usage collectors that have been registered to a server method + const reportingUsageCollector = getReportingUsageCollector(server, callCluster); // TODO instead of hardcoding, loop through a set of usage collectors that have been registered to a server method const [ kibana, reporting ] = await Promise.all([ kibanaUsageCollector.fetch(), - reportingCollector.fetch(), + reportingUsageCollector.fetch(), ]); reply({ From cf44a32bd099d576c869df3b26541187e4aa4d00 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Mon, 21 May 2018 17:13:40 -0700 Subject: [PATCH 2/2] a few cleanup changes --- .../kibana_monitoring/lib/get_collector_types_combiner.js | 2 -- x-pack/plugins/reporting/index.js | 1 - .../reporting/server/usage/get_reporting_usage_collector.js | 2 +- x-pack/plugins/xpack_main/server/routes/api/v1/kibana_stats.js | 2 +- 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/lib/get_collector_types_combiner.js b/x-pack/plugins/monitoring/server/kibana_monitoring/lib/get_collector_types_combiner.js index 7c0d44699d958..f50b080a9afc9 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/lib/get_collector_types_combiner.js +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/lib/get_collector_types_combiner.js @@ -10,9 +10,7 @@ import { KIBANA_SETTINGS_TYPE, KIBANA_USAGE_TYPE, } from '../../../common/constants'; - import { KIBANA_REPORTING_TYPE } from '../../../../reporting/common/constants'; - import { sourceKibana } from './source_kibana'; /* diff --git a/x-pack/plugins/reporting/index.js b/x-pack/plugins/reporting/index.js index 4e3813707b4db..4e4b2d18e59cc 100644 --- a/x-pack/plugins/reporting/index.js +++ b/x-pack/plugins/reporting/index.js @@ -158,7 +158,6 @@ export const reporting = (kibana) => { }); // Register a function to with Monitoring to manage the collection of usage stats - // TODO: take plugin stats collection out of Monitoring: https://github.com/elastic/kibana/issues/18242 monitoringPlugin.status.once('green', () => { if (monitoringPlugin.collectorSet) { const callCluster = callClusterFactory(server).getCallClusterInternal(); // uses callWithInternal as this is for internal collection diff --git a/x-pack/plugins/reporting/server/usage/get_reporting_usage_collector.js b/x-pack/plugins/reporting/server/usage/get_reporting_usage_collector.js index ad4dbd6a24da2..6c129a45108cb 100644 --- a/x-pack/plugins/reporting/server/usage/get_reporting_usage_collector.js +++ b/x-pack/plugins/reporting/server/usage/get_reporting_usage_collector.js @@ -8,7 +8,7 @@ import { uniq } from 'lodash'; import { getExportTypesHandler } from './get_export_type_handler'; import { getReportCountsByParameter } from './get_reporting_type_counts'; import { KIBANA_REPORTING_TYPE } from '../../common/constants'; -import { UsageCollector } from '../../../monitoring/server/kibana_monitoring'; // TODO: should come from xpack_main +import { UsageCollector } from '../../../monitoring/server/kibana_monitoring'; /** * @typedef {Object} ReportingUsageStats Almost all of these stats are optional. diff --git a/x-pack/plugins/xpack_main/server/routes/api/v1/kibana_stats.js b/x-pack/plugins/xpack_main/server/routes/api/v1/kibana_stats.js index ef2691e34309f..94a35e657003a 100644 --- a/x-pack/plugins/xpack_main/server/routes/api/v1/kibana_stats.js +++ b/x-pack/plugins/xpack_main/server/routes/api/v1/kibana_stats.js @@ -23,7 +23,7 @@ export function kibanaStatsRoute(server) { try { const kibanaUsageCollector = getKibanaUsageCollector(server, callCluster); - const reportingUsageCollector = getReportingUsageCollector(server, callCluster); // TODO instead of hardcoding, loop through a set of usage collectors that have been registered to a server method + const reportingUsageCollector = getReportingUsageCollector(server, callCluster); const [ kibana, reporting ] = await Promise.all([ kibanaUsageCollector.fetch(),