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

[Monitoring/Reporting Stats] Move hardcoded Reporting stats type collection out of monitoring plugin #18894

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 0 additions & 5 deletions x-pack/plugins/monitoring/common/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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';
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import {
KIBANA_STATS_TYPE,
KIBANA_SETTINGS_TYPE,
KIBANA_USAGE_TYPE,
KIBANA_REPORTING_TYPE,
} from '../../../common/constants';
import { KIBANA_REPORTING_TYPE } from '../../../../reporting/common/constants';
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a step two planned to get rid of this and the abstraction leak below? Or maybe you can get rid of it now?

Copy link
Member Author

Choose a reason for hiding this comment

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

This module that combines collection types together into a bulk payload unfortunately does have to stay, but just for the short term. It's to support the behavior of Kibana Monitoring that sends kibana stats to the monitoring/_bulk endpoint in ES. Unfortunately, this module needs to have a lot of awareness of the usage collectors because in the current schema, usage is a field of kibana_stats.

image

We will take the responsibility of sending stats to ES out of Kibana and have Metricbeats used for monitoring all the systems. Once we do that, we can remove get_collector_types_combiner

Copy link
Member Author

Choose a reason for hiding this comment

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

import { sourceKibana } from './source_kibana';

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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', () => {
Expand Down
8 changes: 7 additions & 1 deletion x-pack/plugins/reporting/common/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
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';
14 changes: 12 additions & 2 deletions x-pack/plugins/reporting/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Copy link
Member Author

Choose a reason for hiding this comment

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

This needs testing on what happens when Monitoring is disabled but Reporting is enabled. cc @pickypg


mirrorPluginStatus(xpackMainPlugin, this);
const checkLicense = checkLicenseFactory(exportTypesRegistry);
xpackMainPlugin.status.once('green', () => {
Expand All @@ -155,6 +157,14 @@ export const reporting = (kibana) => {
xpackMainPlugin.info.feature(this.id).registerLicenseCheckResultsGenerator(checkLicense);
});

// Register a function to with Monitoring to manage the collection of usage stats
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));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
* @typedef {Object} ReportingUsageStats Almost all of these stats are optional.
Expand Down Expand Up @@ -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
}
};
}
};
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -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({});
Expand Down Expand Up @@ -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);
});

Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/reporting/server/usage/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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);

const [ kibana, reporting ] = await Promise.all([
kibanaUsageCollector.fetch(),
reportingCollector.fetch(),
reportingUsageCollector.fetch(),
]);

reply({
Expand Down