Skip to content

Commit

Permalink
[Discover][ES|QL] Update inspect details for the main ES|QL fetch (el…
Browse files Browse the repository at this point in the history
…astic#180204)

- Closes elastic#175419

## Summary

This PR enables customization for title/description we show in Inspector
flyout for ES|QL requests.

<img width="500" alt="Screenshot 2024-04-09 at 15 25 11"
src="https://github.com/elastic/kibana/assets/1415710/9636458b-496f-4a87-bd1c-8d125f867752">
<img width="500" alt="Screenshot 2024-04-09 at 15 25 20"
src="https://github.com/elastic/kibana/assets/1415710/3efe83c1-49ef-47e5-867f-eae3f00ca488">



### 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
  • Loading branch information
jughosta authored Apr 15, 2024
1 parent 59edae2 commit 35930c0
Show file tree
Hide file tree
Showing 10 changed files with 162 additions and 15 deletions.
15 changes: 14 additions & 1 deletion src/plugins/data/common/query/text_based_query_state_to_ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,28 @@ import {
interface Args extends QueryState {
timeFieldName?: string;
inputQuery?: Query;
titleForInspector?: string;
descriptionForInspector?: string;
}

/**
* Converts QueryState to expression AST
* @param filters array of kibana filters
* @param query kibana query or aggregate query
* @param inputQuery
* @param time kibana time range
* @param dataView
* @param titleForInspector
* @param descriptionForInspector
*/
export function textBasedQueryStateToExpressionAst({
filters,
query,
inputQuery,
time,
timeFieldName,
titleForInspector,
descriptionForInspector,
}: Args) {
const kibana = buildExpressionFunction<ExpressionFunctionKibana>('kibana', {});
let q;
Expand All @@ -51,7 +59,12 @@ export function textBasedQueryStateToExpressionAst({
const mode = getAggregateQueryMode(query);
for (const esMode of ['sql', 'esql']) {
if (mode === esMode && esMode in query) {
const essql = aggregateQueryToAst(query, timeFieldName);
const essql = aggregateQueryToAst({
query,
timeField: timeFieldName,
titleForInspector,
descriptionForInspector,
});

if (essql) {
ast.chain.push(essql);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,35 @@ describe('textBasedQueryStateToAstWithValidation', () => {
})
);
});

it('returns an object with the correct structure for ES|QL', async () => {
const dataView = createStubDataView({
spec: {
id: 'foo',
title: 'foo',
timeFieldName: '@timestamp',
},
});
const actual = await textBasedQueryStateToAstWithValidation({
filters: [],
query: { esql: 'from logs*' },
time: {
from: 'now',
to: 'now+7d',
},
dataView,
titleForInspector: 'Custom title',
descriptionForInspector: 'Custom desc',
});
expect(actual).toHaveProperty(
'chain.2.arguments',
expect.objectContaining({
query: ['from logs*'],
timeField: ['@timestamp'],
locale: ['en'],
titleForInspector: ['Custom title'],
descriptionForInspector: ['Custom desc'],
})
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,28 @@ interface Args extends QueryState {
dataView?: DataView;
inputQuery?: Query;
timeFieldName?: string;
titleForInspector?: string;
descriptionForInspector?: string;
}

/**
* Converts QueryState to expression AST
* @param filters array of kibana filters
* @param query kibana query or aggregate query
* @param inputQuery
* @param time kibana time range
* @param dataView
* @param titleForInspector
* @param descriptionForInspector
*/
export async function textBasedQueryStateToAstWithValidation({
filters,
query,
inputQuery,
time,
dataView,
titleForInspector,
descriptionForInspector,
}: Args) {
let ast;
if (query && isOfAggregateQueryType(query)) {
Expand All @@ -37,6 +45,8 @@ export async function textBasedQueryStateToAstWithValidation({
inputQuery,
time,
timeFieldName: dataView?.timeFieldName,
titleForInspector,
descriptionForInspector,
});
}
return ast;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,37 @@ import { aggregateQueryToAst } from './aggregate_query_to_ast';

describe('aggregateQueryToAst', () => {
it('should return a function', () => {
expect(aggregateQueryToAst({ esql: 'from foo' })).toHaveProperty('type', 'function');
expect(aggregateQueryToAst({ query: { esql: 'from foo' } })).toHaveProperty('type', 'function');
});

it('should forward arguments', () => {
expect(aggregateQueryToAst({ esql: 'from foo' }, 'baz')).toHaveProperty(
expect(
aggregateQueryToAst({
query: { esql: 'from foo' },
timeField: 'baz',
})
).toHaveProperty(
'arguments',
expect.objectContaining({
query: ['from foo'],
timeField: ['baz'],
})
);

expect(
aggregateQueryToAst({
query: { esql: 'from foo' },
timeField: 'baz',
titleForInspector: 'Custom title',
descriptionForInspector: 'Custom desc',
})
).toHaveProperty(
'arguments',
expect.objectContaining({
query: ['from foo'],
timeField: ['baz'],
titleForInspector: ['Custom title'],
descriptionForInspector: ['Custom desc'],
})
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,17 @@ import { AggregateQuery } from '../../query';
import { EssqlExpressionFunctionDefinition } from './essql';
import { EsqlExpressionFunctionDefinition } from './esql';

export const aggregateQueryToAst = (
query: AggregateQuery,
timeField?: string
): undefined | ExpressionAstFunction => {
export const aggregateQueryToAst = ({
query,
timeField,
titleForInspector,
descriptionForInspector,
}: {
query: AggregateQuery;
timeField?: string;
titleForInspector?: string;
descriptionForInspector?: string;
}): undefined | ExpressionAstFunction => {
if ('sql' in query) {
return buildExpressionFunction<EssqlExpressionFunctionDefinition>('essql', {
query: query.sql,
Expand All @@ -26,6 +33,8 @@ export const aggregateQueryToAst = (
query: query.esql,
timeField,
locale: i18n.getLocale(),
titleForInspector,
descriptionForInspector,
}).toAst();
}
};
39 changes: 31 additions & 8 deletions src/plugins/data/common/search/expressions/esql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ interface Arguments {
// timezone?: string;
timeField?: string;
locale?: string;

/**
* Requests' meta for showing in Inspector
*/
titleForInspector?: string;
descriptionForInspector?: string;
}

export type EsqlExpressionFunctionDefinition = ExpressionFunctionDefinition<
Expand Down Expand Up @@ -107,10 +113,24 @@ export const getEsqlFn = ({ getStartDependencies }: EsqlFnArguments) => {
defaultMessage: 'The locale to use.',
}),
},
titleForInspector: {
aliases: ['titleForInspector'],
types: ['string'],
help: i18n.translate('data.search.esql.titleForInspector.help', {
defaultMessage: 'The title to show in Inspector.',
}),
},
descriptionForInspector: {
aliases: ['descriptionForInspector'],
types: ['string'],
help: i18n.translate('data.search.esql.descriptionForInspector.help', {
defaultMessage: 'The description to show in Inspector.',
}),
},
},
fn(
input,
{ query, /* timezone, */ timeField, locale },
{ query, /* timezone, */ timeField, locale, titleForInspector, descriptionForInspector },
{ abortSignal, inspectorAdapters, getKibanaRequest }
) {
return defer(() =>
Expand Down Expand Up @@ -158,14 +178,17 @@ export const getEsqlFn = ({ getStartDependencies }: EsqlFnArguments) => {
}

const request = inspectorAdapters.requests.start(
i18n.translate('data.search.dataRequest.title', {
defaultMessage: 'Data',
}),
{
description: i18n.translate('data.search.es_search.dataRequest.description', {
defaultMessage:
'This request queries Elasticsearch to fetch the data for the visualization.',
titleForInspector ??
i18n.translate('data.search.dataRequest.title', {
defaultMessage: 'Data',
}),
{
description:
descriptionForInspector ??
i18n.translate('data.search.es_search.dataRequest.description', {
defaultMessage:
'This request queries Elasticsearch to fetch the data for the visualization.',
}),
},
startTime
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
import { pluck } from 'rxjs';
import { lastValueFrom } from 'rxjs';
import { i18n } from '@kbn/i18n';
import { Query, AggregateQuery, Filter } from '@kbn/es-query';
import type { Adapters } from '@kbn/inspector-plugin/common';
import type { DataPublicPluginStart } from '@kbn/data-plugin/public';
Expand Down Expand Up @@ -41,6 +42,12 @@ export function fetchTextBased(
time: timeRange,
dataView,
inputQuery,
titleForInspector: i18n.translate('discover.inspectorTextBasedRequestTitle', {
defaultMessage: 'Table',
}),
descriptionForInspector: i18n.translate('discover.inspectorTextBasedRequestDescription', {
defaultMessage: 'This request queries Elasticsearch to fetch results for the table.',
}),
})
.then((ast) => {
if (ast) {
Expand Down
18 changes: 18 additions & 0 deletions test/functional/apps/discover/group4/_esql_view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const testSubjects = getService('testSubjects');
const monacoEditor = getService('monacoEditor');
const security = getService('security');
const inspector = getService('inspector');
const retry = getService('retry');
const browser = getService('browser');
const find = getService('find');
Expand Down Expand Up @@ -256,6 +257,23 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
});
});

describe('inspector', () => {
beforeEach(async () => {
await PageObjects.common.navigateToApp('discover');
await PageObjects.timePicker.setDefaultAbsoluteRange();
});

it('shows Discover and Lens requests in Inspector', async () => {
await PageObjects.discover.selectTextBaseLang();
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.discover.waitUntilSearchingHasFinished();
await inspector.open();
const requestNames = await inspector.getRequestNames();
expect(requestNames).to.contain('Table');
expect(requestNames).to.contain('Visualization');
});
});

describe('query history', () => {
beforeEach(async () => {
await PageObjects.common.navigateToApp('discover');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -724,12 +724,18 @@ describe('Textbased Data Source', () => {
},
Object {
"arguments": Object {
"descriptionForInspector": Array [
"This request queries Elasticsearch to fetch the data for the visualization.",
],
"locale": Array [
"en",
],
"query": Array [
"FROM foo",
],
"titleForInspector": Array [
"Visualization",
],
},
"function": "esql",
"type": "function",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { i18n } from '@kbn/i18n';
import { Ast } from '@kbn/interpreter';
import { textBasedQueryStateToExpressionAst } from '@kbn/data-plugin/common';
import type { OriginalColumn } from '../../../common/types';
Expand Down Expand Up @@ -44,6 +45,13 @@ function getExpressionForLayer(
const textBasedQueryToAst = textBasedQueryStateToExpressionAst({
query: layer.query,
timeFieldName,
titleForInspector: i18n.translate('xpack.lens.inspectorTextBasedRequestDataTitle', {
defaultMessage: 'Visualization',
}),
descriptionForInspector: i18n.translate('xpack.lens.inspectorTextBasedRequestDescription', {
defaultMessage:
'This request queries Elasticsearch to fetch the data for the visualization.',
}),
});

textBasedQueryToAst.chain.push({
Expand Down

0 comments on commit 35930c0

Please sign in to comment.