From 6bc83d613dbf3995ac5fba6e3bf5f58262c9e969 Mon Sep 17 00:00:00 2001 From: Ahmad Bamieh Date: Mon, 26 Oct 2020 23:34:13 +0300 Subject: [PATCH] [7.x] [Usage collection] Make `schema` mandatory (#79999) (#81595) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Usage collection] Make `schema` mandatory (#79999) Co-authored-by: Ahmad Bamieh Co-authored-by: Christiane (Tina) Heiligers Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_settings_collector.ts * eslint fix Co-authored-by: Alejandro Fernández Haro --- .telemetryrc.json | 5 +++++ .../externally_defined_collector.ts | 9 +++++--- .../unmapped_collector.ts | 1 + src/legacy/server/i18n/constants.ts | 5 ----- .../telemetry_localization_collector.ts | 11 +++++++--- src/plugins/data/server/server.api.md | 1 + .../server/collectors/csp/csp_collector.ts | 4 ++-- .../telemetry/schema/legacy_plugins.json | 21 +++++++++++++++++++ .../server/collector/collector.test.ts | 1 + .../server/collector/collector.ts | 12 ++++++++++- .../server/collector/collector_set.test.ts | 12 ++++++++--- .../server/collector/collector_set.ts | 18 +++++++++++++--- .../server/collector/index.ts | 2 +- .../server/collector/usage_collector.ts | 11 +++++++++- src/plugins/usage_collection/server/index.ts | 1 + .../collectors/get_settings_collector.ts | 15 +++++++++++-- 16 files changed, 105 insertions(+), 24 deletions(-) create mode 100644 src/plugins/telemetry/schema/legacy_plugins.json diff --git a/.telemetryrc.json b/.telemetryrc.json index 3d1b0df1d8f93..0f1530c6225d6 100644 --- 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 index 00a8d643e27b3..decee9df28185 100644 --- 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 { +function createCollector(): UsageCollectorOptions { return { type: 'from_fn_collector', isReady: () => true, @@ -46,7 +49,7 @@ function createCollector(): CollectorOptions { } export function defineCollectorFromVariable() { - const fromVarCollector: CollectorOptions = { + const fromVarCollector: UsageCollectorOptions = { 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 index 1ea360fcd9e96..143bb168aff35 100644 --- 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({ type: 'unmapped_collector', isReady: () => true, diff --git a/src/legacy/server/i18n/constants.ts b/src/legacy/server/i18n/constants.ts index 96fa420d4c6e1..a7a410dbcb5b3 100644 --- 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 index 89566dfd4ef68..fb837f5ae28df 100644 --- 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({ + 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 index ce6de0a3ba9a6..a96d8e52134cb 100644 --- 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 index c45a83588ee44..8cd60e5477d02 100644 --- 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 { +export function createCspCollector(http: HttpServiceSetup): UsageCollectorOptions { return { type: 'csp', isReady: () => true, diff --git a/src/plugins/telemetry/schema/legacy_plugins.json b/src/plugins/telemetry/schema/legacy_plugins.json new file mode 100644 index 0000000000000..1a7c0ccb15082 --- /dev/null +++ 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 index 375fe4f7686c0..875414fbeec48 100644 --- 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 index 11a709c037783..951418d448cbd 100644 --- 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 { + /** + * 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; + /** + * Schema definition of the output of the `fetch` method. + */ schema?: MakeSchemaFrom; fetch: (collectorFetchContext: CollectorFetchContext) => Promise | 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; - isReady: () => Promise | boolean; } export class Collector { diff --git a/src/plugins/usage_collection/server/collector/collector_set.test.ts b/src/plugins/usage_collection/server/collector/collector_set.test.ts index 45a3437777c5f..359a2d214f991 100644 --- 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 index 4e64cbc1bf30f..c52830cda6513 100644 --- 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 = (options: CollectorOptions) => { + public makeStatsCollector = < + T, + U, + O extends CollectorOptions = CollectorOptions // 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 = (options: CollectorOptions) => { + public makeUsageCollector = < + T, + U = T, + O extends UsageCollectorOptions = UsageCollectorOptions + >( + 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 index c294ba77d3cdb..da85f9ab181c9 100644 --- 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 index bf861a94fccff..5bfc36537e0b0 100644 --- 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 = CollectorOptions & + Required, 'schema'>>; export class UsageCollector extends Collector< T, U > { + constructor(protected readonly log: Logger, collectorOptions: UsageCollectorOptions) { + 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 index 80e34b1502cda..f7a08fdb5e9dd 100644 --- 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 index 1ea473b0a8d0f..8d48132c00e79 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 @@ -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 true, + schema: { + xpack: { + default_admin_email: { type: 'text' }, + }, + }, async fetch(this: KibanaSettingsCollector) { let kibanaSettingsData; const defaultAdminEmail = await checkForEmailValue(config);