[Telemetry] Fix bug introduced in elastic#55859 (elastic#57441)

* [Telemetry] Refactor to TS Monitoring telemetry_collection files * [Telemetry] Fetch side documents generated by monitoring to build up the Kibana plugins stats * Update x-pack/legacy/plugins/monitoring/server/telemetry_collection/get_beats_stats.ts Co-Authored-By: Ahmad Bamieh <[email protected]> * Fix import in test file * Move mocha tests to Jest + TS * Fix extended telemetry in functional tests * Fix types * [Telemetry] Fix bug in usage_collector wrong function override * Revert integration tests (change not needed) Co-authored-by: Ahmad Bamieh <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
afharo · Feb 17, 2020 · 10d4549 · 10d4549
1 parent 2c7fba2
commit 10d4549
Show file tree

Hide file tree

Showing 18 changed files with 673 additions and 282 deletions.
diff --git a/src/legacy/core_plugins/telemetry/server/collection_manager.ts b/src/legacy/core_plugins/telemetry/server/collection_manager.ts
@@ -41,8 +41,8 @@ export interface StatsCollectionConfig {
   usageCollection: UsageCollectionSetup;
   callCluster: CallCluster;
   server: any;
-  start: string;
-  end: string;
+  start: string | number;
+  end: string | number;
 }
 
 export type StatsGetterConfig = UnencryptedStatsGetterConfig | EncryptedStatsGetterConfig;
@@ -193,7 +193,7 @@ export class TelemetryCollectionManager {
         }
       } catch (err) {
         statsCollectionConfig.server.log(
-          ['debu', 'telemetry', 'collection'],
+          ['debug', 'telemetry', 'collection'],
           `Failed to collect any usage with registered collections.`
         );
         // swallow error to try next collection;

diff --git a/src/plugins/usage_collection/server/collector/collector.ts b/src/plugins/usage_collection/server/collector/collector.ts
@@ -85,7 +85,7 @@ export class Collector<T = unknown, U = T> {
   protected defaultFormatterForBulkUpload(result: T) {
     return {
       type: this.type,
-      payload: result,
+      payload: (result as unknown) as U,
     };
   }
 }
diff --git a/src/plugins/usage_collection/server/collector/usage_collector.ts b/src/plugins/usage_collection/server/collector/usage_collector.ts
@@ -24,14 +24,14 @@ export class UsageCollector<T = unknown, U = { usage: { [key: string]: T } }> ex
   T,
   U
 > {
-  protected defaultUsageFormatterForBulkUpload(result: T) {
+  protected defaultFormatterForBulkUpload(result: T) {
     return {
       type: KIBANA_STATS_TYPE,
-      payload: {
+      payload: ({
         usage: {
           [this.type]: result,
         },
-      },
+      } as unknown) as U,
     };
   }
 }
diff --git a/...tests__/fixtures/beats_stats_results.json → ...mocks__/fixtures/beats_stats_results.json b/...tests__/fixtures/beats_stats_results.json → ...mocks__/fixtures/beats_stats_results.json
diff --git a/...etry_collection/__tests__/create_query.js → ...telemetry_collection/create_query.test.ts b/...etry_collection/__tests__/create_query.js → ...telemetry_collection/create_query.test.ts
@@ -4,14 +4,13 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import expect from '@kbn/expect';
 import { set } from 'lodash';
-import { createTypeFilter, createQuery } from '../create_query.js';
+import { createTypeFilter, createQuery } from './create_query';
 
 describe('Create Type Filter', () => {
   it('Builds a type filter syntax', () => {
     const typeFilter = createTypeFilter('my_type');
-    expect(typeFilter).to.eql({
+    expect(typeFilter).toStrictEqual({
       bool: { should: [{ term: { _type: 'my_type' } }, { term: { type: 'my_type' } }] },
     });
   });
@@ -36,7 +35,7 @@ describe('Create Query', () => {
         ],
       },
     };
-    expect(result).to.be.eql(expected);
+    expect(result).toStrictEqual(expected);
   });
 
   it('Uses `type` option to add type filter with minimal fields', () => {
@@ -47,7 +46,7 @@ describe('Create Query', () => {
       { term: { _type: 'test-type-yay' } },
       { term: { type: 'test-type-yay' } },
     ]);
-    expect(result).to.be.eql(expected);
+    expect(result).toStrictEqual(expected);
   });
 
   it('Uses `type` option to add type filter with all other option fields', () => {
@@ -77,6 +76,6 @@ describe('Create Query', () => {
         ],
       },
     };
-    expect(result).to.be.eql(expected);
+    expect(result).toStrictEqual(expected);
   });
 });
diff --git a/...rver/telemetry_collection/create_query.js → ...rver/telemetry_collection/create_query.ts b/...rver/telemetry_collection/create_query.js → ...rver/telemetry_collection/create_query.ts
@@ -4,7 +4,6 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { defaults } from 'lodash';
 import moment from 'moment';
 
 /*
@@ -14,14 +13,26 @@ import moment from 'moment';
  * TODO: this backwards compatibility helper will only be supported for 5.x-6. This
  * function should be removed in 7.0
  */
-export const createTypeFilter = type => {
+export const createTypeFilter = (type: string) => {
   return {
     bool: {
       should: [{ term: { _type: type } }, { term: { type } }],
     },
   };
 };
 
+export interface QueryOptions {
+  type?: string;
+  filters?: object[];
+  clusterUuid?: string;
+  start?: string | number;
+  end?: string | number;
+}
+
+interface RangeFilter {
+  range: { [key: string]: { format?: string; gte?: string | number; lte?: string | number } };
+}
+
 /*
  * Creates the boilerplace for querying monitoring data, including filling in
  * start time and end time, and injecting additional filters.
@@ -36,9 +47,8 @@ export const createTypeFilter = type => {
  * @param {Date} options.start - numeric timestamp (optional)
  * @param {Date} options.end - numeric timestamp (optional)
  */
-export function createQuery(options) {
-  options = defaults(options, { filters: [] });
-  const { type, clusterUuid, start, end, filters } = options;
+export function createQuery(options: QueryOptions) {
+  const { type, clusterUuid, start, end, filters = [] } = options;
 
   let typeFilter;
   if (type) {
@@ -50,7 +60,7 @@ export function createQuery(options) {
     clusterUuidFilter = { term: { cluster_uuid: clusterUuid } };
   }
 
-  let timeRangeFilter;
+  let timeRangeFilter: RangeFilter | undefined;
   if (start || end) {
     timeRangeFilter = {
       range: {

diff --git a/...try_collection/__tests__/get_all_stats.js → ...elemetry_collection/get_all_stats.test.ts b/...try_collection/__tests__/get_all_stats.js → ...elemetry_collection/get_all_stats.test.ts
@@ -4,12 +4,13 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import expect from '@kbn/expect';
 import sinon from 'sinon';
-import { addStackStats, getAllStats, handleAllStats } from '../get_all_stats';
+import { addStackStats, getAllStats, handleAllStats } from './get_all_stats';
+import { ESClusterStats } from './get_es_stats';
+import { KibanaStats } from './get_kibana_stats';
+import { ClustersHighLevelStats } from './get_high_level_stats';
 
-// FAILING: https://github.com/elastic/kibana/issues/51371
-describe.skip('get_all_stats', () => {
+describe('get_all_stats', () => {
   const size = 123;
   const start = 0;
   const end = 1;
@@ -100,9 +101,6 @@ describe.skip('get_all_stats', () => {
 
   describe('getAllStats', () => {
     it('returns clusters', async () => {
-      const clusterUuidsResponse = {
-        aggregations: { cluster_uuids: { buckets: [{ key: 'a' }] } },
-      };
       const esStatsResponse = {
         hits: {
           hits: [{ _id: 'a', _source: { cluster_uuid: 'a' } }],
@@ -177,15 +175,25 @@ describe.skip('get_all_stats', () => {
       callCluster
         .withArgs('search')
         .onCall(0)
-        .returns(Promise.resolve(clusterUuidsResponse))
-        .onCall(1)
         .returns(Promise.resolve(esStatsResponse))
-        .onCall(2)
+        .onCall(1)
         .returns(Promise.resolve(kibanaStatsResponse))
+        .onCall(2)
+        .returns(Promise.resolve(logstashStatsResponse))
         .onCall(3)
-        .returns(Promise.resolve(logstashStatsResponse));
+        .returns(Promise.resolve({})) // Beats stats
+        .onCall(4)
+        .returns(Promise.resolve({})); // Beats state
 
-      expect(await getAllStats({ callCluster, server, start, end })).to.eql(allClusters);
+      expect(
+        await getAllStats([{ clusterUuid: 'a' }], {
+          callCluster: callCluster as any,
+          usageCollection: {} as any,
+          server,
+          start,
+          end,
+        })
+      ).toStrictEqual(allClusters);
     });
 
     it('returns empty clusters', async () => {
@@ -195,21 +203,33 @@ describe.skip('get_all_stats', () => {
 
       callCluster.withArgs('search').returns(Promise.resolve(clusterUuidsResponse));
 
-      expect(await getAllStats({ callCluster, server, start, end })).to.eql([]);
+      expect(
+        await getAllStats([], {
+          callCluster: callCluster as any,
+          usageCollection: {} as any,
+          server,
+          start,
+          end,
+        })
+      ).toStrictEqual([]);
     });
   });
 
   describe('handleAllStats', () => {
     it('handles response', () => {
-      const clusters = handleAllStats(esClusters, { kibana: kibanaStats, logstash: logstashStats });
+      const clusters = handleAllStats(esClusters as ESClusterStats[], {
+        kibana: (kibanaStats as unknown) as KibanaStats,
+        logstash: (logstashStats as unknown) as ClustersHighLevelStats,
+        beats: {},
+      });
 
-      expect(clusters).to.eql(expectedClusters);
+      expect(clusters).toStrictEqual(expectedClusters);
     });
 
     it('handles no clusters response', () => {
-      const clusters = handleAllStats([], {});
+      const clusters = handleAllStats([], {} as any);
 
-      expect(clusters).to.have.length(0);
+      expect(clusters).toHaveLength(0);
     });
   });
 
@@ -230,9 +250,9 @@ describe.skip('get_all_stats', () => {
         },
       };
 
-      addStackStats(cluster, stats, 'xyz');
+      addStackStats(cluster as ESClusterStats, stats, 'xyz');
 
-      expect(cluster.stack_stats.xyz).to.be(stats.a);
+      expect((cluster as any).stack_stats.xyz).toStrictEqual(stats.a);
     });
   });
 });
diff --git a/...ver/telemetry_collection/get_all_stats.js → ...ver/telemetry_collection/get_all_stats.ts b/...ver/telemetry_collection/get_all_stats.js → ...ver/telemetry_collection/get_all_stats.ts
@@ -6,22 +6,26 @@
 
 import { get, set, merge } from 'lodash';
 
+import { StatsGetter } from 'src/legacy/core_plugins/telemetry/server/collection_manager';
 import { LOGSTASH_SYSTEM_ID, KIBANA_SYSTEM_ID, BEATS_SYSTEM_ID } from '../../common/constants';
-import { getElasticsearchStats } from './get_es_stats';
-import { getKibanaStats } from './get_kibana_stats';
+import { getElasticsearchStats, ESClusterStats } from './get_es_stats';
+import { getKibanaStats, KibanaStats } from './get_kibana_stats';
 import { getBeatsStats } from './get_beats_stats';
 import { getHighLevelStats } from './get_high_level_stats';
 
+type PromiseReturnType<T extends (...args: any[]) => any> = ReturnType<T> extends Promise<infer R>
+  ? R
+  : T;
+
 /**
  * Get statistics for all products joined by Elasticsearch cluster.
+ * Returns the array of clusters joined with the Kibana and Logstash instances.
  *
- * @param {Object} server The Kibana server instance used to call ES as the internal user
- * @param {function} callCluster The callWithRequest or callWithInternalUser handler
- * @param {Date} start The starting range to request data
- * @param {Date} end The ending range to request data
- * @return {Promise} The array of clusters joined with the Kibana and Logstash instances.
  */
-export async function getAllStats(clustersDetails, { server, callCluster, start, end }) {
+export const getAllStats: StatsGetter = async (
+  clustersDetails,
+  { server, callCluster, start, end }
+) => {
   const clusterUuids = clustersDetails.map(clusterDetails => clusterDetails.clusterUuid);
 
   const [esClusters, kibana, logstash, beats] = await Promise.all([
@@ -32,7 +36,7 @@ export async function getAllStats(clustersDetails, { server, callCluster, start,
   ]);
 
   return handleAllStats(esClusters, { kibana, logstash, beats });
-}
+};
 
 /**
  * Combine the statistics from the stack to create "cluster" stats that associate all products together based on the cluster
@@ -41,9 +45,21 @@ export async function getAllStats(clustersDetails, { server, callCluster, start,
  * @param {Array} clusters The Elasticsearch clusters
  * @param {Object} kibana The Kibana instances keyed by Cluster UUID
  * @param {Object} logstash The Logstash nodes keyed by Cluster UUID
- * @return {Array} The clusters joined with the Kibana and Logstash instances under each cluster's {@code stack_stats}.
+ *
+ * Returns the clusters joined with the Kibana and Logstash instances under each cluster's {@code stack_stats}.
  */
-export function handleAllStats(clusters, { kibana, logstash, beats }) {
+export function handleAllStats(
+  clusters: ESClusterStats[],
+  {
+    kibana,
+    logstash,
+    beats,
+  }: {
+    kibana: KibanaStats;
+    logstash: PromiseReturnType<typeof getHighLevelStats>;
+    beats: PromiseReturnType<typeof getBeatsStats>;
+  }
+) {
   return clusters.map(cluster => {
     // if they are using Kibana or Logstash, then add it to the cluster details under cluster.stack_stats
     addStackStats(cluster, kibana, KIBANA_SYSTEM_ID);
@@ -62,8 +78,12 @@ export function handleAllStats(clusters, { kibana, logstash, beats }) {
  * @param {Object} allProductStats Product stats, keyed by Cluster UUID
  * @param {String} product The product name being added (e.g., 'kibana' or 'logstash')
  */
-export function addStackStats(cluster, allProductStats, product) {
-  const productStats = get(allProductStats, cluster.cluster_uuid);
+export function addStackStats<T extends { [clusterUuid: string]: K }, K>(
+  cluster: ESClusterStats & { stack_stats?: { [product: string]: K } },
+  allProductStats: T,
+  product: string
+) {
+  const productStats = allProductStats[cluster.cluster_uuid];
 
   // Don't add it if they're not using (or configured to report stats) this product for this cluster
   if (productStats) {
@@ -75,12 +95,20 @@ export function addStackStats(cluster, allProductStats, product) {
   }
 }
 
-export function mergeXPackStats(cluster, allProductStats, path, product) {
+export function mergeXPackStats<T extends { [clusterUuid: string]: unknown }>(
+  cluster: ESClusterStats & { stack_stats?: { xpack?: { [product: string]: unknown } } },
+  allProductStats: T,
+  path: string,
+  product: string
+) {
   const productStats = get(allProductStats, cluster.cluster_uuid + '.' + path);
 
   if (productStats || productStats === 0) {
-    if (!get(cluster, 'stack_stats.xpack')) {
-      set(cluster, 'stack_stats.xpack', {});
+    if (!cluster.stack_stats) {
+      cluster.stack_stats = {};
+    }
+    if (!cluster.stack_stats.xpack) {
+      cluster.stack_stats.xpack = {};
     }
 
     const mergeStats = {};