[Discover-next] add query enhancements plugin as a core plugin

[kavilla]

NOTE: Will update this and move some of the stuff that was added if it was a 3rd party plugin and will add justifications based on this rubric:

#7081 (comment)

Also need to swap the default values around.

Description

Introduces the addition of a new core plugin called Query Enhancements

This plugin seems to be focused on enhancing query capabilities, particularly in areas like PPL (Piped Processing Language), SQL, and natural language query assistance.

Plugin Configuration and Setup

The plugin is set up with the necessary configuration files:

• src/plugins/query_enhancements/opensearch_dashboards.json: Defines the plugin's metadata and dependencies.
• src/plugins/query_enhancements/common/config.ts: Contains the configuration schema for the plugin.
• src/plugins/query_enhancements/public/plugin.tsx: The main plugin file that sets up the public-facing part of the plugin.
• src/plugins/query_enhancements/server/plugin.ts: The server-side setup for the plugin.

Query Assist Feature

A significant part of the plugin is dedicated to a "Query Assist" feature:

• src/plugins/query_enhancements/public/query_assist/: This directory contains components, hooks, and utilities for the Query Assist feature.
• src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx: A React component for the Query Assist bar.
• src/plugins/query_enhancements/server/routes/query_assist/: Server-side routing for Query Assist functionality.

Search Interceptors

The plugin introduces several search interceptors:

• src/plugins/query_enhancements/public/search/ppl_search_interceptor.ts
• src/plugins/query_enhancements/public/search/sql_search_interceptor.ts
• src/plugins/query_enhancements/public/search/sql_async_search_interceptor.ts

These interceptors likely modify or enhance the search functionality for different query languages.

Data Source Connection

There's a new feature for data source connections:

• src/plugins/query_enhancements/public/data_source_connection/: This directory contains components and services for managing data source connections.

Server-side Search Strategies

The plugin implements server-side search strategies for different query types:

• src/plugins/query_enhancements/server/search/ppl_search_strategy.ts
• src/plugins/query_enhancements/server/search/sql_search_strategy.ts
• src/plugins/query_enhancements/server/search/sql_async_search_strategy.ts

Modifications to Existing Files

There are some modifications to existing files outside the new plugin directory:

• src/plugins/data/public/index.ts: Exports new types and interfaces.
• src/plugins/data/public/ui/index.ts: Exports new UI components.
• src/plugins/data/public/ui/query_editor/index.tsx: Modifies the QueryEditor component.

Testing

The plugin includes a comprehensive test setup:

• src/plugins/query_enhancements/test/: Contains Jest configuration and setup files for testing.

Acknowledgements

Contains work by @sejli @joshuali925 @Swiddis @paulstn @mengweieric
Thx @ashwin-pc for the description summary

Issues Related

#6072
#6074
#6075

Changelog

• feat: add query enhancements plugin as a core plugin

Check List

• All tests pass
  • yarn test:jest
  • yarn test:jest_integration
• New functionality includes testing.
• New functionality has been documented.
• Update CHANGELOG.md
• Commits are signed per the DCO using --signoff

[xluo-aws]

@kavilla Do we allow OSD core call plugin APIs?

[ashwin-pc]

Overall no major blockers. Had a few comments though

[ashwin-pc]

This Readme could use more data.

[ashwin-pc]

Can we add tests for these?

[ashwin-pc]

This is fine for now, but consider moving these into a custom hook with tests once the dataset work is complete

[kavilla]

yeah this should be tossed once data set is added

[ashwin-pc]

Do we need so many obsrvables? Cant we combine the whole connection state into one observable and share it?

e.g.

import { BehaviorSubject, Observable } from 'rxjs';
import { CoreStart, CoreSetup } from 'opensearch-dashboards/public';
import { Connection, ConnectionsServiceDeps } from '../../types';
import { API } from '../../../common';

interface ConnectionsState {
  isDataSourceEnabled: boolean;
  selectedConnection?: Connection;
  uiService?: DataPublicPluginStart['ui'];
}

export class ConnectionsService {
  private http: CoreSetup['http'];
  private savedObjects: CoreStart['savedObjects'];
  private state$ = new BehaviorSubject<ConnectionsState>({
    isDataSourceEnabled: false,
    selectedConnection: undefined,
    uiService: undefined,
  });

  constructor(deps: ConnectionsServiceDeps) {
    deps.startServices.then(([coreStart, depsStart]) => {
      this.http = deps.http;
      this.savedObjects = coreStart.savedObjects;
      this.updateState({
        uiService: depsStart.data.ui,
        isDataSourceEnabled: depsStart.dataSource?.dataSourceEnabled || false,
      });
    });
  }

  private updateState(newState: Partial<ConnectionsState>) {
    this.state$.next({ ...this.state$.value, ...newState });
  }

  getState$(): Observable<ConnectionsState> {
    return this.state$.asObservable();
  }

  getSavedObjects() {
    return this.savedObjects;
  }

  async getConnections(): Promise<Connection[]> {
    return this.http.get(API.DATA_SOURCE.CONNECTIONS);
  }

  async getConnectionById(id: string): Promise<Connection> {
    return this.http.get(`${API.DATA_SOURCE.CONNECTIONS}/${id}`);
  }

  setSelectedConnection(connection: Connection | undefined) {
    this.updateState({ selectedConnection: connection });
  }
}

And use it like:

const ConnectionsBar: React.FC<ConnectionsProps> = ({ connectionsService, toasts }) => {
  const [state, setState] = useState<ConnectionsState>({
    isDataSourceEnabled: false,
    selectedConnection: undefined,
    uiService: undefined,
  });

  useEffect(() => {
    const subscription = connectionsService.getState$().subscribe(setState);
    return () => subscription.unsubscribe();
  }, [connectionsService]);

  // ... rest of the logic
}

[ashwin-pc]

This service also needs unit tests

[ashwin-pc]

Tests?

[ashwin-pc]

Also i dont think this should be an extention. The dataset selection should be a core part of the querybar. We dont need to make this an extension. I see the query bar being split into 2 parts:

• Query Header
• Query Body

The dataset selector and other connection related stuff can then go into the QueryBar header.

e.g.

// Generic QueryContainer Component
const QueryContainer = ({ title, children }) => {
  const [isExpanded, setIsExpanded] = useState(false);
  return (
    <div>
      <QueryHeader onToggleExpand={() => setIsExpanded(!isExpanded)} isExpanded={isExpanded} title={title} />
      {isExpanded && <QueryBody>{children}</QueryBody>}
    </div>
  );
};

// Generic QueryHeader Component
const QueryHeader = ({ onToggleExpand, isExpanded, title }) => (
  <div>
    <button onClick={onToggleExpand}>{isExpanded ? 'v' : '>'} {title}</button>
    <DatasetSelector />
    <LanguageSelector />
    <QueryActions />
  </div>
);

// Generic QueryBody Component
const QueryBody = ({ children }) => (
  <div>
    {children}
  </div>
);

[ashwin-pc]

Do we still need this?

[ashwin-pc]

How does this work?

[ashwin-pc]

Why are we introducing new routes?

[ashwin-pc]

We should write tests for the search strategies

[ashwin-pc]

Whats this for?

[kavilla]

@kavilla Do we allow OSD core call plugin APIs?

yeah. it's actually as simple data.search

[ashwin-pc]

@kavilla Do we allow OSD core call plugin APIs?

yeah. it's actually as simple data.search

Wait, what now? where did this happen?

[ashwin-pc]

@joshuali925 can you help fix this soo so that we dont ignore these tests for 2.16?

[ashwin-pc]

@joshuali925 Same goes here

[ashwin-pc]

My comments will be addressed in a fast follow. Approve=ing this for now so that dependency features can be merged in. The comments here are just for code quality and tests

[joshuali925]

diff to fix some code and tests in query assist

diff --git a/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx b/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx
index 84ac854c980..e87e74ce299 100644
--- a/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx
+++ b/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx
@@ -45,7 +45,7 @@ export const QueryAssistBar: React.FC<QueryAssistInputProps> = (props) => {
     const subscription = props.connectionsService
       .getSelectedConnection$()
       .subscribe((connection) => {
-        dataSourceIdRef.current = connection?.id;
+        dataSourceIdRef.current = connection?.dataSource.id;
       });
     return () => subscription.unsubscribe();
   }, [props.connectionsService]);
diff --git a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx
index ea568959152..41fc36dd71c 100644
--- a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx
+++ b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx
@@ -24,13 +24,10 @@ const httpMock = coreSetupMock.http;
 
 jest.mock('../components', () => ({
   QueryAssistBar: jest.fn(() => <div>QueryAssistBar</div>),
-}));
-
-jest.mock('../components/query_assist_banner', () => ({
   QueryAssistBanner: jest.fn(() => <div>QueryAssistBanner</div>),
 }));
 
-describe.skip('CreateExtension', () => {
+describe('CreateExtension', () => {
   afterEach(() => {
     jest.clearAllMocks();
   });
diff --git a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx
index bd990b57f5a..8d51cb8998c 100644
--- a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx
+++ b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx
@@ -28,7 +28,7 @@ const getAvailableLanguages$ = (
   connectionsService.getSelectedConnection$().pipe(
     distinctUntilChanged(),
     switchMap(async (connection) => {
-      const dataSourceId = connection?.id;
+      const dataSourceId = connection?.dataSource.id;
       const cached = availableLanguagesByDataSource.get(dataSourceId);
       if (cached !== undefined) return cached;
       const languages = await http
@@ -102,6 +102,7 @@ interface QueryAssistWrapperProps {
 
 const QueryAssistWrapper: React.FC<QueryAssistWrapperProps> = (props) => {
   const [visible, setVisible] = useState(false);
 
   useEffect(() => {
     let mounted = true;
@@ -112,6 +113,7 @@ const QueryAssistWrapper: React.FC<QueryAssistWrapperProps> = (props) => {
       props.http
     ).subscribe((languages) => {
       const available = languages.includes(props.dependencies.language);
       if (mounted) setVisible(props.invert ? !available : available);
     });
 
diff --git a/src/plugins/query_enhancements/server/routes/query_assist/agents.test.ts b/src/plugins/query_enhancements/server/routes/query_assist/agents.test.ts
index f22bdb14836..5e31191c9c9 100644
--- a/src/plugins/query_enhancements/server/routes/query_assist/agents.test.ts
+++ b/src/plugins/query_enhancements/server/routes/query_assist/agents.test.ts
@@ -5,25 +5,29 @@
 
 import { ApiResponse } from '@opensearch-project/opensearch';
 import { ResponseError } from '@opensearch-project/opensearch/lib/errors';
+import { loggerMock } from '@osd/logging/target/mocks';
 import { RequestHandlerContext } from 'src/core/server';
-// // eslint-disable-next-line @osd/eslint/no-restricted-paths
-// import { CoreRouteHandlerContext } from 'src/core/server/core_route_handler_context';
 import { coreMock } from '../../../../../core/server/mocks';
-import { loggerMock } from '@osd/logging/target/mocks';
 import { getAgentIdByConfig, requestAgentByConfig } from './agents';
 
-describe.skip('Agents helper functions', () => {
-  // const coreContext = new CoreRouteHandlerContext(
-  //   coreMock.createInternalStart(),
-  //   httpServerMock.createOpenSearchDashboardsRequest()
-  // );
+describe('Agents helper functions', () => {
   const coreContext = coreMock.createRequestHandlerContext();
   const client = coreContext.opensearch.client.asCurrentUser;
   const mockedTransport = client.transport.request as jest.Mock;
   const context: RequestHandlerContext = {
     core: coreContext,
+    // @ts-ignore
     dataSource: jest.fn(),
-    query_assist: { dataSourceEnabled: false, logger: loggerMock.create() },
+    query_assist: {
+      dataSourceEnabled: false,
+      logger: loggerMock.create(),
+      configPromise: Promise.resolve({
+        enabled: true,
+        queryAssist: {
+          supportedLanguages: [{ language: 'PPL', agentConfig: 'testConfig' }],
+        },
+      }),
+    },
   };
 
   afterEach(() => {
diff --git a/src/plugins/query_enhancements/server/types.ts b/src/plugins/query_enhancements/server/types.ts
index 2ab716b9892..1a5d962e705 100644
--- a/src/plugins/query_enhancements/server/types.ts
+++ b/src/plugins/query_enhancements/server/types.ts
@@ -51,7 +51,10 @@ export interface FacetRequest {
   };
 }
 
-declare module '../../../core/server' {
+// TODO declare it in core changes the type for every reference. we only need
+// this declaration in query_enhancements, but it doesn't seem possible
+// https://github.com/opensearch-project/OpenSearch-Dashboards/issues/4274
+/* declare module '../../../core/server' {
   interface RequestHandlerContext {
     query_assist: {
       logger: Logger;
@@ -64,4 +67,4 @@ declare module '../../../core/server' {
       dataSourceEnabled: boolean;
     };
   }
-}
+} */

[kavilla]

l will breakdown the fast follows as issues and re-link them here.

[kavilla]

diff to fix some code and tests in query assist

diff --git a/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx b/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx
index 84ac854c980..e87e74ce299 100644
--- a/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx
+++ b/src/plugins/query_enhancements/public/query_assist/components/query_assist_bar.tsx
@@ -45,7 +45,7 @@ export const QueryAssistBar: React.FC<QueryAssistInputProps> = (props) => {
     const subscription = props.connectionsService
       .getSelectedConnection$()
       .subscribe((connection) => {
-        dataSourceIdRef.current = connection?.id;
+        dataSourceIdRef.current = connection?.dataSource.id;
       });
     return () => subscription.unsubscribe();
   }, [props.connectionsService]);
diff --git a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx
index ea568959152..41fc36dd71c 100644
--- a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx
+++ b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx
@@ -24,13 +24,10 @@ const httpMock = coreSetupMock.http;
 
 jest.mock('../components', () => ({
   QueryAssistBar: jest.fn(() => <div>QueryAssistBar</div>),
-}));
-
-jest.mock('../components/query_assist_banner', () => ({
   QueryAssistBanner: jest.fn(() => <div>QueryAssistBanner</div>),
 }));
 
-describe.skip('CreateExtension', () => {
+describe('CreateExtension', () => {
   afterEach(() => {
     jest.clearAllMocks();
   });
diff --git a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx
index bd990b57f5a..8d51cb8998c 100644
--- a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx
+++ b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx
@@ -28,7 +28,7 @@ const getAvailableLanguages$ = (
   connectionsService.getSelectedConnection$().pipe(
     distinctUntilChanged(),
     switchMap(async (connection) => {
-      const dataSourceId = connection?.id;
+      const dataSourceId = connection?.dataSource.id;
       const cached = availableLanguagesByDataSource.get(dataSourceId);
       if (cached !== undefined) return cached;
       const languages = await http
@@ -102,6 +102,7 @@ interface QueryAssistWrapperProps {
 
 const QueryAssistWrapper: React.FC<QueryAssistWrapperProps> = (props) => {
   const [visible, setVisible] = useState(false);
 
   useEffect(() => {
     let mounted = true;
@@ -112,6 +113,7 @@ const QueryAssistWrapper: React.FC<QueryAssistWrapperProps> = (props) => {
       props.http
     ).subscribe((languages) => {
       const available = languages.includes(props.dependencies.language);
       if (mounted) setVisible(props.invert ? !available : available);
     });
 
diff --git a/src/plugins/query_enhancements/server/routes/query_assist/agents.test.ts b/src/plugins/query_enhancements/server/routes/query_assist/agents.test.ts
index f22bdb14836..5e31191c9c9 100644
--- a/src/plugins/query_enhancements/server/routes/query_assist/agents.test.ts
+++ b/src/plugins/query_enhancements/server/routes/query_assist/agents.test.ts
@@ -5,25 +5,29 @@
 
 import { ApiResponse } from '@opensearch-project/opensearch';
 import { ResponseError } from '@opensearch-project/opensearch/lib/errors';
+import { loggerMock } from '@osd/logging/target/mocks';
 import { RequestHandlerContext } from 'src/core/server';
-// // eslint-disable-next-line @osd/eslint/no-restricted-paths
-// import { CoreRouteHandlerContext } from 'src/core/server/core_route_handler_context';
 import { coreMock } from '../../../../../core/server/mocks';
-import { loggerMock } from '@osd/logging/target/mocks';
 import { getAgentIdByConfig, requestAgentByConfig } from './agents';
 
-describe.skip('Agents helper functions', () => {
-  // const coreContext = new CoreRouteHandlerContext(
-  //   coreMock.createInternalStart(),
-  //   httpServerMock.createOpenSearchDashboardsRequest()
-  // );
+describe('Agents helper functions', () => {
   const coreContext = coreMock.createRequestHandlerContext();
   const client = coreContext.opensearch.client.asCurrentUser;
   const mockedTransport = client.transport.request as jest.Mock;
   const context: RequestHandlerContext = {
     core: coreContext,
+    // @ts-ignore
     dataSource: jest.fn(),
-    query_assist: { dataSourceEnabled: false, logger: loggerMock.create() },
+    query_assist: {
+      dataSourceEnabled: false,
+      logger: loggerMock.create(),
+      configPromise: Promise.resolve({
+        enabled: true,
+        queryAssist: {
+          supportedLanguages: [{ language: 'PPL', agentConfig: 'testConfig' }],
+        },
+      }),
+    },
   };
 
   afterEach(() => {
diff --git a/src/plugins/query_enhancements/server/types.ts b/src/plugins/query_enhancements/server/types.ts
index 2ab716b9892..1a5d962e705 100644
--- a/src/plugins/query_enhancements/server/types.ts
+++ b/src/plugins/query_enhancements/server/types.ts
@@ -51,7 +51,10 @@ export interface FacetRequest {
   };
 }
 
-declare module '../../../core/server' {
+// TODO declare it in core changes the type for every reference. we only need
+// this declaration in query_enhancements, but it doesn't seem possible
+// https://github.com/opensearch-project/OpenSearch-Dashboards/issues/4274
+/* declare module '../../../core/server' {
   interface RequestHandlerContext {
     query_assist: {
       logger: Logger;
@@ -64,4 +67,4 @@ declare module '../../../core/server' {
       dataSourceEnabled: boolean;
     };
   }
-}
+} */

thx joshua, I merged this PR into main did you want to open a pr for that fix into the main repo? otherwise i can do that

[opensearch-trigger-bot]

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-7212-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 9194d83303879b224a0d416b03be4cadfe855f7f
# Push it to GitHub
git push --set-upstream origin backport/backport-7212-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-7212-to-2.x.

[joshuali925]

i added a PR for the skipped tests #7272

[opensearch-trigger-bot]

The backport to 2.16 failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.16 2.16
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.16
# Create a new branch
git switch --create backport/backport-7212-to-2.16
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 9194d83303879b224a0d416b03be4cadfe855f7f
# Push it to GitHub
git push --set-upstream origin backport/backport-7212-to-2.16
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.16

Then, create a pull request where the base branch is 2.16 and the compare/head branch is backport/backport-7212-to-2.16.