hasData service fixes and improvements (#137824) (#138043)

* fix has data

* clean up remote cluster comment

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit e2b85258bf)

Co-authored-by: Anton Dosov <anton.dosov@elastic.co>

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
+  ],
 };
 
 /**

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();
   }
 
   /**

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<

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'])
     );

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 reponse).toBe(false);
+    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 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 reponse).toBe(true);
+    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 response).toBe(true);
   });
 });

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;
+      });
   };
 }
 

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();
+  });
+});

src/plugins/data_views/server/has_user_data_view.ts

@@ -12,14 +12,13 @@ import {
   SavedObjectsFindResponse,
 } from '@kbn/core/server';
 import { DataViewSavedObjectAttrs } from '../common/data_views';
-import { DEFAULT_ASSETS_TO_IGNORE } from '../common/constants';
 
 interface Deps {
   esClient: ElasticsearchClient;
   soClient: SavedObjectsClientContract;
 }
 
-export const getIndexPattern = async ({
+export const getDataViews = async ({
   soClient,
 }: Deps): Promise<SavedObjectsFindResponse<DataViewSavedObjectAttrs, unknown>> =>
   soClient.find<DataViewSavedObjectAttrs>({
@@ -30,31 +29,27 @@ export const getIndexPattern = async ({
     perPage: 100,
   });
 
-export const hasUserIndexPattern = async (
+/**
+ * Checks if user has access to any data view,
+ * excluding those that are automatically created by ese (hardcoded)
+ * @param esClient
+ * @param soClient
+ * @param dataViews
+ */
+export const hasUserDataView = async (
   { esClient, soClient }: Deps,
-  indexPatterns?: SavedObjectsFindResponse<DataViewSavedObjectAttrs, unknown>
+  dataViews?: SavedObjectsFindResponse<DataViewSavedObjectAttrs, unknown>
 ): Promise<boolean> => {
-  if (!indexPatterns) {
-    indexPatterns = await getIndexPattern({ esClient, soClient });
+  if (!dataViews) {
+    dataViews = await getDataViews({ esClient, soClient });
   }
 
-  if (indexPatterns.total > 0) {
+  if (dataViews.total === 0) {
+    return false;
+  } else {
+    // filter here data views that we know are not created by user during on-boarding for smoother on-boarding experience
+    // currently there is no such data views,
+
     return true;
   }
-
-  const resolveResponse = await esClient.indices.resolveIndex({
-    name: `${DEFAULT_ASSETS_TO_IGNORE.LOGS_INDEX_PATTERN}`,
-  });
-
-  if (resolveResponse) {
-    if (resolveResponse.indices.length > 0) return true;
-
-    const hasAnyNonDefaultFleetDataStreams = resolveResponse.data_streams.some(
-      (ds) =>
-        ds.name !== DEFAULT_ASSETS_TO_IGNORE.LOGS_DATA_STREAM_TO_IGNORE &&
-        ds.name !== DEFAULT_ASSETS_TO_IGNORE.ENT_SEARCH_LOGS_DATA_STREAM_TO_IGNORE
-    );
-    if (hasAnyNonDefaultFleetDataStreams) return true;
-  }
-  return false;
 };

src/plugins/data_views/server/has_user_index_pattern.test.ts

@@ -1,138 +0,0 @@
-/*
- * 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 { hasUserIndexPattern } from './has_user_index_pattern';
-import { elasticsearchServiceMock, savedObjectsClientMock } from '@kbn/core/server/mocks';
-
-describe('hasUserIndexPattern', () => {
-  const esClient = elasticsearchServiceMock.createScopedClusterClient().asCurrentUser;
-  const soClient = savedObjectsClientMock.create();
-
-  beforeEach(() => jest.resetAllMocks());
-
-  it('returns false when there are no index patterns', async () => {
-    soClient.find.mockResolvedValue({
-      page: 1,
-      per_page: 100,
-      total: 0,
-      saved_objects: [],
-    });
-    expect(await hasUserIndexPattern({ esClient, soClient })).toEqual(false);
-  });
-
-  describe('when no index patterns exist', () => {
-    beforeEach(() => {
-      soClient.find.mockResolvedValue({
-        page: 1,
-        per_page: 100,
-        total: 0,
-        saved_objects: [],
-      });
-    });
-
-    it('calls indices.resolveIndex for the index patterns', async () => {
-      esClient.indices.resolveIndex.mockResponse({
-        indices: [],
-        data_streams: [],
-        aliases: [],
-      });
-      await hasUserIndexPattern({ esClient, soClient });
-      expect(esClient.indices.resolveIndex).toHaveBeenCalledWith({
-        name: 'logs-*',
-      });
-    });
-
-    it('returns false if no data_streams exists', async () => {
-      esClient.indices.resolveIndex.mockResponse({
-        indices: [],
-        data_streams: [],
-        aliases: [],
-      });
-      expect(await hasUserIndexPattern({ esClient, soClient })).toEqual(false);
-    });
-
-    it('returns true if any index exists', async () => {
-      esClient.indices.resolveIndex.mockResponse({
-        indices: [{ name: 'logs', attributes: [] }],
-        data_streams: [],
-        aliases: [],
-      });
-      expect(await hasUserIndexPattern({ esClient, soClient })).toEqual(true);
-    });
-
-    it('returns false if only logs-elastic_agent data stream exists', async () => {
-      esClient.indices.resolveIndex.mockResponse({
-        indices: [],
-        data_streams: [
-          {
-            name: 'logs-elastic_agent',
-            timestamp_field: '@timestamp',
-            backing_indices: ['.ds-logs-elastic_agent'],
-          },
-        ],
-        aliases: [],
-      });
-      expect(await hasUserIndexPattern({ esClient, soClient })).toEqual(false);
-    });
-
-    it('returns false if only logs-enterprise_search.api-default data stream exists', async () => {
-      esClient.indices.resolveIndex.mockResponse({
-        indices: [],
-        data_streams: [
-          {
-            name: 'logs-enterprise_search.api-default',
-            timestamp_field: '@timestamp',
-            backing_indices: ['.ds-logs-enterprise_search.api-default-2022.03.07-000001'],
-          },
-        ],
-        aliases: [],
-      });
-      expect(await hasUserIndexPattern({ esClient, soClient })).toEqual(false);
-    });
-
-    it('returns true if any other data stream exists', async () => {
-      esClient.indices.resolveIndex.mockResponse({
-        indices: [],
-        data_streams: [
-          {
-            name: 'other',
-            timestamp_field: '@timestamp',
-            backing_indices: ['.ds-other'],
-          },
-        ],
-        aliases: [],
-      });
-      expect(await hasUserIndexPattern({ esClient, soClient })).toEqual(true);
-    });
-
-    it('returns true if any other data stream exists with logs-enterprise_search.api-default and logs-elastic_agent', async () => {
-      esClient.indices.resolveIndex.mockResponse({
-        indices: [],
-        data_streams: [
-          {
-            name: 'other',
-            timestamp_field: '@timestamp',
-            backing_indices: ['.ds-other'],
-          },
-          {
-            name: 'logs-enterprise_search.api-default',
-            timestamp_field: '@timestamp',
-            backing_indices: ['.ds-logs-enterprise_search.api-default-2022.03.07-000001'],
-          },
-          {
-            name: 'logs-elastic_agent',
-            timestamp_field: '@timestamp',
-            backing_indices: ['.ds-logs-elastic_agent'],
-          },
-        ],
-        aliases: [],
-      });
-      expect(await hasUserIndexPattern({ esClient, soClient })).toEqual(true);
-    });
-  });
-});

src/plugins/data_views/server/index_patterns_api_client.ts

@@ -10,7 +10,7 @@ import { ElasticsearchClient, SavedObjectsClientContract } from '@kbn/core/serve
 import { GetFieldsOptions, IDataViewsApiClient } from '../common/types';
 import { DataViewMissingIndices } from '../common/lib';
 import { IndexPatternsFetcher } from './fetcher';
-import { hasUserIndexPattern } from './has_user_index_pattern';
+import { hasUserDataView } from './has_user_data_view';
 
 export class IndexPatternsApiServer implements IDataViewsApiClient {
   esClient: ElasticsearchClient;
@@ -52,8 +52,8 @@ export class IndexPatternsApiServer implements IDataViewsApiClient {
   /**
    * Is there a user created data view?
    */
-  async hasUserIndexPattern() {
-    return hasUserIndexPattern({
+  async hasUserDataView() {
+    return hasUserDataView({
       esClient: this.esClient,
       soClient: this.savedObjectsClient,
     });

src/plugins/data_views/server/routes/has_data_views.ts

@@ -7,7 +7,7 @@
  */
 
 import { IRouter } from '@kbn/core/server';
-import { getIndexPattern, hasUserIndexPattern } from '../has_user_index_pattern';
+import { getDataViews, hasUserDataView } from '../has_user_data_view';
 
 export const registerHasDataViewsRoute = (router: IRouter): void => {
   router.get(
@@ -19,11 +19,11 @@ export const registerHasDataViewsRoute = (router: IRouter): void => {
       const core = await ctx.core;
       const savedObjectsClient = core.savedObjects.client;
       const elasticsearchClient = core.elasticsearch.client.asCurrentUser;
-      const dataViews = await getIndexPattern({
+      const dataViews = await getDataViews({
         esClient: elasticsearchClient,
         soClient: savedObjectsClient,
       });
-      const checkDataPattern = await hasUserIndexPattern(
+      const hasUserDataViewResult = await hasUserDataView(
         {
           esClient: elasticsearchClient,
           soClient: savedObjectsClient,
@@ -31,8 +31,8 @@ export const registerHasDataViewsRoute = (router: IRouter): void => {
         dataViews
       );
       const response = {
-        hasDataView: !!dataViews.total,
-        hasUserDataView: !!checkDataPattern,
+        hasDataView: dataViews.total > 0,
+        hasUserDataView: hasUserDataViewResult,
       };
       return res.ok({ body: response });
     }