[Monitoring] Add a yellow status phase in plugin init (#18939) (#19075)

* [Monitoring] Add a yellow status phase in the startup lifecycle

* comments

* more comments

* more comment

* undo register => registerType function name change

x-pack/plugins/monitoring/init.js

@@ -10,46 +10,36 @@ import { initMonitoringXpackInfo } from './server/init_monitoring_xpack_info';
 import { initKibanaMonitoring } from './server/kibana_monitoring';
 
 /**
- * Initialize the Kibana Monitoring plugin by starting up asynchronous server
- * tasks, based on user-defined configuration
- * - webserver route handling
- * - monitoring cluster health checker
- * - instantiation of an elasticsearch-js client exposed as a server plugin object
- * - start kibana ops monitoring loop
- * - start monitoring cluster x-pack license and features check loop
+ * Initialize the Kibana Monitoring plugin by starting up asynchronous server tasks
+ * - [1] instantiation of an elasticsearch-js client exposed as a server plugin object
+ * - [2] start monitoring cluster x-pack license and features check
+ * - [3] webserver route handling
+ * - [4] start the internal monitoring collectors
+ * - [5] expose the monitoring collector object for other plugins to register with
+ * - [6] set monitoring plugin status to green
  * @param monitoringPlugin {Object} Monitoring UI plugin
  * @param server {Object} HapiJS server instance
  */
 export const init = (monitoringPlugin, server) => {
+  monitoringPlugin.status.yellow('Initializing');
   const xpackMainPlugin = server.plugins.xpack_main;
 
   xpackMainPlugin.status.once('green', async () => {
     const config = server.config();
     const uiEnabled = config.get('xpack.monitoring.ui.enabled');
-    const features = [];
-    const onceMonitoringGreen = callbackFn => monitoringPlugin.status.once('green', () => callbackFn()); // avoid race condition in things that require ES client
 
     if (uiEnabled) {
-      // Instantiate the dedicated ES client
-      features.push(instantiateClient(server));
-
-      // route handlers depend on xpackInfo (exposed as server.plugins.monitoring.info)
-      onceMonitoringGreen(async () => {
-        await initMonitoringXpackInfo(server);
-      });
-
-      // Require only routes needed for ui app
-      features.push(requireUIRoutes(server));
+      await instantiateClient(server); // Instantiate the dedicated ES client
+      await initMonitoringXpackInfo(server); // Route handlers depend on this for xpackInfo
+      await requireUIRoutes(server);
     }
 
-    // Send Kibana usage / server ops to the monitoring bulk api
     if (config.get('xpack.monitoring.kibana.collection.enabled')) {
-      onceMonitoringGreen(() => {
-        features.push(initKibanaMonitoring(monitoringPlugin.kbnServer, server));
-      });
+      const collector = initKibanaMonitoring(monitoringPlugin.kbnServer, server); // instantiate an object for collecting/sending metrics and usage stats
+      server.expose('typeCollector', collector); // expose the collector object on the server. other plugins will call typeCollector.register(typeDefinition) to define their own collection
     }
 
-    Promise.all(features);
+    monitoringPlugin.status.green('Ready');
   });
 
   server.injectUiAppVars('monitoring', (server) => {

x-pack/plugins/monitoring/server/kibana_monitoring/init_kibana_monitoring.js

@@ -11,21 +11,25 @@ import { startCollector } from './start_collector';
 /**
  * @param kbnServer {Object} manager of Kibana services - see `src/server/kbn_server` in Kibana core
  * @param server {Object} HapiJS server instance
+ * @return {Object} TypeCollector instance to be exposed at a higher level, for other plugins to register their own type collectors
  */
-export async function initKibanaMonitoring(kbnServer, server) {
+export function initKibanaMonitoring(kbnServer, server) {
   const mainXpackInfo = server.plugins.xpack_main.info;
   const mainMonitoring = mainXpackInfo.feature('monitoring');
 
+  let collector;
+
   if (mainXpackInfo && mainMonitoring.isAvailable() && mainMonitoring.isEnabled()) {
     const client = server.plugins.elasticsearch.getCluster('admin').createClient({
       plugins: [monitoringBulk]
     });
-
-    startCollector(kbnServer, server, client);
+    collector = startCollector(kbnServer, server, client);
   } else {
     server.log(
       ['error', LOGGING_TAG, KIBANA_MONITORING_LOGGING_TAG],
       'Unable to retrieve X-Pack info from the admin cluster. Kibana monitoring will be disabled until Kibana is restarted.'
     );
   }
+
+  return collector;
 }

x-pack/plugins/monitoring/server/kibana_monitoring/lib/get_collector_types_combiner.js

@@ -14,11 +14,23 @@ import {
 import { sourceKibana } from './source_kibana';
 
 /*
- * Note: The ES Bulk Data Format is an array with 2 objects.
- * The first object is the header, it has a field for the action (index), and
- * metadata of the document (_index, _type, _id).
- * Since the action types are always "index", there second object which is the
- * payload, or the actual document to index.
+ * Combine stats collected from different sources into a single bulk payload.
+ *
+ * The ES Bulk Data Format is an array with 2 objects:
+ * - The first object is the header, it has a field for the action (index), and
+ *   metadata of the document (_index, _type, _id).
+ * - The second object is the actual document to index.
+ *
+ * NOTE: https://github.com/elastic/kibana/issues/12504 asks that plugins have
+ * a way to register their own stats. It's not hard to move the stats collector
+ * methods under the ownership of the plugins that want it, but this module's
+ * behavior doesn't fit well with plugins registering their own stats. See the
+ * abstraction leak comments in the code.
+ *
+ * This module should go away when stats are collected by a Kibana metricbeat moduleset.
+ *  - Individual plugin operational stats can be added to the `/stats?extended` API response.
+ *  - Individual plugin usage stats can go into a new API similar to the `_xpack/usage` API in ES.
+ *  - Each plugin will have its own top-level property in the responses for these APIs.
  */
 export function getCollectorTypesCombiner(kbnServer, config, _sourceKibana = sourceKibana) {
   return payload => {
@@ -37,7 +49,7 @@ export function getCollectorTypesCombiner(kbnServer, config, _sourceKibana = sou
     if (statsHeader && statsPayload) {
       const [ usageHeader, usagePayload ] = findItem(KIBANA_USAGE_TYPE);
       const kibanaUsage = (usageHeader && usagePayload) ? usagePayload : null;
-      const reportingUsage = (reportingHeader && reportingPayload) ? reportingPayload : null;
+      const reportingUsage = (reportingHeader && reportingPayload) ? reportingPayload : null; // this is an abstraction leak
       statsResult = [
         statsHeader,
         {
@@ -49,7 +61,7 @@ export function getCollectorTypesCombiner(kbnServer, config, _sourceKibana = sou
         set(statsResult, '[1].usage', kibanaUsage);
       }
       if (reportingUsage) {
-        set(statsResult, '[1].usage.xpack.reporting', reportingUsage);
+        set(statsResult, '[1].usage.xpack.reporting', reportingUsage); // this is an abstraction leak
       }
     }
 

x-pack/plugins/monitoring/server/kibana_monitoring/lib/type_collector.js

@@ -10,6 +10,11 @@ import Promise from 'bluebird';
 
 const LOGGING_TAGS = [LOGGING_TAG, KIBANA_MONITORING_LOGGING_TAG];
 
+/*
+ * A collector object has types registered into it with the register(type)
+ * function. Each type that gets registered defines how to fetch its own data
+ * and combine it into a unified payload for bulk upload.
+ */
 export class TypeCollector {
 
   /*
@@ -47,15 +52,13 @@ export class TypeCollector {
   }
 
   /*
-   * @param {Object} collectorOptions (required)
-   * Fields:
-   * - type {String}
-   * - init {Function} (optional)
-   * - fetch {Function}
-   * - cleanup {Function} (optional)
+   * @param {String} type.type
+   * @param {Function} type.init (optional)
+   * @param {Function} type.fetch
+   * @param {Function} type.cleanup (optional)
    */
-  register(collectorOptions) {
-    this._collectors.push(collectorOptions);
+  register(type) {
+    this._collectors.push(type);
   }
 
   /*
@@ -106,7 +109,7 @@ export class TypeCollector {
 
     if (payload.length > 0) {
       try {
-        const combinedData = this._combineTypes(payload);
+        const combinedData = this._combineTypes(payload); // use the collector types combiner
         this._log.debug(`Uploading bulk Kibana monitoring payload`);
         this._onPayload(flatten(combinedData));
       } catch(err) {

x-pack/plugins/monitoring/server/kibana_monitoring/start_collector.js

@@ -41,7 +41,7 @@ export function startCollector(kbnServer, server, client, _sendBulkPayload = sen
   collector.register(getUsageCollector(server, callCluster));
   collector.register(getOpsStatsCollector(server));
   collector.register(getSettingsCollector(server));
-  collector.register(getReportingCollector(server, callCluster));
+  collector.register(getReportingCollector(server, callCluster)); // TODO: move this to Reporting init
 
   // Startup Kibana cleanly or reconnect to Elasticsearch
   server.plugins.elasticsearch.status.on('green', () => {
@@ -53,4 +53,6 @@ export function startCollector(kbnServer, server, client, _sendBulkPayload = sen
   server.plugins.elasticsearch.status.on('red', () => {
     collector.cleanup();
   });
+
+  return collector;
 }

x-pack/plugins/xpack_main/server/routes/api/v1/kibana_stats.js

@@ -19,7 +19,7 @@ export function kibanaStatsRoute(server) {
 
       try {
         const kibanaUsageCollector = getUsageCollector(server, callCluster);
-        const reportingCollector = getReportingCollector(server, callCluster);
+        const reportingCollector = getReportingCollector(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(),