From f499b4f73fd01a7ca882fa2248197df566d0de5b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 16 Apr 2024 20:01:41 +0000 Subject: [PATCH] [MDS] Support for Timeline (#6385) * Add MDS support to Timeline Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Refactor to function and add unit tests Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Fix typo in args Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Refactor build request to pass unit tests Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Add to CHANGELOG Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Refactor error messages + address comments Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Fix ut Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Change to data source feature Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Fix UT Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Address comments Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> --------- Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> (cherry picked from commit 73e5d78c968f1ab20d28eed6ac6ea772f9c6b620) Signed-off-by: github-actions[bot] # Conflicts: # CHANGELOG.md --- .../opensearch_dashboards.json | 1 + .../helpers/timeline_request_handler.ts | 4 +- .../server/handlers/chain_runner.js | 4 +- .../server/lib/fetch_data_source_id.test.ts | 152 ++++++++++++++++++ .../server/lib/fetch_data_source_id.ts | 42 +++++ .../vis_type_timeline/server/lib/services.ts | 10 ++ .../vis_type_timeline/server/plugin.ts | 10 +- .../vis_type_timeline/server/routes/run.ts | 5 +- .../series_functions/opensearch/index.js | 19 ++- .../opensearch/lib/build_request.js | 3 +- src/plugins/vis_type_timeline/server/types.ts | 15 ++ 11 files changed, 259 insertions(+), 6 deletions(-) create mode 100644 src/plugins/vis_type_timeline/server/lib/fetch_data_source_id.test.ts create mode 100644 src/plugins/vis_type_timeline/server/lib/fetch_data_source_id.ts create mode 100644 src/plugins/vis_type_timeline/server/lib/services.ts diff --git a/src/plugins/vis_type_timeline/opensearch_dashboards.json b/src/plugins/vis_type_timeline/opensearch_dashboards.json index 14b3af415177..c4fa1e8d40fd 100644 --- a/src/plugins/vis_type_timeline/opensearch_dashboards.json +++ b/src/plugins/vis_type_timeline/opensearch_dashboards.json @@ -4,6 +4,7 @@ "opensearchDashboardsVersion": "opensearchDashboards", "server": true, "ui": true, + "optionalPlugins": ["dataSource"], "requiredPlugins": ["visualizations", "data", "expressions"], "requiredBundles": ["opensearchDashboardsUtils", "opensearchDashboardsReact", "visDefaultEditor"] } diff --git a/src/plugins/vis_type_timeline/public/helpers/timeline_request_handler.ts b/src/plugins/vis_type_timeline/public/helpers/timeline_request_handler.ts index 467fef727f29..d7b955f96ef9 100644 --- a/src/plugins/vis_type_timeline/public/helpers/timeline_request_handler.ts +++ b/src/plugins/vis_type_timeline/public/helpers/timeline_request_handler.ts @@ -129,10 +129,12 @@ export function getTimelineRequestHandler({ }); } catch (e) { if (e && e.body) { + const errorTitle = + e.body.attributes && e.body.attributes.title ? ` (${e.body.attributes.title})` : ''; const err = new Error( `${i18n.translate('timeline.requestHandlerErrorTitle', { defaultMessage: 'Timeline request error', - })}: ${e.body.title} ${e.body.message}` + })}${errorTitle}: ${e.body.message}` ); err.stack = e.stack; throw err; diff --git a/src/plugins/vis_type_timeline/server/handlers/chain_runner.js b/src/plugins/vis_type_timeline/server/handlers/chain_runner.js index 75382b73de57..39af9939056f 100644 --- a/src/plugins/vis_type_timeline/server/handlers/chain_runner.js +++ b/src/plugins/vis_type_timeline/server/handlers/chain_runner.js @@ -47,7 +47,9 @@ export default function chainRunner(tlConfig) { let sheet; function throwWithCell(cell, exception) { - throw new Error(' in cell #' + (cell + 1) + ': ' + exception.message); + const e = new Error(exception.message); + e.name = `Expression parse error in cell #${cell + 1}`; + throw e; } // Invokes a modifier function, resolving arguments into series as needed diff --git a/src/plugins/vis_type_timeline/server/lib/fetch_data_source_id.test.ts b/src/plugins/vis_type_timeline/server/lib/fetch_data_source_id.test.ts new file mode 100644 index 000000000000..e5596a001a2d --- /dev/null +++ b/src/plugins/vis_type_timeline/server/lib/fetch_data_source_id.test.ts @@ -0,0 +1,152 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { savedObjectsClientMock } from '../../../../core/server/mocks'; +import { fetchDataSourceIdByName } from './fetch_data_source_id'; +import { OpenSearchFunctionConfig } from '../types'; + +jest.mock('./services', () => ({ + getDataSourceEnabled: jest + .fn() + .mockReturnValueOnce({ enabled: false }) + .mockReturnValue({ enabled: true }), +})); + +describe('fetchDataSourceIdByName()', () => { + const validId = 'some-valid-id'; + const config: OpenSearchFunctionConfig = { + q: null, + metric: null, + split: null, + index: null, + timefield: null, + kibana: null, + opensearchDashboards: null, + interval: null, + }; + const client = savedObjectsClientMock.create(); + client.find = jest.fn().mockImplementation((props) => { + if (props.search === '"No Results With Filter"') { + return Promise.resolve({ + saved_objects: [ + { + id: 'some-non-matching-id', + attributes: { + title: 'No Results With Filter Some Suffix', + }, + }, + ], + }); + } + if (props.search === '"Duplicate Title"') { + return Promise.resolve({ + saved_objects: [ + { + id: 'duplicate-id-1', + attributes: { + title: 'Duplicate Title', + }, + }, + { + id: 'duplicate-id-2', + attributes: { + title: 'Duplicate Title', + }, + }, + ], + }); + } + if (props.search === '"Some Data Source"') { + return Promise.resolve({ + saved_objects: [ + { + id: validId, + attributes: { + title: 'Some Data Source', + }, + }, + ], + }); + } + if (props.search === '"Some Prefix"') { + return Promise.resolve({ + saved_objects: [ + { + id: 'some-id-2', + attributes: { + title: 'Some Prefix B', + }, + }, + { + id: validId, + attributes: { + title: 'Some Prefix', + }, + }, + ], + }); + } + + return Promise.resolve({ saved_objects: [] }); + }); + + it('should return undefined if data_source_name is not present', async () => { + expect(await fetchDataSourceIdByName(config, client)).toBe(undefined); + }); + + it('should return undefined if data_source_name is an empty string', async () => { + expect(await fetchDataSourceIdByName({ ...config, data_source_name: '' }, client)).toBe( + undefined + ); + }); + + it('should throw errors when MDS is disabled', async () => { + await expect( + fetchDataSourceIdByName({ ...config, data_source_name: 'Some Data Source' }, client) + ).rejects.toThrowError( + 'To query from multiple data sources, first enable the data source feature' + ); + }); + + it.each([ + { + dataSourceName: 'Non-existent Data Source', + expectedResultCount: 0, + }, + { + dataSourceName: 'No Results With Filter', + expectedResultCount: 0, + }, + { + dataSourceName: 'Duplicate Title', + expectedResultCount: 2, + }, + ])( + 'should throw errors when non-existent or duplicate data_source_name is provided', + async ({ dataSourceName, expectedResultCount }) => { + await expect( + fetchDataSourceIdByName({ ...config, data_source_name: dataSourceName }, client) + ).rejects.toThrowError( + `Expected exactly 1 result for data_source_name "${dataSourceName}" but got ${expectedResultCount} results` + ); + } + ); + + it.each([ + { + dataSourceName: 'Some Data Source', + }, + { + dataSourceName: 'Some Prefix', + }, + ])( + 'should return valid id when data_source_name exists and is unique', + async ({ dataSourceName }) => { + expect( + await fetchDataSourceIdByName({ ...config, data_source_name: dataSourceName }, client) + ).toBe(validId); + } + ); +}); diff --git a/src/plugins/vis_type_timeline/server/lib/fetch_data_source_id.ts b/src/plugins/vis_type_timeline/server/lib/fetch_data_source_id.ts new file mode 100644 index 000000000000..e3d0d76d23e7 --- /dev/null +++ b/src/plugins/vis_type_timeline/server/lib/fetch_data_source_id.ts @@ -0,0 +1,42 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObjectsClientContract } from 'src/core/server'; +import { DataSourceAttributes } from 'src/plugins/data_source/common/data_sources'; +import { getDataSourceEnabled } from './services'; +import { OpenSearchFunctionConfig } from '../types'; + +export const fetchDataSourceIdByName = async ( + config: OpenSearchFunctionConfig, + client: SavedObjectsClientContract +) => { + if (!config.data_source_name) { + return undefined; + } + + if (!getDataSourceEnabled().enabled) { + throw new Error('To query from multiple data sources, first enable the data source feature'); + } + + const dataSources = await client.find({ + type: 'data-source', + perPage: 100, + search: `"${config.data_source_name}"`, + searchFields: ['title'], + fields: ['id', 'title'], + }); + + const possibleDataSourceIds = dataSources.saved_objects.filter( + (obj) => obj.attributes.title === config.data_source_name + ); + + if (possibleDataSourceIds.length !== 1) { + throw new Error( + `Expected exactly 1 result for data_source_name "${config.data_source_name}" but got ${possibleDataSourceIds.length} results` + ); + } + + return possibleDataSourceIds.pop()?.id; +}; diff --git a/src/plugins/vis_type_timeline/server/lib/services.ts b/src/plugins/vis_type_timeline/server/lib/services.ts new file mode 100644 index 000000000000..13b257622abd --- /dev/null +++ b/src/plugins/vis_type_timeline/server/lib/services.ts @@ -0,0 +1,10 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { createGetterSetter } from '../../../opensearch_dashboards_utils/common'; + +export const [getDataSourceEnabled, setDataSourceEnabled] = createGetterSetter<{ + enabled: boolean; +}>('DataSource'); diff --git a/src/plugins/vis_type_timeline/server/plugin.ts b/src/plugins/vis_type_timeline/server/plugin.ts index d2c7097ac419..f768e51e93d0 100644 --- a/src/plugins/vis_type_timeline/server/plugin.ts +++ b/src/plugins/vis_type_timeline/server/plugin.ts @@ -34,6 +34,7 @@ import { TypeOf, schema } from '@osd/config-schema'; import { RecursiveReadonly } from '@osd/utility-types'; import { deepFreeze } from '@osd/std'; +import { DataSourcePluginSetup } from 'src/plugins/data_source/server'; import { PluginStart } from '../../data/server'; import { CoreSetup, PluginInitializerContext } from '../../../core/server'; import { configSchema } from '../config'; @@ -42,11 +43,16 @@ import { functionsRoute } from './routes/functions'; import { validateOpenSearchRoute } from './routes/validate_es'; import { runRoute } from './routes/run'; import { ConfigManager } from './lib/config_manager'; +import { setDataSourceEnabled } from './lib/services'; const experimentalLabel = i18n.translate('timeline.uiSettings.experimentalLabel', { defaultMessage: 'experimental', }); +export interface TimelinePluginSetupDeps { + dataSource?: DataSourcePluginSetup; +} + export interface TimelinePluginStartDeps { data: PluginStart; } @@ -57,7 +63,7 @@ export interface TimelinePluginStartDeps { export class Plugin { constructor(private readonly initializerContext: PluginInitializerContext) {} - public async setup(core: CoreSetup): void { + public async setup(core: CoreSetup, { dataSource }: TimelinePluginSetupDeps): void { const config = await this.initializerContext.config .create>() .pipe(first()) @@ -80,6 +86,8 @@ export class Plugin { ); }; + setDataSourceEnabled({ enabled: !!dataSource }); + const logger = this.initializerContext.logger.get('timeline'); const router = core.http.createRouter(); diff --git a/src/plugins/vis_type_timeline/server/routes/run.ts b/src/plugins/vis_type_timeline/server/routes/run.ts index ab6a993b4bb5..af1005ebcb8f 100644 --- a/src/plugins/vis_type_timeline/server/routes/run.ts +++ b/src/plugins/vis_type_timeline/server/routes/run.ts @@ -122,7 +122,10 @@ export function runRoute( } else { return response.internalError({ body: { - message: err.toString(), + attributes: { + title: err.name, + }, + message: err.message, }, }); } diff --git a/src/plugins/vis_type_timeline/server/series_functions/opensearch/index.js b/src/plugins/vis_type_timeline/server/series_functions/opensearch/index.js index 8837116bfc02..5192059f3e6d 100644 --- a/src/plugins/vis_type_timeline/server/series_functions/opensearch/index.js +++ b/src/plugins/vis_type_timeline/server/series_functions/opensearch/index.js @@ -34,6 +34,7 @@ import { OPENSEARCH_SEARCH_STRATEGY } from '../../../../data/server'; import Datasource from '../../lib/classes/datasource'; import buildRequest from './lib/build_request'; import toSeriesList from './lib/agg_response_to_series_list'; +import { fetchDataSourceIdByName } from '../../lib/fetch_data_source_id'; export default new Datasource('es', { args: [ @@ -112,6 +113,14 @@ export default new Datasource('es', { defaultMessage: `**DO NOT USE THIS**. It's fun for debugging fit functions, but you really should use the interval picker`, }), }, + { + name: 'data_source_name', + types: ['string', 'null'], // If null, the query will proceed with local cluster + help: i18n.translate('timeline.help.functions.opensearch.args.dataSourceNameHelpText', { + defaultMessage: + 'Specify a data source to query from. This will only work if multiple data sources is enabled', + }), + }, ], help: i18n.translate('timeline.help.functions.opensearchHelpText', { defaultMessage: 'Pull data from an opensearch instance', @@ -148,7 +157,15 @@ export default new Datasource('es', { const opensearchShardTimeout = tlConfig.opensearchShardTimeout; - const body = buildRequest(config, tlConfig, scriptedFields, opensearchShardTimeout); + const dataSourceId = await fetchDataSourceIdByName(config, tlConfig.savedObjectsClient); + + const body = buildRequest( + config, + tlConfig, + scriptedFields, + opensearchShardTimeout, + dataSourceId + ); const deps = (await tlConfig.getStartServices())[1]; diff --git a/src/plugins/vis_type_timeline/server/series_functions/opensearch/lib/build_request.js b/src/plugins/vis_type_timeline/server/series_functions/opensearch/lib/build_request.js index 8436b4dbb04a..90fb7b819a08 100644 --- a/src/plugins/vis_type_timeline/server/series_functions/opensearch/lib/build_request.js +++ b/src/plugins/vis_type_timeline/server/series_functions/opensearch/lib/build_request.js @@ -34,7 +34,7 @@ import { buildAggBody } from './agg_body'; import createDateAgg from './create_date_agg'; import { UI_SETTINGS } from '../../../../../data/server'; -export default function buildRequest(config, tlConfig, scriptedFields, timeout) { +export default function buildRequest(config, tlConfig, scriptedFields, timeout, dataSourceId) { const bool = { must: [] }; const timeFilter = { @@ -105,6 +105,7 @@ export default function buildRequest(config, tlConfig, scriptedFields, timeout) } return { + ...(!!dataSourceId && { dataSourceId }), params: request, }; } diff --git a/src/plugins/vis_type_timeline/server/types.ts b/src/plugins/vis_type_timeline/server/types.ts index f021ffeae00f..2fa3a25c5813 100644 --- a/src/plugins/vis_type_timeline/server/types.ts +++ b/src/plugins/vis_type_timeline/server/types.ts @@ -29,3 +29,18 @@ */ export { TimelineFunctionInterface, TimelineFunctionConfig } from './lib/classes/timeline_function'; + +export interface OpenSearchFunctionConfig { + q: string | null; + metric: string | null; + split: string | null; + index: string | null; + timefield: string | null; + kibana: boolean | null; + opensearchDashboards: boolean | null; + /** + * @deprecated This property should not be set in the Timeline expression. Users should use the interval picker React component instead + */ + interval: string | null; + data_source_name?: string | null; +}