Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] [Discover] Address chart performance issues for non-transformational and non-time-based ES|QL queries (#200583) #201185

Merged
merged 1 commit into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/plugins/unified_histogram/public/__mocks__/lens_vis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const getLensVisMock = async ({
breakdownField,
dataView,
allSuggestions,
hasHistogramSuggestionForESQL,
isTransformationalESQL,
table,
}: {
filters: QueryParams['filters'];
Expand All @@ -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;
Expand All @@ -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,
});
Expand Down
142 changes: 113 additions & 29 deletions src/plugins/unified_histogram/public/chart/chart.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async function mountComponent({
isPlainRecord,
hasDashboardPermissions,
isChartLoading,
hasHistogramSuggestionForESQL,
isTransformationalESQL,
}: {
customToggle?: ReactElement;
noChart?: boolean;
Expand All @@ -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 } } }))
Expand Down Expand Up @@ -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: '',
Expand All @@ -108,7 +110,7 @@ async function mountComponent({
breakdownField: undefined,
columns: [],
allSuggestions,
hasHistogramSuggestionForESQL,
isTransformationalESQL,
})
).lensService;

Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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();
Expand Down
15 changes: 0 additions & 15 deletions src/plugins/unified_histogram/public/layout/helpers.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('LensVisService suggestions', () => {
],
isPlainRecord: true,
allSuggestions: [],
hasHistogramSuggestionForESQL: false,
isTransformationalESQL: true,
});

expect(lensVis.currentSuggestionContext?.type).toBe(UnifiedHistogramSuggestionType.unsupported);
Expand Down Expand Up @@ -115,7 +115,7 @@ describe('LensVisService suggestions', () => {
],
isPlainRecord: true,
allSuggestions: [],
hasHistogramSuggestionForESQL: true,
isTransformationalESQL: false,
});

expect(lensVis.currentSuggestionContext?.type).toBe(
Expand Down Expand Up @@ -153,7 +153,7 @@ describe('LensVisService suggestions', () => {
],
isPlainRecord: true,
allSuggestions: [],
hasHistogramSuggestionForESQL: true,
isTransformationalESQL: false,
});

expect(lensVis.currentSuggestionContext?.type).toBe(
Expand Down Expand Up @@ -191,7 +191,7 @@ describe('LensVisService suggestions', () => {
],
isPlainRecord: true,
allSuggestions: [],
hasHistogramSuggestionForESQL: true,
isTransformationalESQL: true,
});

expect(lensVis.currentSuggestionContext?.type).toBe(UnifiedHistogramSuggestionType.unsupported);
Expand Down Expand Up @@ -225,7 +225,7 @@ describe('LensVisService suggestions', () => {
],
isPlainRecord: true,
allSuggestions: [],
hasHistogramSuggestionForESQL: true,
isTransformationalESQL: false,
});

expect(lensVis.currentSuggestionContext?.type).toBe(
Expand Down Expand Up @@ -276,7 +276,7 @@ describe('LensVisService suggestions', () => {
],
isPlainRecord: true,
allSuggestions: allSuggestionsMock,
hasHistogramSuggestionForESQL: true,
isTransformationalESQL: false,
});

expect(lensVis.currentSuggestionContext?.type).toBe(
Expand Down Expand Up @@ -307,7 +307,7 @@ describe('LensVisService suggestions', () => {
],
isPlainRecord: true,
allSuggestions: [],
hasHistogramSuggestionForESQL: true,
isTransformationalESQL: false,
});

expect(lensVis.currentSuggestionContext?.type).toBe(
Expand Down
Loading