[Discover] Address chart performance issues for non-transformational …

…and non-time-based ES|QL queries (elastic#200583) - Closes elastic#199608 ## Summary This PR changes the logic around when suggestions from lens API are used. Previously for non-transformational query and non-time-based data it would try to render one of lens suggestions supplying chart data via `table` prop. Now, it would not render any chart. Before: - Data view mode => Static histogram configuration - ES|QL mode and non-transformational query => _**Gets lens suggestions.**_ If histogram chart is not possible, **_takes the first lens suggestion for rendering the chart_** - ES|QL mode and transformational query => Gets lens suggestions. Takes the first lens suggestion for rendering the chart. After: - Data view mode => Static histogram configuration (same) - ES|QL mode and non-transformational query => ~~_**Gets lens suggestions.**_~~ If histogram chart is not possible, **_renders nothing_** (updated) - ES|QL mode and transformational query => Gets lens suggestions. Takes the first lens suggestion for rendering the chart. (same) ### Testing As per originally reported case: 1. `node scripts/es_archiver --kibana-url=http://elastic:changeme@localhost:5601 --es-url=http://elastic:changeme@localhost:9200 load test/functional/fixtures/es_archiver/many_fields` 2. Navigate to Discover, switch to ES|QL mode and enter `FROM indices-stats | LIMIT 10` 3. No chart is expected. Also there should be no regression for elastic#195863 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Davis McPhee <[email protected]> Co-authored-by: Stratoula Kalafateli <[email protected]>
neptunian · Nov 21, 2024 · b9439e6 · b9439e6
1 parent 6f0bda8
commit b9439e6
Show file tree

Hide file tree

Showing 7 changed files with 197 additions and 106 deletions.
diff --git a/src/plugins/unified_histogram/public/__mocks__/lens_vis.ts b/src/plugins/unified_histogram/public/__mocks__/lens_vis.ts
@@ -32,7 +32,7 @@ export const getLensVisMock = async ({
   breakdownField,
   dataView,
   allSuggestions,
-  hasHistogramSuggestionForESQL,
+  isTransformationalESQL,
   table,
 }: {
   filters: QueryParams['filters'];
@@ -44,7 +44,7 @@ export const getLensVisMock = async ({
   timeRange?: TimeRange | null;
   breakdownField: DataViewField | undefined;
   allSuggestions?: Suggestion[];
-  hasHistogramSuggestionForESQL?: boolean;
+  isTransformationalESQL?: boolean;
   table?: Datatable;
 }): Promise<{
   lensService: LensVisService;
@@ -60,7 +60,9 @@ export const getLensVisMock = async ({
           if ('query' in context && context.query === query) {
             return allSuggestions;
           }
-          return hasHistogramSuggestionForESQL ? [histogramESQLSuggestionMock] : [];
+          return !isTransformationalESQL && dataView.isTimeBased()
+            ? [histogramESQLSuggestionMock]
+            : [];
         }
       : lensApi.suggestions,
   });

diff --git a/src/plugins/unified_histogram/public/chart/chart.test.tsx b/src/plugins/unified_histogram/public/chart/chart.test.tsx
@@ -44,7 +44,7 @@ async function mountComponent({
   isPlainRecord,
   hasDashboardPermissions,
   isChartLoading,
-  hasHistogramSuggestionForESQL,
+  isTransformationalESQL,
 }: {
   customToggle?: ReactElement;
   noChart?: boolean;
@@ -57,7 +57,7 @@ async function mountComponent({
   isPlainRecord?: boolean;
   hasDashboardPermissions?: boolean;
   isChartLoading?: boolean;
-  hasHistogramSuggestionForESQL?: boolean;
+  isTransformationalESQL?: boolean;
 } = {}) {
   (searchSourceInstanceMock.fetch$ as jest.Mock).mockImplementation(
     jest.fn().mockReturnValue(of({ rawResponse: { hits: { total: noHits ? 0 : 2 } } }))
@@ -87,7 +87,9 @@ async function mountComponent({
 
   const requestParams = {
     query: isPlainRecord
-      ? { esql: 'from logs | limit 10' }
+      ? isTransformationalESQL
+        ? { esql: 'from logs | limit 10 | stats var0 = avg(bytes) by extension' }
+        : { esql: 'from logs | limit 10' }
       : {
           language: 'kuery',
           query: '',
@@ -108,7 +110,7 @@ async function mountComponent({
       breakdownField: undefined,
       columns: [],
       allSuggestions,
-      hasHistogramSuggestionForESQL,
+      isTransformationalESQL,
     })
   ).lensService;
 
@@ -211,12 +213,111 @@ describe('Chart', () => {
     expect(component.find('[data-test-subj="unifiedHistogramChart"]').exists()).toBeTruthy();
   });
 
-  test('render when is text based and not timebased', async () => {
-    const component = await mountComponent({ isPlainRecord: true, dataView: dataViewMock });
+  test('should render when is text based, transformational and non-time-based', async () => {
+    const component = await mountComponent({
+      isPlainRecord: true,
+      dataView: dataViewMock,
+      isTransformationalESQL: true,
+    });
     expect(
       component.find('[data-test-subj="unifiedHistogramToggleChartButton"]').exists()
     ).toBeTruthy();
     expect(component.find('[data-test-subj="unifiedHistogramChart"]').exists()).toBeTruthy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramEditFlyoutVisualization"]').exists()
+    ).toBeTruthy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramSaveVisualization"]').exists()
+    ).toBeTruthy();
+  });
+
+  test('should not render when is text based, non-transformational and non-time-based', async () => {
+    const component = await mountComponent({
+      isPlainRecord: true,
+      dataView: dataViewMock,
+      isTransformationalESQL: false,
+    });
+    expect(
+      component.find('[data-test-subj="unifiedHistogramToggleChartButton"]').exists()
+    ).toBeTruthy();
+    expect(component.find('[data-test-subj="unifiedHistogramChart"]').exists()).toBeFalsy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramEditFlyoutVisualization"]').exists()
+    ).toBeFalsy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramSaveVisualization"]').exists()
+    ).toBeFalsy();
+  });
+
+  test('should not render when is text based, non-transformational, non-time-based and suggestions are available', async () => {
+    const component = await mountComponent({
+      allSuggestions: allSuggestionsMock,
+      isPlainRecord: true,
+      dataView: dataViewMock,
+      isTransformationalESQL: false,
+    });
+    expect(
+      component.find('[data-test-subj="unifiedHistogramToggleChartButton"]').exists()
+    ).toBeTruthy();
+    expect(component.find('[data-test-subj="unifiedHistogramChart"]').exists()).toBeFalsy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramEditFlyoutVisualization"]').exists()
+    ).toBeFalsy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramSaveVisualization"]').exists()
+    ).toBeFalsy();
+  });
+
+  test('should render when is text based, non-transformational and time-based', async () => {
+    const component = await mountComponent({
+      isPlainRecord: true,
+      isTransformationalESQL: false,
+    });
+    expect(
+      component.find('[data-test-subj="unifiedHistogramToggleChartButton"]').exists()
+    ).toBeTruthy();
+    expect(component.find('[data-test-subj="unifiedHistogramChart"]').exists()).toBeTruthy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramEditFlyoutVisualization"]').exists()
+    ).toBeTruthy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramSaveVisualization"]').exists()
+    ).toBeTruthy();
+  });
+
+  test('should render when is text based, transformational and time-based', async () => {
+    const component = await mountComponent({
+      isPlainRecord: true,
+      isTransformationalESQL: true,
+    });
+    expect(
+      component.find('[data-test-subj="unifiedHistogramToggleChartButton"]').exists()
+    ).toBeTruthy();
+    expect(component.find('[data-test-subj="unifiedHistogramChart"]').exists()).toBeTruthy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramEditFlyoutVisualization"]').exists()
+    ).toBeTruthy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramSaveVisualization"]').exists()
+    ).toBeTruthy();
+  });
+
+  test('should not render when is text based, transformational and no suggestions available', async () => {
+    const component = await mountComponent({
+      allSuggestions: [],
+      isPlainRecord: true,
+      isTransformationalESQL: true,
+    });
+    expect(
+      component.find('[data-test-subj="unifiedHistogramToggleChartButton"]').exists()
+    ).toBeTruthy();
+    expect(component.find('[data-test-subj="unifiedHistogramChart"]').exists()).toBeFalsy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramEditFlyoutVisualization"]').exists()
+    ).toBeFalsy();
+    expect(
+      component.find('[data-test-subj="unifiedHistogramSaveVisualization"]').exists()
+    ).toBeFalsy();
   });
 
   test('render progress bar when text based and request is loading', async () => {
@@ -267,42 +368,25 @@ describe('Chart', () => {
     expect(component.find(BreakdownFieldSelector).exists()).toBeFalsy();
   });
 
-  it('should render the edit on the fly button when chart is visible and suggestions exist', async () => {
-    const component = await mountComponent({
-      allSuggestions: allSuggestionsMock,
-      isPlainRecord: true,
-    });
-    expect(
-      component.find('[data-test-subj="unifiedHistogramEditFlyoutVisualization"]').exists()
-    ).toBeTruthy();
-  });
-
-  it('should not render the edit on the fly button when chart is visible and suggestions dont exist', async () => {
+  it('should not render the save button when text-based and the dashboard save by value permissions are false', async () => {
     const component = await mountComponent({
       allSuggestions: [],
-      hasHistogramSuggestionForESQL: false,
-      isPlainRecord: true,
-    });
-    expect(
-      component.find('[data-test-subj="unifiedHistogramEditFlyoutVisualization"]').exists()
-    ).toBeFalsy();
-  });
-
-  it('should render the save button when chart is visible and suggestions exist', async () => {
-    const component = await mountComponent({
-      allSuggestions: allSuggestionsMock,
+      isTransformationalESQL: false,
       isPlainRecord: true,
+      hasDashboardPermissions: false,
     });
+    expect(component.find('[data-test-subj="unifiedHistogramChart"]').exists()).toBeTruthy();
     expect(
       component.find('[data-test-subj="unifiedHistogramSaveVisualization"]').exists()
-    ).toBeTruthy();
+    ).toBeFalsy();
   });
 
   it('should not render the save button when the dashboard save by value permissions are false', async () => {
     const component = await mountComponent({
       allSuggestions: allSuggestionsMock,
       hasDashboardPermissions: false,
     });
+    expect(component.find('[data-test-subj="unifiedHistogramChart"]').exists()).toBeTruthy();
     expect(
       component.find('[data-test-subj="unifiedHistogramSaveVisualization"]').exists()
     ).toBeFalsy();

diff --git a/src/plugins/unified_histogram/public/layout/helpers.ts b/src/plugins/unified_histogram/public/layout/helpers.ts
diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.attributes.test.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.attributes.test.ts
@@ -765,7 +765,7 @@ describe('LensVisService attributes', () => {
       columns: [],
       isPlainRecord: true,
       allSuggestions: [], // none available
-      hasHistogramSuggestionForESQL: true,
+      isTransformationalESQL: false,
     });
     expect(lensVis.visContext?.attributes.state.query).toStrictEqual(histogramQuery);
   });

diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts
@@ -86,7 +86,7 @@ describe('LensVisService suggestions', () => {
       ],
       isPlainRecord: true,
       allSuggestions: [],
-      hasHistogramSuggestionForESQL: false,
+      isTransformationalESQL: true,
     });
 
     expect(lensVis.currentSuggestionContext?.type).toBe(UnifiedHistogramSuggestionType.unsupported);
@@ -115,7 +115,7 @@ describe('LensVisService suggestions', () => {
       ],
       isPlainRecord: true,
       allSuggestions: [],
-      hasHistogramSuggestionForESQL: true,
+      isTransformationalESQL: false,
     });
 
     expect(lensVis.currentSuggestionContext?.type).toBe(
@@ -153,7 +153,7 @@ describe('LensVisService suggestions', () => {
       ],
       isPlainRecord: true,
       allSuggestions: [],
-      hasHistogramSuggestionForESQL: true,
+      isTransformationalESQL: false,
     });
 
     expect(lensVis.currentSuggestionContext?.type).toBe(
@@ -191,7 +191,7 @@ describe('LensVisService suggestions', () => {
       ],
       isPlainRecord: true,
       allSuggestions: [],
-      hasHistogramSuggestionForESQL: true,
+      isTransformationalESQL: true,
     });
 
     expect(lensVis.currentSuggestionContext?.type).toBe(UnifiedHistogramSuggestionType.unsupported);
@@ -225,7 +225,7 @@ describe('LensVisService suggestions', () => {
       ],
       isPlainRecord: true,
       allSuggestions: [],
-      hasHistogramSuggestionForESQL: true,
+      isTransformationalESQL: false,
     });
 
     expect(lensVis.currentSuggestionContext?.type).toBe(
@@ -276,7 +276,7 @@ describe('LensVisService suggestions', () => {
       ],
       isPlainRecord: true,
       allSuggestions: allSuggestionsMock,
-      hasHistogramSuggestionForESQL: true,
+      isTransformationalESQL: false,
     });
 
     expect(lensVis.currentSuggestionContext?.type).toBe(
@@ -307,7 +307,7 @@ describe('LensVisService suggestions', () => {
       ],
       isPlainRecord: true,
       allSuggestions: [],
-      hasHistogramSuggestionForESQL: true,
+      isTransformationalESQL: false,
     });
 
     expect(lensVis.currentSuggestionContext?.type).toBe(