[MDS] Support for Timeline (opensearch-project#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>
zhyuanqi · Apr 16, 2024 · 73e5d78 · 73e5d78
1 parent 4199162
commit 73e5d78
Show file tree

Hide file tree

Showing 12 changed files with 260 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -88,6 +88,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Multiple Datasource] Add default icon in multi-selectable picker ([#6357](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6357))
 - [Workspace] Add APIs to support plugin state in request ([#6303](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6303))
 - [Workspace] Filter left nav menu items according to the current workspace ([#6234](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6234))
+- [Multiple Datasource] Add multi data source support to Timeline ([#6385](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6385))
 - [Multiple Datasource] Refactor data source selector component to include placeholder and add tests ([#6372](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6372))
 - Replace control characters before logging ([#6402](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6402))
 - [Dynamic Configurations] Improve dynamic configurations by adding cache and simplifying client fetch ([#6364](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6364))

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<DataSourceAttributes>({
+    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
@@ -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
@@ -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<TypeOf<typeof configSchema>>()
       .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
@@ -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
@@ -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
@@ -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
@@ -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;
+}