[7.x] [Usage collection] Make schema mandatory (#79999) (#81595)

* [Usage collection] Make `schema` mandatory (#79999) Co-authored-by: Ahmad Bamieh <[email protected]> Co-authored-by: Christiane (Tina) Heiligers <[email protected]> Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_settings_collector.ts * eslint fix Co-authored-by: Alejandro Fernández Haro <[email protected]>
elastic · Oct 26, 2020 · 6bc83d6 · 6bc83d6
1 parent cd06550
commit 6bc83d6
Show file tree

Hide file tree

Showing 16 changed files with 105 additions and 24 deletions.
diff --git a/.telemetryrc.json b/.telemetryrc.json
@@ -7,5 +7,10 @@
       "src/plugins/testbed/",
       "src/plugins/kibana_utils/"
     ]
+  },
+  {
+    "output": "src/plugins/telemetry/schema/legacy_plugins.json",
+    "root": "src/legacy/server/",
+    "exclude": []
   }
 ]
diff --git a/src/fixtures/telemetry_collectors/externally_defined_collector.ts b/src/fixtures/telemetry_collectors/externally_defined_collector.ts
@@ -16,7 +16,10 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { CollectorSet, CollectorOptions } from '../../plugins/usage_collection/server/collector';
+import {
+  CollectorSet,
+  UsageCollectorOptions,
+} from '../../plugins/usage_collection/server/collector';
 import { loggerMock } from '../../core/server/logging/logger.mock';
 
 const collectorSet = new CollectorSet({
@@ -28,7 +31,7 @@ interface Usage {
   locale: string;
 }
 
-function createCollector(): CollectorOptions<Usage> {
+function createCollector(): UsageCollectorOptions<Usage> {
   return {
     type: 'from_fn_collector',
     isReady: () => true,
@@ -46,7 +49,7 @@ function createCollector(): CollectorOptions<Usage> {
 }
 
 export function defineCollectorFromVariable() {
-  const fromVarCollector: CollectorOptions<Usage> = {
+  const fromVarCollector: UsageCollectorOptions<Usage> = {
     type: 'from_variable_collector',
     isReady: () => true,
     fetch(): Usage {

diff --git a/src/fixtures/telemetry_collectors/unmapped_collector.ts b/src/fixtures/telemetry_collectors/unmapped_collector.ts
@@ -28,6 +28,7 @@ interface Usage {
   locale: string;
 }
 
+// @ts-expect-error Intentionally not specifying `schema`
 export const myCollector = makeUsageCollector<Usage>({
   type: 'unmapped_collector',
   isReady: () => true,

diff --git a/src/legacy/server/i18n/constants.ts b/src/legacy/server/i18n/constants.ts
@@ -18,8 +18,3 @@
  */
 
 export const I18N_RC = '.i18nrc.json';
-
-/**
- * The type name used within the Monitoring index to publish localization stats.
- */
-export const KIBANA_LOCALIZATION_STATS_TYPE = 'localization';
diff --git a/src/legacy/server/i18n/localization/telemetry_localization_collector.ts b/src/legacy/server/i18n/localization/telemetry_localization_collector.ts
@@ -21,7 +21,6 @@ import { i18nLoader } from '@kbn/i18n';
 import { size } from 'lodash';
 import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
 import { getIntegrityHashes, Integrities } from './file_integrity';
-import { KIBANA_LOCALIZATION_STATS_TYPE } from '../constants';
 
 export interface UsageStats {
   locale: string;
@@ -63,14 +62,20 @@ export function createCollectorFetch({
   };
 }
 
+// TODO: Migrate out of the Legacy dir
 export function registerLocalizationUsageCollector(
   usageCollection: UsageCollectionSetup,
   helpers: LocalizationUsageCollectorHelpers
 ) {
-  const collector = usageCollection.makeUsageCollector({
-    type: KIBANA_LOCALIZATION_STATS_TYPE,
+  const collector = usageCollection.makeUsageCollector<UsageStats>({
+    type: 'localization',
     isReady: () => true,
     fetch: createCollectorFetch(helpers),
+    schema: {
+      locale: { type: 'keyword' },
+      integrities: { DYNAMIC_KEY: { type: 'text' } },
+      labelsCount: { type: 'long' },
+    },
   });
 
   usageCollection.registerCollector(collector);

diff --git a/src/plugins/data/server/server.api.md b/src/plugins/data/server/server.api.md
@@ -26,6 +26,7 @@ import { ISearchSource } from 'src/plugins/data/public';
 import { KibanaRequest } from 'src/core/server';
 import { LegacyAPICaller } from 'kibana/server';
 import { Logger } from 'kibana/server';
+import { Logger as Logger_2 } from 'src/core/server';
 import { LoggerFactory } from '@kbn/logging';
 import { Moment } from 'moment';
 import moment from 'moment';

diff --git a/src/plugins/kibana_usage_collection/server/collectors/csp/csp_collector.ts b/src/plugins/kibana_usage_collection/server/collectors/csp/csp_collector.ts
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import { UsageCollectionSetup, CollectorOptions } from 'src/plugins/usage_collection/server';
+import { UsageCollectionSetup, UsageCollectorOptions } from 'src/plugins/usage_collection/server';
 import { HttpServiceSetup, CspConfig } from '../../../../../core/server';
 
 interface Usage {
@@ -26,7 +26,7 @@ interface Usage {
   rulesChangedFromDefault: boolean;
 }
 
-export function createCspCollector(http: HttpServiceSetup): CollectorOptions<Usage> {
+export function createCspCollector(http: HttpServiceSetup): UsageCollectorOptions<Usage> {
   return {
     type: 'csp',
     isReady: () => true,

diff --git a/src/plugins/telemetry/schema/legacy_plugins.json b/src/plugins/telemetry/schema/legacy_plugins.json
@@ -0,0 +1,21 @@
+{
+  "properties": {
+    "localization": {
+      "properties": {
+        "locale": {
+          "type": "keyword"
+        },
+        "integrities": {
+          "properties": {
+            "DYNAMIC_KEY": {
+              "type": "text"
+            }
+          }
+        },
+        "labelsCount": {
+          "type": "long"
+        }
+      }
+    }
+  }
+}
diff --git a/src/plugins/usage_collection/server/collector/collector.test.ts b/src/plugins/usage_collection/server/collector/collector.test.ts
@@ -122,6 +122,7 @@ describe('collector', () => {
         type: 'my_test_collector',
         isReady: () => false,
         fetch: () => fetchOutput,
+        schema: { testPass: { type: 'long' } },
       });
       expect(collector.formatForBulkUpload(fetchOutput)).toStrictEqual({
         type: 'kibana_stats',

diff --git a/src/plugins/usage_collection/server/collector/collector.ts b/src/plugins/usage_collection/server/collector/collector.ts
@@ -71,17 +71,27 @@ export interface CollectorFetchContext {
 }
 
 export interface CollectorOptions<T = unknown, U = T> {
+  /**
+   * Unique string identifier for the collector
+   */
   type: string;
   init?: Function;
+  /**
+   * Method to return `true`/`false` to confirm if the collector is ready for the `fetch` method to be called.
+   */
+  isReady: () => Promise<boolean> | boolean;
+  /**
+   * Schema definition of the output of the `fetch` method.
+   */
   schema?: MakeSchemaFrom<T>;
   fetch: (collectorFetchContext: CollectorFetchContext) => Promise<T> | T;
   /*
    * A hook for allowing the fetched data payload to be organized into a typed
    * data model for internal bulk upload. See defaultFormatterForBulkUpload for
    * a generic example.
+   * @deprecated Used only by the Legacy Monitoring collection (to be removed in 8.0)
    */
   formatForBulkUpload?: CollectorFormatForBulkUpload<T, U>;
-  isReady: () => Promise<boolean> | boolean;
 }
 
 export class Collector<T = unknown, U = T> {

diff --git a/src/plugins/usage_collection/server/collector/collector_set.test.ts b/src/plugins/usage_collection/server/collector/collector_set.test.ts
@@ -20,7 +20,7 @@
 import { noop } from 'lodash';
 import { Collector } from './collector';
 import { CollectorSet } from './collector_set';
-import { UsageCollector } from './usage_collector';
+import { UsageCollector, UsageCollectorOptions } from './usage_collector';
 import {
   elasticsearchServiceMock,
   loggingSystemMock,
@@ -73,8 +73,9 @@ describe('CollectorSet', () => {
           // Even for Collector vs. UsageCollector
           new UsageCollector(logger, {
             type: 'test_duplicated',
-            fetch: () => 2,
+            fetch: () => ({ prop: 2 }),
             isReady: () => false,
+            schema: { prop: { type: 'long' } },
           })
         )
       ).toThrowError(`Usage collector's type "test_duplicated" is duplicated.`);
@@ -252,7 +253,12 @@ describe('CollectorSet', () => {
   });
 
   describe('isUsageCollector', () => {
-    const collectorOptions = { type: 'MY_TEST_COLLECTOR', fetch: () => {}, isReady: () => true };
+    const collectorOptions: UsageCollectorOptions = {
+      type: 'MY_TEST_COLLECTOR',
+      fetch: () => ({ test: 1 }),
+      isReady: () => true,
+      schema: { test: { type: 'long' } },
+    };
 
     it('returns true only for UsageCollector instances', () => {
       const collectors = new CollectorSet({ logger });

diff --git a/src/plugins/usage_collection/server/collector/collector_set.ts b/src/plugins/usage_collection/server/collector/collector_set.ts
@@ -26,7 +26,7 @@ import {
   SavedObjectsClientContract,
 } from 'kibana/server';
 import { Collector, CollectorOptions } from './collector';
-import { UsageCollector } from './usage_collector';
+import { UsageCollector, UsageCollectorOptions } from './usage_collector';
 
 interface CollectorSetConfig {
   logger: Logger;
@@ -45,10 +45,22 @@ export class CollectorSet {
     this.maximumWaitTimeForAllCollectorsInS = maximumWaitTimeForAllCollectorsInS || 60;
   }
 
-  public makeStatsCollector = <T, U>(options: CollectorOptions<T, U>) => {
+  public makeStatsCollector = <
+    T,
+    U,
+    O extends CollectorOptions<T, U> = CollectorOptions<T, U> // Used to allow extra properties (the Collector constructor extends the class with the additional options provided)
+  >(
+    options: O
+  ) => {
     return new Collector(this.logger, options);
   };
-  public makeUsageCollector = <T, U = T>(options: CollectorOptions<T, U>) => {
+  public makeUsageCollector = <
+    T,
+    U = T,
+    O extends UsageCollectorOptions<T, U> = UsageCollectorOptions<T, U>
+  >(
+    options: O
+  ) => {
     return new UsageCollector(this.logger, options);
   };
 

diff --git a/src/plugins/usage_collection/server/collector/index.ts b/src/plugins/usage_collection/server/collector/index.ts
@@ -26,4 +26,4 @@ export {
   CollectorOptions,
   CollectorFetchContext,
 } from './collector';
-export { UsageCollector } from './usage_collector';
+export { UsageCollector, UsageCollectorOptions } from './usage_collector';
diff --git a/src/plugins/usage_collection/server/collector/usage_collector.ts b/src/plugins/usage_collection/server/collector/usage_collector.ts
@@ -17,13 +17,22 @@
  * under the License.
  */
 
+import { Logger } from 'src/core/server';
 import { KIBANA_STATS_TYPE } from '../../common/constants';
-import { Collector } from './collector';
+import { Collector, CollectorOptions } from './collector';
+
+// Enforce the `schema` property for UsageCollectors
+export type UsageCollectorOptions<T = unknown, U = T> = CollectorOptions<T, U> &
+  Required<Pick<CollectorOptions<T, U>, 'schema'>>;
 
 export class UsageCollector<T = unknown, U = { usage: { [key: string]: T } }> extends Collector<
   T,
   U
 > {
+  constructor(protected readonly log: Logger, collectorOptions: UsageCollectorOptions<T, U>) {
+    super(log, collectorOptions);
+  }
+
   protected defaultFormatterForBulkUpload(result: T) {
     return {
       type: KIBANA_STATS_TYPE,

diff --git a/src/plugins/usage_collection/server/index.ts b/src/plugins/usage_collection/server/index.ts
@@ -25,6 +25,7 @@ export {
   MakeSchemaFrom,
   SchemaField,
   CollectorOptions,
+  UsageCollectorOptions,
   Collector,
   CollectorFetchContext,
 } from './collector';

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
@@ -10,7 +10,10 @@ import {
   CLUSTER_ALERTS_ADDRESS_CONFIG_KEY,
 } from '../../../common/constants';
 import { Logger } from '../../../../../../src/core/server';
-import { Collector } from '../../../../../../src/plugins/usage_collection/server';
+import {
+  Collector,
+  UsageCollectionSetup,
+} from '../../../../../../src/plugins/usage_collection/server';
 import { MonitoringConfig } from '../../config';
 import { CoreServices } from '../../core_services';
 
@@ -74,10 +77,18 @@ export interface KibanaSettingsCollector extends Collector<EmailSettingData | un
   getEmailValueStructure(email: string | null): EmailSettingData;
 }
 
-export function getSettingsCollector(usageCollection: any, config: MonitoringConfig) {
+export function getSettingsCollector(
+  usageCollection: UsageCollectionSetup,
+  config: MonitoringConfig
+) {
   return usageCollection.makeStatsCollector({
     type: KIBANA_SETTINGS_TYPE,
     isReady: () => true,
+    schema: {
+      xpack: {
+        default_admin_email: { type: 'text' },
+      },
+    },
     async fetch(this: KibanaSettingsCollector) {
       let kibanaSettingsData;
       const defaultAdminEmail = await checkForEmailValue(config);