hasData service fixes and improvements (#137824)

* fix has data * clean up remote cluster comment Co-authored-by: Kibana Machine <[email protected]>
elastic · Aug 3, 2022 · e2b8525 · e2b8525
1 parent 241e469
commit e2b8525
Show file tree

Hide file tree

Showing 11 changed files with 242 additions and 203 deletions.
diff --git a/src/plugins/data_views/common/constants.ts b/src/plugins/data_views/common/constants.ts
@@ -22,15 +22,16 @@ export const RUNTIME_FIELD_TYPES = [
 ] as const;
 
 /**
- * Used to determine if the instance has some user created index patterns by filtering index patterns
- * that are created and backed only by Fleet server data
- * Should be revised after https://github.com/elastic/kibana/issues/82851 is fixed
- * For more background see: https://github.com/elastic/kibana/issues/107020
+ * Used to optimize on-boarding experience to determine if the instance has some user created data views or data indices/streams by filtering data sources
+ * that are created by default by elastic in ese.
+ * We should somehow prevent creating initial data for the users without their explicit action
+ * instead of relying on these hardcoded assets
  */
 export const DEFAULT_ASSETS_TO_IGNORE = {
-  LOGS_INDEX_PATTERN: 'logs-*',
-  LOGS_DATA_STREAM_TO_IGNORE: 'logs-elastic_agent', // ignore ds created by Fleet server itself
-  ENT_SEARCH_LOGS_DATA_STREAM_TO_IGNORE: 'logs-enterprise_search.api-default', // ignore ds created by enterprise search
+  DATA_STREAMS_TO_IGNORE: [
+    'logs-enterprise_search.api-default', // https://github.com/elastic/kibana/issues/134918
+    `logs-enterprise_search.audit-default`, // https://github.com/elastic/kibana/issues/134918
+  ],
 };
 
 /**

diff --git a/src/plugins/data_views/common/data_views/data_views.ts b/src/plugins/data_views/common/data_views/data_views.ts
@@ -468,7 +468,7 @@ export class DataViewsService {
    * Checks if current user has a user created index pattern ignoring fleet's server default index patterns.
    */
   async hasUserDataView(): Promise<boolean> {
-    return this.apiClient.hasUserIndexPattern();
+    return this.apiClient.hasUserDataView();
   }
 
   /**

diff --git a/src/plugins/data_views/common/types.ts b/src/plugins/data_views/common/types.ts
@@ -319,7 +319,7 @@ export interface GetFieldsOptions {
 
 export interface IDataViewsApiClient {
   getFieldsForWildcard: (options: GetFieldsOptions) => Promise<FieldSpec[]>;
-  hasUserIndexPattern: () => Promise<boolean>;
+  hasUserDataView: () => Promise<boolean>;
 }
 
 export type AggregationRestrictions = Record<

diff --git a/src/plugins/data_views/public/data_views/data_views_api_client.ts b/src/plugins/data_views/public/data_views/data_views_api_client.ts
@@ -63,7 +63,7 @@ export class DataViewsApiClient implements IDataViewsApiClient {
   /**
    * Does a user created data view exist?
    */
-  async hasUserIndexPattern(): Promise<boolean> {
+  async hasUserDataView(): Promise<boolean> {
     const response = await this._request<{ result: boolean }>(
       this._getUrl(['has_user_index_pattern'])
     );

diff --git a/src/plugins/data_views/public/services/has_data.test.ts b/src/plugins/data_views/public/services/has_data.test.ts
@@ -32,11 +32,11 @@ describe('when calling hasData service', () => {
 
     const hasData = new HasData();
     const hasDataService = hasData.start(coreStart);
-    const reponse = hasDataService.hasESData();
+    const response = hasDataService.hasESData();
 
     expect(spy).toHaveBeenCalledTimes(1);
 
-    expect(await reponse).toBe(true);
+    expect(await response).toBe(true);
   });
 
   it('should return false for hasESData when no indices exist', async () => {
@@ -54,11 +54,83 @@ describe('when calling hasData service', () => {
 
     const hasData = new HasData();
     const hasDataService = hasData.start(coreStart);
-    const reponse = hasDataService.hasESData();
+    const response = hasDataService.hasESData();
 
     expect(spy).toHaveBeenCalledTimes(1);
 
-    expect(await reponse).toBe(false);
+    expect(await response).toBe(false);
+  });
+
+  it('should return false for hasESData when only automatically created sources exist', async () => {
+    const coreStart = coreMock.createStart();
+    const http = coreStart.http;
+
+    // Mock getIndices
+    const spy = jest.spyOn(http, 'get').mockImplementation((path: any) =>
+      Promise.resolve({
+        aliases: [],
+        data_streams: path.includes('*:*')
+          ? [] // return empty on remote cluster call
+          : [
+              {
+                name: 'logs-enterprise_search.api-default',
+                timestamp_field: '@timestamp',
+                backing_indices: ['.ds-logs-enterprise_search.api-default-2022.03.07-000001'],
+              },
+            ],
+        indices: [],
+      })
+    );
+
+    const hasData = new HasData();
+    const hasDataService = hasData.start(coreStart);
+    const response = hasDataService.hasESData();
+
+    expect(spy).toHaveBeenCalledTimes(1);
+
+    expect(await response).toBe(false);
+  });
+
+  it('should hit search api in case resolve api throws', async () => {
+    const coreStart = coreMock.createStart();
+    const http = coreStart.http;
+
+    const spyGetIndices = jest
+      .spyOn(http, 'get')
+      .mockImplementation(() => Promise.reject(new Error('oops')));
+
+    const spySearch = jest
+      .spyOn(http, 'post')
+      .mockImplementation(() => Promise.resolve({ total: 10 }));
+    const hasData = new HasData();
+    const hasDataService = hasData.start(coreStart);
+    const response = await hasDataService.hasESData();
+
+    expect(response).toBe(true);
+
+    expect(spyGetIndices).toHaveBeenCalledTimes(1);
+    expect(spySearch).toHaveBeenCalledTimes(1);
+  });
+
+  it('should return false in case search api throws', async () => {
+    const coreStart = coreMock.createStart();
+    const http = coreStart.http;
+
+    const spyGetIndices = jest
+      .spyOn(http, 'get')
+      .mockImplementation(() => Promise.reject(new Error('oops')));
+
+    const spySearch = jest
+      .spyOn(http, 'post')
+      .mockImplementation(() => Promise.reject(new Error('oops')));
+    const hasData = new HasData();
+    const hasDataService = hasData.start(coreStart);
+    const response = await hasDataService.hasESData();
+
+    expect(response).toBe(true);
+
+    expect(spyGetIndices).toHaveBeenCalledTimes(1);
+    expect(spySearch).toHaveBeenCalledTimes(1);
   });
 
   it('should return true for hasDataView when server returns true', async () => {
@@ -75,11 +147,11 @@ describe('when calling hasData service', () => {
 
     const hasData = new HasData();
     const hasDataService = hasData.start(coreStart);
-    const reponse = hasDataService.hasDataView();
+    const response = hasDataService.hasDataView();
 
     expect(spy).toHaveBeenCalledTimes(1);
 
-    expect(await reponse).toBe(true);
+    expect(await response).toBe(true);
   });
 
   it('should return false for hasDataView when server returns false', async () => {
@@ -96,11 +168,27 @@ describe('when calling hasData service', () => {
 
     const hasData = new HasData();
     const hasDataService = hasData.start(coreStart);
-    const reponse = hasDataService.hasDataView();
+    const response = hasDataService.hasDataView();
+
+    expect(spy).toHaveBeenCalledTimes(1);
+
+    expect(await response).toBe(false);
+  });
+
+  it('should return true for hasDataView when server throws an error', async () => {
+    const coreStart = coreMock.createStart();
+    const http = coreStart.http;
+
+    // Mock getIndices
+    const spy = jest.spyOn(http, 'get').mockImplementation(() => Promise.reject(new Error('Oops')));
+
+    const hasData = new HasData();
+    const hasDataService = hasData.start(coreStart);
+    const response = hasDataService.hasDataView();
 
     expect(spy).toHaveBeenCalledTimes(1);
 
-    expect(await reponse).toBe(false);
+    expect(await response).toBe(true);
   });
 
   it('should return false for hasUserDataView when server returns false', async () => {
@@ -117,11 +205,11 @@ describe('when calling hasData service', () => {
 
     const hasData = new HasData();
     const hasDataService = hasData.start(coreStart);
-    const reponse = hasDataService.hasUserDataView();
+    const response = hasDataService.hasUserDataView();
 
     expect(spy).toHaveBeenCalledTimes(1);
 
-    expect(await reponse).toBe(false);
+    expect(await response).toBe(false);
   });
 
   it('should return true for hasUserDataView when server returns true', async () => {
@@ -138,10 +226,26 @@ describe('when calling hasData service', () => {
 
     const hasData = new HasData();
     const hasDataService = hasData.start(coreStart);
-    const reponse = hasDataService.hasUserDataView();
+    const response = hasDataService.hasUserDataView();
+
+    expect(spy).toHaveBeenCalledTimes(1);
+
+    expect(await response).toBe(true);
+  });
+
+  it('should return true for hasUserDataView when server throws an error', async () => {
+    const coreStart = coreMock.createStart();
+    const http = coreStart.http;
+
+    // Mock getIndices
+    const spy = jest.spyOn(http, 'get').mockImplementation(() => Promise.reject(new Error('Oops')));
+
+    const hasData = new HasData();
+    const hasDataService = hasData.start(coreStart);
+    const response = hasDataService.hasUserDataView();
 
     expect(spy).toHaveBeenCalledTimes(1);
 
-    expect(await reponse).toBe(true);
+    expect(await response).toBe(true);
   });
 });
diff --git a/src/plugins/data_views/public/services/has_data.ts b/src/plugins/data_views/public/services/has_data.ts
@@ -14,14 +14,12 @@ import { IndicesResponse, IndicesResponseModified } from '../types';
 export class HasData {
   private removeAliases = (source: IndicesResponseModified): boolean => !source.item.indices;
 
-  private isUserDataIndex = (source: IndicesResponseModified): boolean => {
+  private isUserDataSource = (source: IndicesResponseModified): boolean => {
     // filter out indices that start with `.`
     if (source.name.startsWith('.')) return false;
 
     // filter out sources from DEFAULT_ASSETS_TO_IGNORE
-    if (source.name === DEFAULT_ASSETS_TO_IGNORE.LOGS_DATA_STREAM_TO_IGNORE) return false;
-    if (source.name === DEFAULT_ASSETS_TO_IGNORE.ENT_SEARCH_LOGS_DATA_STREAM_TO_IGNORE)
-      return false;
+    if (DEFAULT_ASSETS_TO_IGNORE.DATA_STREAMS_TO_IGNORE.includes(source.name)) return false; // filter out data streams that we know are created automatically during on-boarding
 
     return true;
   };
@@ -44,14 +42,14 @@ export class HasData {
        * Check to see if a data view exists
        */
       hasDataView: async (): Promise<boolean> => {
-        const dataViewsCheck = await this.findDataViews(http);
+        const dataViewsCheck = await this.hasDataViews(http);
         return dataViewsCheck;
       },
       /**
        * Check to see if user created data views exist
        */
       hasUserDataView: async (): Promise<boolean> => {
-        const userDataViewsCheck = await this.findUserDataViews(http);
+        const userDataViewsCheck = await this.hasUserDataViews(http);
         return userDataViewsCheck;
       },
     };
@@ -106,7 +104,11 @@ export class HasData {
       .then((resp) => {
         return !!(resp && resp.total >= 0);
       })
-      .catch(() => false);
+      .catch((e) => {
+        // eslint-disable-next-line no-console
+        console.warn(`getIndicesViaSearch failed with error, assuming there is data`, e);
+        return true;
+      });
 
   private getIndices = async ({
     http,
@@ -136,7 +138,7 @@ export class HasData {
       showAllIndices: false,
     })
       .then((dataSources: IndicesResponseModified[]) => {
-        return dataSources.some(this.isUserDataIndex);
+        return dataSources.some(this.isUserDataSource);
       })
       .catch(() => this.getIndicesViaSearch({ http, pattern: '*', showAllIndices: false }));
 
@@ -156,22 +158,33 @@ export class HasData {
   private getHasDataViews = async ({ http }: { http: HttpStart }): Promise<HasDataViewsResponse> =>
     http.get<HasDataViewsResponse>(`/internal/data_views/has_data_views`);
 
-  private findDataViews = (http: HttpStart): Promise<boolean> => {
+  private hasDataViews = (http: HttpStart): Promise<boolean> => {
     return this.getHasDataViews({ http })
       .then((response: HasDataViewsResponse) => {
         const { hasDataView } = response;
         return hasDataView;
       })
-      .catch(() => false);
+      .catch((e) => {
+        // eslint-disable-next-line no-console
+        console.warn(`hasDataViews failed with error, assuming there are data views`, e);
+        return true;
+      });
   };
 
-  private findUserDataViews = (http: HttpStart): Promise<boolean> => {
+  private hasUserDataViews = (http: HttpStart): Promise<boolean> => {
     return this.getHasDataViews({ http })
       .then((response: HasDataViewsResponse) => {
         const { hasUserDataView } = response;
         return hasUserDataView;
       })
-      .catch(() => false);
+      .catch((e) => {
+        // eslint-disable-next-line no-console
+        console.warn(
+          `hasUserDataViews failed with error, assuming there are user-created data views`,
+          e
+        );
+        return true;
+      });
   };
 }
 

diff --git a/src/plugins/data_views/server/has_user_data_view.test.ts b/src/plugins/data_views/server/has_user_data_view.test.ts
@@ -0,0 +1,64 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { hasUserDataView } from './has_user_data_view';
+import { elasticsearchServiceMock, savedObjectsClientMock } from '@kbn/core/server/mocks';
+
+describe('hasUserDataView', () => {
+  const esClient = elasticsearchServiceMock.createScopedClusterClient().asCurrentUser;
+  const soClient = savedObjectsClientMock.create();
+
+  beforeEach(() => jest.resetAllMocks());
+
+  it('returns false when there are no data views', async () => {
+    soClient.find.mockResolvedValue({
+      page: 1,
+      per_page: 100,
+      total: 0,
+      saved_objects: [],
+    });
+    expect(await hasUserDataView({ esClient, soClient })).toEqual(false);
+  });
+
+  it('returns true when there are data views', async () => {
+    soClient.find.mockResolvedValue({
+      page: 1,
+      per_page: 100,
+      total: 1,
+      saved_objects: [
+        {
+          id: '1',
+          references: [],
+          type: 'index-pattern',
+          score: 99,
+          attributes: { title: 'my-pattern-*' },
+        },
+      ],
+    });
+    expect(await hasUserDataView({ esClient, soClient })).toEqual(true);
+  });
+
+  it('can shortcut using api internally', async () => {
+    const dataViewsFindResponse = {
+      page: 1,
+      per_page: 100,
+      total: 1,
+      saved_objects: [
+        {
+          id: '1',
+          references: [],
+          type: 'index-pattern',
+          score: 99,
+          attributes: { title: 'my-pattern-*' },
+        },
+      ],
+    };
+    expect(await hasUserDataView({ esClient, soClient }, dataViewsFindResponse)).toEqual(true);
+    expect(soClient.find).not.toBeCalled();
+  });
+});