From 7a245e071d061b49ebb6b80d5fb8c734437cce0d Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Tue, 3 Nov 2020 14:48:45 -0700 Subject: [PATCH 1/5] updates README --- src/plugins/usage_collection/README.md | 7 ++++++- .../usage_collection/server/collector/collector_set.ts | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/plugins/usage_collection/README.md b/src/plugins/usage_collection/README.md index 5a853972d34a82..3d446689c6e9cf 100644 --- a/src/plugins/usage_collection/README.md +++ b/src/plugins/usage_collection/README.md @@ -62,6 +62,8 @@ All you need to provide is a `type` for organizing your fields, `schema` field t total: 'long', }, }, + isReady: () => isCollectorFetchReady, // method to return true/false to confirm if the collector is ready for the fetch method to be called. + fetch: async (collectorFetchContext: CollectorFetchContext) => { // query ES or saved objects and get some data @@ -84,6 +86,9 @@ All you need to provide is a `type` for organizing your fields, `schema` field t Some background: - `MY_USAGE_TYPE` can be any string. It usually matches the plugin name. As a safety mechanism, we double check there are no duplicates at the moment of registering the collector. +- isReady() (added in v7.2.0 and v6.8.4) is a way for a usage collector to announce that some async process must finish first before it will return data synchronously. If any collector is not ready, we try fetching again, up to a set amount of time. Once the allocated time interval is up, we collect data from those collectors that are ready and skip any that are not. What this means is that if a collector returns `true` for `isReady` and it actually isn't ready to return data, there won't be telemetry data for that user for the day. Every usage collector owner needs to really think about what it would mean for them if they missed the first few documents when Kibana starts and should implement this function with custom logic. + + (e.g. the task manager needs to run a task first). If any collector reports that it is not ready when we try and collect data, we reset a flag to try again up to a specified interval - The `fetch` method needs to support multiple contexts in which it is called. For example, when stats are pulled from a Kibana Metricbeat module, the Beat calls Kibana's stats API to invoke usage collection. In this case, the `fetch` method is called as a result of an HTTP API request and `callCluster` wraps `callWithRequest` or `esClient` wraps `asCurrentUser`, where the request headers are expected to have read privilege on the entire `.kibana' index. The `fetch` method also exposes the saved objects client that will have the correct scope when the collectors' `fetch` method is called. @@ -154,7 +159,7 @@ If any of your properties is an array, the schema definition must follow the con ```ts export const myCollector = makeUsageCollector({ type: 'my_working_collector', - isReady: () => true, + isReady: () => isFetchReadyToReturnData(), fetch() { return { my_greeting: 'hello', diff --git a/src/plugins/usage_collection/server/collector/collector_set.ts b/src/plugins/usage_collection/server/collector/collector_set.ts index c52830cda6513b..fa0fd513334f32 100644 --- a/src/plugins/usage_collection/server/collector/collector_set.ts +++ b/src/plugins/usage_collection/server/collector/collector_set.ts @@ -117,6 +117,7 @@ export class CollectorSet { const allReady = collectorsTypesNotReady.length === 0; if (!allReady && this.maximumWaitTimeForAllCollectorsInS >= 0) { + // some collectors return false for isReady() and we still have time left to wait const nowTimestamp = +new Date(); this._waitingForAllCollectorsTimestamp = this._waitingForAllCollectorsTimestamp || nowTimestamp; From b945ad08c98990feef7673478d2dfaf4d1e4a2d0 Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Tue, 3 Nov 2020 14:49:45 -0700 Subject: [PATCH 2/5] Removes comment --- src/plugins/usage_collection/server/collector/collector_set.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plugins/usage_collection/server/collector/collector_set.ts b/src/plugins/usage_collection/server/collector/collector_set.ts index fa0fd513334f32..c52830cda6513b 100644 --- a/src/plugins/usage_collection/server/collector/collector_set.ts +++ b/src/plugins/usage_collection/server/collector/collector_set.ts @@ -117,7 +117,6 @@ export class CollectorSet { const allReady = collectorsTypesNotReady.length === 0; if (!allReady && this.maximumWaitTimeForAllCollectorsInS >= 0) { - // some collectors return false for isReady() and we still have time left to wait const nowTimestamp = +new Date(); this._waitingForAllCollectorsTimestamp = this._waitingForAllCollectorsTimestamp || nowTimestamp; From ac46d8b944df9f9f7c46c8fecb459e93b38c79dd Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Tue, 3 Nov 2020 16:58:40 -0700 Subject: [PATCH 3/5] updates usage_collection README with more details about isReady --- src/plugins/usage_collection/README.md | 15 ++++++++++----- .../server/collector/collector.ts | 1 + 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/plugins/usage_collection/README.md b/src/plugins/usage_collection/README.md index 3d446689c6e9cf..8c8748ebf9cc89 100644 --- a/src/plugins/usage_collection/README.md +++ b/src/plugins/usage_collection/README.md @@ -8,7 +8,12 @@ To integrate with the telemetry services for usage collection of your feature, t ## Creating and Registering Usage Collector -All you need to provide is a `type` for organizing your fields, `schema` field to define the expected types of usage fields reported, and a `fetch` method for returning your usage data. Then you need to make the Telemetry service aware of the collector by registering it. +Your usage collector needs to provide +- a `type` for organizing your fields, +- `schema` field to define the expected types of usage fields reported, +- a `fetch` method for returning your usage data, and +- an `isReady` method (that returns true or false) for letting the telemetry service know if it needs to wait for any asynchronous action. +Then you need to make the Telemetry service aware of the collector by registering it. 1. Make sure `usageCollection` is in your optional Plugins: @@ -62,7 +67,7 @@ All you need to provide is a `type` for organizing your fields, `schema` field t total: 'long', }, }, - isReady: () => isCollectorFetchReady, // method to return true/false to confirm if the collector is ready for the fetch method to be called. + isReady: () => isCollectorFetchReady, // Method to return `true`/`false` to confirm if the collector is ready for the `fetch` method to be called. fetch: async (collectorFetchContext: CollectorFetchContext) => { @@ -86,9 +91,9 @@ All you need to provide is a `type` for organizing your fields, `schema` field t Some background: - `MY_USAGE_TYPE` can be any string. It usually matches the plugin name. As a safety mechanism, we double check there are no duplicates at the moment of registering the collector. -- isReady() (added in v7.2.0 and v6.8.4) is a way for a usage collector to announce that some async process must finish first before it will return data synchronously. If any collector is not ready, we try fetching again, up to a set amount of time. Once the allocated time interval is up, we collect data from those collectors that are ready and skip any that are not. What this means is that if a collector returns `true` for `isReady` and it actually isn't ready to return data, there won't be telemetry data for that user for the day. Every usage collector owner needs to really think about what it would mean for them if they missed the first few documents when Kibana starts and should implement this function with custom logic. - (e.g. the task manager needs to run a task first). If any collector reports that it is not ready when we try and collect data, we reset a flag to try again up to a specified interval +- `isReady` (added in v7.2.0 and v6.8.4) is a way for a usage collector to announce that some async process must finish first before it will return data synchronously (e.g. the task manager needs to run a task first). If any collector reports that it is not ready when we call its `fetch` method, we reset a flag to try again and, after a set amount of time, collect data from those collectors that are ready and skip any that are not. This means that if a collector returns `true` for `isReady` and it actually isn't ready to return data, there won't be telemetry data from that collector for the day. You should consider what it means if your collector doesn't return data in the first few documents when Kibana starts or, if we should wait for any other reason (e.g. the task manager needs to run your task first). If you need to tell telemetry collection to wait, you should implement this function with custom logic. If your `fetch` method will always return data synchronously or you're intentionally catching errors in your `fetch` method, then you can return true for `isReady` as shown in the example below. + - The `fetch` method needs to support multiple contexts in which it is called. For example, when stats are pulled from a Kibana Metricbeat module, the Beat calls Kibana's stats API to invoke usage collection. In this case, the `fetch` method is called as a result of an HTTP API request and `callCluster` wraps `callWithRequest` or `esClient` wraps `asCurrentUser`, where the request headers are expected to have read privilege on the entire `.kibana' index. The `fetch` method also exposes the saved objects client that will have the correct scope when the collectors' `fetch` method is called. @@ -159,7 +164,7 @@ If any of your properties is an array, the schema definition must follow the con ```ts export const myCollector = makeUsageCollector({ type: 'my_working_collector', - isReady: () => isFetchReadyToReturnData(), + isReady: () => true, // `fetch` doesn't call async methods fetch() { return { my_greeting: 'hello', diff --git a/src/plugins/usage_collection/server/collector/collector.ts b/src/plugins/usage_collection/server/collector/collector.ts index 73febc0183fc55..4986e22ed90248 100644 --- a/src/plugins/usage_collection/server/collector/collector.ts +++ b/src/plugins/usage_collection/server/collector/collector.ts @@ -101,6 +101,7 @@ export class Collector { * @param {Function} options.init (optional) - initialization function * @param {Function} options.fetch - function to query data * @param {Function} options.formatForBulkUpload - optional + * @param {boolean} options.isReady - boolean to indicate collector is ready to report data * @param {Function} options.rest - optional other properties */ constructor( From eca93e8a69e23b4e7b937f3f522b0e01d67c402a Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Wed, 4 Nov 2020 08:08:23 -0700 Subject: [PATCH 4/5] minor changes to README --- src/plugins/usage_collection/README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/plugins/usage_collection/README.md b/src/plugins/usage_collection/README.md index 8c8748ebf9cc89..2ac3de510f8aee 100644 --- a/src/plugins/usage_collection/README.md +++ b/src/plugins/usage_collection/README.md @@ -12,7 +12,8 @@ Your usage collector needs to provide - a `type` for organizing your fields, - `schema` field to define the expected types of usage fields reported, - a `fetch` method for returning your usage data, and -- an `isReady` method (that returns true or false) for letting the telemetry service know if it needs to wait for any asynchronous action. +- an `isReady` method (that returns true or false) for letting the telemetry service know if it needs to wait for any asynchronous action (initialization of clients or other services) before calling the `fetch` method. + Then you need to make the Telemetry service aware of the collector by registering it. 1. Make sure `usageCollection` is in your optional Plugins: @@ -67,7 +68,7 @@ Then you need to make the Telemetry service aware of the collector by registerin total: 'long', }, }, - isReady: () => isCollectorFetchReady, // Method to return `true`/`false` to confirm if the collector is ready for the `fetch` method to be called. + isReady: () => isCollectorFetchReady, // Method to return `true`/`false` or Promise(`true`/`false`) to confirm if the collector is ready for the `fetch` method to be called. fetch: async (collectorFetchContext: CollectorFetchContext) => { @@ -92,7 +93,7 @@ Some background: - `MY_USAGE_TYPE` can be any string. It usually matches the plugin name. As a safety mechanism, we double check there are no duplicates at the moment of registering the collector. -- `isReady` (added in v7.2.0 and v6.8.4) is a way for a usage collector to announce that some async process must finish first before it will return data synchronously (e.g. the task manager needs to run a task first). If any collector reports that it is not ready when we call its `fetch` method, we reset a flag to try again and, after a set amount of time, collect data from those collectors that are ready and skip any that are not. This means that if a collector returns `true` for `isReady` and it actually isn't ready to return data, there won't be telemetry data from that collector for the day. You should consider what it means if your collector doesn't return data in the first few documents when Kibana starts or, if we should wait for any other reason (e.g. the task manager needs to run your task first). If you need to tell telemetry collection to wait, you should implement this function with custom logic. If your `fetch` method will always return data synchronously or you're intentionally catching errors in your `fetch` method, then you can return true for `isReady` as shown in the example below. +- `isReady` (added in v7.2.0 and v6.8.4) is a way for a usage collector to announce that some async process must finish first before it can return data in the `fetch` method (e.g. a client needs to ne initialized, or the task manager needs to run a task first). If any collector reports that it is not ready when we call its `fetch` method, we reset a flag to try again and, after a set amount of time, collect data from those collectors that are ready and skip any that are not. This means that if a collector returns `true` for `isReady` and it actually isn't ready to return data, there won't be telemetry data from that collector in that telemetry report (usually once per day). You should consider what it means if your collector doesn't return data in the first few documents when Kibana starts or, if we should wait for any other reason (e.g. the task manager needs to run your task first). If you need to tell telemetry collection to wait, you should implement this function with custom logic. If your `fetch` method can run without the need of any previous dependencies, then you can return true for `isReady` as shown in the example below. - The `fetch` method needs to support multiple contexts in which it is called. For example, when stats are pulled from a Kibana Metricbeat module, the Beat calls Kibana's stats API to invoke usage collection. In this case, the `fetch` method is called as a result of an HTTP API request and `callCluster` wraps `callWithRequest` or `esClient` wraps `asCurrentUser`, where the request headers are expected to have read privilege on the entire `.kibana' index. The `fetch` method also exposes the saved objects client that will have the correct scope when the collectors' `fetch` method is called. @@ -164,7 +165,7 @@ If any of your properties is an array, the schema definition must follow the con ```ts export const myCollector = makeUsageCollector({ type: 'my_working_collector', - isReady: () => true, // `fetch` doesn't call async methods + isReady: () => true, // `fetch` doesn't require any validation for dependencies to be met fetch() { return { my_greeting: 'hello', From a80b62c727c33be49937a4a708ce26a0525cce2c Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Wed, 4 Nov 2020 08:53:47 -0700 Subject: [PATCH 5/5] Collector class params --- src/plugins/usage_collection/server/collector/collector.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/usage_collection/server/collector/collector.ts b/src/plugins/usage_collection/server/collector/collector.ts index 4986e22ed90248..c04b087d4adf56 100644 --- a/src/plugins/usage_collection/server/collector/collector.ts +++ b/src/plugins/usage_collection/server/collector/collector.ts @@ -72,7 +72,7 @@ export interface CollectorOptions { type: string; init?: Function; /** - * Method to return `true`/`false` to confirm if the collector is ready for the `fetch` method to be called. + * Method to return `true`/`false` or Promise(`true`/`false`) to confirm if the collector is ready for the `fetch` method to be called. */ isReady: () => Promise | boolean; /** @@ -101,7 +101,7 @@ export class Collector { * @param {Function} options.init (optional) - initialization function * @param {Function} options.fetch - function to query data * @param {Function} options.formatForBulkUpload - optional - * @param {boolean} options.isReady - boolean to indicate collector is ready to report data + * @param {Function} options.isReady - method that returns a boolean or Promise of a boolean to indicate the collector is ready to report data * @param {Function} options.rest - optional other properties */ constructor(