From d82b8d763195fe3ca322287ae26bb3e4f5ba0107 Mon Sep 17 00:00:00 2001 From: chrisronline Date: Mon, 23 Mar 2020 11:29:10 -0400 Subject: [PATCH 1/6] Align api/stats with the monitoring logic for kibana ops metrics --- src/legacy/server/status/index.js | 2 +- .../status/routes/api/register_stats.js | 29 +++++++++++++------ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/legacy/server/status/index.js b/src/legacy/server/status/index.js index a9544049182a7..df02b3c45ec2f 100644 --- a/src/legacy/server/status/index.js +++ b/src/legacy/server/status/index.js @@ -57,7 +57,7 @@ export function statusMixin(kbnServer, server, config) { // init routes registerStatusPage(kbnServer, server, config); registerStatusApi(kbnServer, server, config); - registerStatsApi(usageCollection, server, config); + registerStatsApi(usageCollection, server, config, kbnServer); // expore shared functionality server.decorate('server', 'getOSInfo', getOSInfo); diff --git a/src/legacy/server/status/routes/api/register_stats.js b/src/legacy/server/status/routes/api/register_stats.js index e218c1caf1701..2dd66cb8caff7 100644 --- a/src/legacy/server/status/routes/api/register_stats.js +++ b/src/legacy/server/status/routes/api/register_stats.js @@ -21,7 +21,7 @@ import Joi from 'joi'; import boom from 'boom'; import { i18n } from '@kbn/i18n'; import { wrapAuthConfig } from '../../wrap_auth_config'; -import { KIBANA_STATS_TYPE } from '../../constants'; +import { getKibanaInfoForStats } from '../../lib'; const STATS_NOT_READY_MESSAGE = i18n.translate('server.stats.notReadyMessage', { defaultMessage: 'Stats are not ready yet. Please try again later.', @@ -37,7 +37,7 @@ const STATS_NOT_READY_MESSAGE = i18n.translate('server.stats.notReadyMessage', { * - Any other value causes a statusCode 400 response (Bad Request) * Including ?exclude_usage in the query string excludes the usage stats from the response. Same value semantics as ?extended */ -export function registerStatsApi(usageCollection, server, config) { +export function registerStatsApi(usageCollection, server, config, kbnServer) { const wrapAuth = wrapAuthConfig(config.get('status.allowAnonymous')); const getClusterUuid = async callCluster => { @@ -50,6 +50,17 @@ export function registerStatsApi(usageCollection, server, config) { return usageCollection.toObject(usage); }; + let lastMetrics = null; + /* kibana_stats gets singled out from the collector set as it is used + * for health-checking Kibana and fetch does not rely on fetching data + * from ES */ + server.newPlatform.setup.core.metrics.getOpsMetrics$().subscribe(metrics => { + lastMetrics = { + ...metrics, + timestamp: new Date().toISOString(), + }; + }); + server.route( wrapAuth({ method: 'GET', @@ -133,15 +144,15 @@ export function registerStatsApi(usageCollection, server, config) { } } - /* kibana_stats gets singled out from the collector set as it is used - * for health-checking Kibana and fetch does not rely on fetching data - * from ES */ - const kibanaStatsCollector = usageCollection.getCollectorByType(KIBANA_STATS_TYPE); - if (!(await kibanaStatsCollector.isReady())) { + if (!lastMetrics) { return boom.serverUnavailable(STATS_NOT_READY_MESSAGE); } - let kibanaStats = await kibanaStatsCollector.fetch(); - kibanaStats = usageCollection.toApiFieldNames(kibanaStats); + const kibanaStats = usageCollection.toApiFieldNames({ + ...lastMetrics, + kibana: getKibanaInfoForStats(server, kbnServer), + last_updated: new Date().toISOString(), + collection_interval_in_millis: config.get('ops.interval'), + }); return { ...kibanaStats, From 25ef9865a646e9d5b186d924cedd8df4dabfba8b Mon Sep 17 00:00:00 2001 From: chrisronline Date: Mon, 23 Mar 2020 15:13:43 -0400 Subject: [PATCH 2/6] Align collectors --- .../server/status/routes/api/register_stats.js | 4 +++- .../collectors/get_ops_stats_collector.ts | 14 +++++++++++++- x-pack/plugins/monitoring/server/plugin.ts | 1 - 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/legacy/server/status/routes/api/register_stats.js b/src/legacy/server/status/routes/api/register_stats.js index 2dd66cb8caff7..8f634b9a4a187 100644 --- a/src/legacy/server/status/routes/api/register_stats.js +++ b/src/legacy/server/status/routes/api/register_stats.js @@ -19,6 +19,7 @@ import Joi from 'joi'; import boom from 'boom'; +import { cloneDeep } from 'lodash'; import { i18n } from '@kbn/i18n'; import { wrapAuthConfig } from '../../wrap_auth_config'; import { getKibanaInfoForStats } from '../../lib'; @@ -54,7 +55,8 @@ export function registerStatsApi(usageCollection, server, config, kbnServer) { /* kibana_stats gets singled out from the collector set as it is used * for health-checking Kibana and fetch does not rely on fetching data * from ES */ - server.newPlatform.setup.core.metrics.getOpsMetrics$().subscribe(metrics => { + server.newPlatform.setup.core.metrics.getOpsMetrics$().subscribe(_metrics => { + const metrics = cloneDeep(_metrics); lastMetrics = { ...metrics, timestamp: new Date().toISOString(), diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts index 00197e98948bf..6061a530229f6 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts @@ -5,6 +5,7 @@ */ import { Observable } from 'rxjs'; +import { cloneDeep, omit, set } from 'lodash'; import moment from 'moment'; import { OpsMetrics } from 'kibana/server'; import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; @@ -22,7 +23,18 @@ export function getOpsStatsCollector( metrics$: Observable ) { let lastMetrics: MonitoringOpsMetrics | null = null; - metrics$.subscribe(metrics => { + metrics$.subscribe(_metrics => { + const metrics = cloneDeep(_metrics); + // Ensure we only include the same data that Metricbeat collection would get + metrics.process = omit(metrics.process, 'pid'); + set(metrics, 'response_times', { + average: metrics.response_times.avg_in_millis, + max: metrics.response_times.max_in_millis, + }); + set(metrics, 'requests', { + ...omit(metrics.requests, 'statusCodes'), + status_codes: metrics.requests.statusCodes, + }); lastMetrics = { ...metrics, timestamp: moment.utc().toISOString(), diff --git a/x-pack/plugins/monitoring/server/plugin.ts b/x-pack/plugins/monitoring/server/plugin.ts index bd1455a2c582f..07f218006b6af 100644 --- a/x-pack/plugins/monitoring/server/plugin.ts +++ b/x-pack/plugins/monitoring/server/plugin.ts @@ -181,7 +181,6 @@ export class Plugin { index: get(legacyConfig, 'kibana.index'), host: serverInfo.host, transport_address: `${serverInfo.host}:${serverInfo.port}`, - port: serverInfo.port.toString(), version: this.initializerContext.env.packageInfo.version, snapshot: snapshotRegex.test(this.initializerContext.env.packageInfo.version), }, From 4b73cd3a2ca2f60f6b19e8a4ed30af74f767605e Mon Sep 17 00:00:00 2001 From: chrisronline Date: Mon, 23 Mar 2020 20:45:04 -0400 Subject: [PATCH 3/6] Add in locale to kibana_settings --- x-pack/legacy/plugins/monitoring/index.js | 1 + .../monitoring/server/kibana_monitoring/bulk_uploader.js | 6 ++++++ .../kibana_monitoring/collectors/get_settings_collector.ts | 3 --- x-pack/plugins/monitoring/server/plugin.ts | 4 +++- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/x-pack/legacy/plugins/monitoring/index.js b/x-pack/legacy/plugins/monitoring/index.js index fcf704b5f65d3..a989e586fa289 100644 --- a/x-pack/legacy/plugins/monitoring/index.js +++ b/x-pack/legacy/plugins/monitoring/index.js @@ -37,6 +37,7 @@ export const monitoring = kibana => { const status = kbnServerStatus.toJSON(); return get(status, 'overall.state'); }, + locale: serverConfig.get('i18n.locale'), }); } diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js b/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js index c09a08f61dc0a..bd3d4c59449fb 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js @@ -62,12 +62,17 @@ export class BulkUploader { this.kibanaStats = kibanaStats; this.kibanaStatusGetter = null; + this.kibanaLocaleGetter = null; } setKibanaStatusGetter(getter) { this.kibanaStatusGetter = getter; } + setKibanaLocaleGetter(getter) { + this.kibanaLocaleGetter = getter; + } + filterCollectorSet(usageCollection) { const successfulUploadInLastDay = this._lastFetchUsageTime && this._lastFetchUsageTime + this._usageInterval > Date.now(); @@ -191,6 +196,7 @@ export class BulkUploader { getKibanaStats() { return { ...this.kibanaStats, + locale: this.kibanaLocaleGetter(), status: this.kibanaStatusGetter(), }; } diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_settings_collector.ts b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_settings_collector.ts index 63e1dbc400787..c66adfcabd671 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_settings_collector.ts +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_settings_collector.ts @@ -12,9 +12,6 @@ import { MonitoringConfig } from '../../config'; * If so, get email from kibana.yml */ export async function getDefaultAdminEmail(config: MonitoringConfig) { - if (!config.cluster_alerts.email_notifications.enabled) { - return null; - } return config.cluster_alerts.email_notifications.email_address || null; } diff --git a/x-pack/plugins/monitoring/server/plugin.ts b/x-pack/plugins/monitoring/server/plugin.ts index 07f218006b6af..1f60e7307a4e0 100644 --- a/x-pack/plugins/monitoring/server/plugin.ts +++ b/x-pack/plugins/monitoring/server/plugin.ts @@ -52,7 +52,7 @@ import { InfraPluginSetup } from '../../infra/server'; export interface LegacyAPI { telemetryCollectionManager: TelemetryCollectionManager; getServerStatus: () => string; - infra: any; + locale: string; } interface PluginsSetup { @@ -86,6 +86,7 @@ interface LegacyShimDependencies { interface IBulkUploader { setKibanaStatusGetter: (getter: () => string | undefined) => void; + setKibanaLocaleGetter: (getter: () => string | undefined) => void; getKibanaStats: () => any; } @@ -279,6 +280,7 @@ export class Plugin { // Set the stats getter this.bulkUploader.setKibanaStatusGetter(() => legacyAPI.getServerStatus()); + this.bulkUploader.setKibanaLocaleGetter(() => legacyAPI.locale); } getLegacyShim( From 143ce887d3c95f441ada49852eab84d995630e4d Mon Sep 17 00:00:00 2001 From: chrisronline Date: Mon, 23 Mar 2020 21:52:01 -0400 Subject: [PATCH 4/6] More tweaks --- .../server/kibana_monitoring/bulk_uploader.js | 23 +++++++++++++++---- .../collectors/get_ops_stats_collector.ts | 5 +--- x-pack/plugins/monitoring/server/plugin.ts | 1 + 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js b/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js index bd3d4c59449fb..545bca397a262 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js @@ -6,7 +6,11 @@ import { defaultsDeep, uniq, compact, get } from 'lodash'; -import { TELEMETRY_COLLECTION_INTERVAL } from '../../common/constants'; +import { + TELEMETRY_COLLECTION_INTERVAL, + KIBANA_SETTINGS_TYPE, + KIBANA_STATS_TYPE_MONITORING, +} from '../../common/constants'; import { sendBulkPayload, monitoringBulk } from './lib'; import { hasMonitoringCluster } from '../es_client/instantiate_client'; @@ -193,12 +197,21 @@ export class BulkUploader { ); } - getKibanaStats() { - return { + getKibanaStats(type) { + const stats = { ...this.kibanaStats, - locale: this.kibanaLocaleGetter(), status: this.kibanaStatusGetter(), }; + + if (type === KIBANA_SETTINGS_TYPE) { + stats.locale = this.kibanaLocaleGetter(); + } + + if (type === KIBANA_STATS_TYPE_MONITORING) { + delete stats.port; + } + + return stats; } /* @@ -258,7 +271,7 @@ export class BulkUploader { ...accum, { index: { _type: type } }, { - kibana: this.getKibanaStats(), + kibana: this.getKibanaStats(type), ...typesNested[type], }, ]; diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts index 6061a530229f6..547b2ee2ae21b 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts @@ -31,10 +31,7 @@ export function getOpsStatsCollector( average: metrics.response_times.avg_in_millis, max: metrics.response_times.max_in_millis, }); - set(metrics, 'requests', { - ...omit(metrics.requests, 'statusCodes'), - status_codes: metrics.requests.statusCodes, - }); + metrics.requests = omit(metrics.requests, 'statusCodes'); lastMetrics = { ...metrics, timestamp: moment.utc().toISOString(), diff --git a/x-pack/plugins/monitoring/server/plugin.ts b/x-pack/plugins/monitoring/server/plugin.ts index 1f60e7307a4e0..d0d7ac39efa24 100644 --- a/x-pack/plugins/monitoring/server/plugin.ts +++ b/x-pack/plugins/monitoring/server/plugin.ts @@ -181,6 +181,7 @@ export class Plugin { name: serverInfo.name, index: get(legacyConfig, 'kibana.index'), host: serverInfo.host, + port: serverInfo.port.toString(), transport_address: `${serverInfo.host}:${serverInfo.port}`, version: this.initializerContext.env.packageInfo.version, snapshot: snapshotRegex.test(this.initializerContext.env.packageInfo.version), From 40e73664d96250f53375d644a70635ee5b9e2643 Mon Sep 17 00:00:00 2001 From: chrisronline Date: Tue, 24 Mar 2020 10:46:31 -0400 Subject: [PATCH 5/6] PR feedback --- src/legacy/server/status/routes/api/register_stats.js | 4 +--- x-pack/legacy/plugins/monitoring/index.js | 1 - .../server/kibana_monitoring/bulk_uploader.js | 11 +---------- .../collectors/get_ops_stats_collector.ts | 8 ++++---- x-pack/plugins/monitoring/server/plugin.ts | 4 +--- 5 files changed, 7 insertions(+), 21 deletions(-) diff --git a/src/legacy/server/status/routes/api/register_stats.js b/src/legacy/server/status/routes/api/register_stats.js index 8f634b9a4a187..2dd66cb8caff7 100644 --- a/src/legacy/server/status/routes/api/register_stats.js +++ b/src/legacy/server/status/routes/api/register_stats.js @@ -19,7 +19,6 @@ import Joi from 'joi'; import boom from 'boom'; -import { cloneDeep } from 'lodash'; import { i18n } from '@kbn/i18n'; import { wrapAuthConfig } from '../../wrap_auth_config'; import { getKibanaInfoForStats } from '../../lib'; @@ -55,8 +54,7 @@ export function registerStatsApi(usageCollection, server, config, kbnServer) { /* kibana_stats gets singled out from the collector set as it is used * for health-checking Kibana and fetch does not rely on fetching data * from ES */ - server.newPlatform.setup.core.metrics.getOpsMetrics$().subscribe(_metrics => { - const metrics = cloneDeep(_metrics); + server.newPlatform.setup.core.metrics.getOpsMetrics$().subscribe(metrics => { lastMetrics = { ...metrics, timestamp: new Date().toISOString(), diff --git a/x-pack/legacy/plugins/monitoring/index.js b/x-pack/legacy/plugins/monitoring/index.js index a989e586fa289..fcf704b5f65d3 100644 --- a/x-pack/legacy/plugins/monitoring/index.js +++ b/x-pack/legacy/plugins/monitoring/index.js @@ -37,7 +37,6 @@ export const monitoring = kibana => { const status = kbnServerStatus.toJSON(); return get(status, 'overall.state'); }, - locale: serverConfig.get('i18n.locale'), }); } diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js b/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js index 545bca397a262..b90a9aa7d139a 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js @@ -8,7 +8,6 @@ import { defaultsDeep, uniq, compact, get } from 'lodash'; import { TELEMETRY_COLLECTION_INTERVAL, - KIBANA_SETTINGS_TYPE, KIBANA_STATS_TYPE_MONITORING, } from '../../common/constants'; @@ -66,17 +65,12 @@ export class BulkUploader { this.kibanaStats = kibanaStats; this.kibanaStatusGetter = null; - this.kibanaLocaleGetter = null; } setKibanaStatusGetter(getter) { this.kibanaStatusGetter = getter; } - setKibanaLocaleGetter(getter) { - this.kibanaLocaleGetter = getter; - } - filterCollectorSet(usageCollection) { const successfulUploadInLastDay = this._lastFetchUsageTime && this._lastFetchUsageTime + this._usageInterval > Date.now(); @@ -203,12 +197,9 @@ export class BulkUploader { status: this.kibanaStatusGetter(), }; - if (type === KIBANA_SETTINGS_TYPE) { - stats.locale = this.kibanaLocaleGetter(); - } - if (type === KIBANA_STATS_TYPE_MONITORING) { delete stats.port; + delete stats.locale; } return stats; diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts index 547b2ee2ae21b..719118af709a1 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts @@ -5,7 +5,7 @@ */ import { Observable } from 'rxjs'; -import { cloneDeep, omit, set } from 'lodash'; +import { cloneDeep, omit } from 'lodash'; import moment from 'moment'; import { OpsMetrics } from 'kibana/server'; import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; @@ -24,13 +24,13 @@ export function getOpsStatsCollector( ) { let lastMetrics: MonitoringOpsMetrics | null = null; metrics$.subscribe(_metrics => { - const metrics = cloneDeep(_metrics); + const metrics: any = cloneDeep(_metrics); // Ensure we only include the same data that Metricbeat collection would get metrics.process = omit(metrics.process, 'pid'); - set(metrics, 'response_times', { + metrics.response_times = { average: metrics.response_times.avg_in_millis, max: metrics.response_times.max_in_millis, - }); + }; metrics.requests = omit(metrics.requests, 'statusCodes'); lastMetrics = { ...metrics, diff --git a/x-pack/plugins/monitoring/server/plugin.ts b/x-pack/plugins/monitoring/server/plugin.ts index d0d7ac39efa24..e7cf649c074d7 100644 --- a/x-pack/plugins/monitoring/server/plugin.ts +++ b/x-pack/plugins/monitoring/server/plugin.ts @@ -52,7 +52,6 @@ import { InfraPluginSetup } from '../../infra/server'; export interface LegacyAPI { telemetryCollectionManager: TelemetryCollectionManager; getServerStatus: () => string; - locale: string; } interface PluginsSetup { @@ -86,7 +85,6 @@ interface LegacyShimDependencies { interface IBulkUploader { setKibanaStatusGetter: (getter: () => string | undefined) => void; - setKibanaLocaleGetter: (getter: () => string | undefined) => void; getKibanaStats: () => any; } @@ -181,6 +179,7 @@ export class Plugin { name: serverInfo.name, index: get(legacyConfig, 'kibana.index'), host: serverInfo.host, + locale: i18n.getLocale(), port: serverInfo.port.toString(), transport_address: `${serverInfo.host}:${serverInfo.port}`, version: this.initializerContext.env.packageInfo.version, @@ -281,7 +280,6 @@ export class Plugin { // Set the stats getter this.bulkUploader.setKibanaStatusGetter(() => legacyAPI.getServerStatus()); - this.bulkUploader.setKibanaLocaleGetter(() => legacyAPI.locale); } getLegacyShim( From 3cd0c21081b800da56927c57c20a4f46c7251a15 Mon Sep 17 00:00:00 2001 From: chrisronline Date: Tue, 24 Mar 2020 14:05:47 -0400 Subject: [PATCH 6/6] PR feedback --- .../kibana_monitoring/collectors/get_ops_stats_collector.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts index 719118af709a1..85357f786ddc1 100644 --- a/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts +++ b/x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_ops_stats_collector.ts @@ -5,7 +5,7 @@ */ import { Observable } from 'rxjs'; -import { cloneDeep, omit } from 'lodash'; +import { cloneDeep } from 'lodash'; import moment from 'moment'; import { OpsMetrics } from 'kibana/server'; import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; @@ -26,12 +26,12 @@ export function getOpsStatsCollector( metrics$.subscribe(_metrics => { const metrics: any = cloneDeep(_metrics); // Ensure we only include the same data that Metricbeat collection would get - metrics.process = omit(metrics.process, 'pid'); + delete metrics.process.pid; metrics.response_times = { average: metrics.response_times.avg_in_millis, max: metrics.response_times.max_in_millis, }; - metrics.requests = omit(metrics.requests, 'statusCodes'); + delete metrics.requests.statusCodes; lastMetrics = { ...metrics, timestamp: moment.utc().toISOString(),