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

feat: support PPL in vega visualization #7285

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export function getSearchParamsFromRequest(
}

return {
index: searchRequest.index.title || searchRequest.index,
index: searchRequest.index?.title || searchRequest.index,
body: searchRequest.body,
...searchParams,
};
Expand Down
10 changes: 8 additions & 2 deletions src/plugins/data/server/search/routes/search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ describe('Search service', () => {
await handler((mockContext as unknown) as RequestHandlerContext, mockRequest, mockResponse);

expect(mockDataStart.search.search).toBeCalled();
expect(mockDataStart.search.search.mock.calls[0][1]).toStrictEqual(mockBody);
expect(mockDataStart.search.search.mock.calls[0][1]).toStrictEqual({
...mockBody,
rawRequest: mockRequest,
});
expect(mockResponse.ok).toBeCalled();
expect(mockResponse.ok.mock.calls[0][0]).toEqual({
body: response,
Expand Down Expand Up @@ -125,7 +128,10 @@ describe('Search service', () => {
await handler((mockContext as unknown) as RequestHandlerContext, mockRequest, mockResponse);

expect(mockDataStart.search.search).toBeCalled();
expect(mockDataStart.search.search.mock.calls[0][1]).toStrictEqual(mockBody);
expect(mockDataStart.search.search.mock.calls[0][1]).toStrictEqual({
...mockBody,
rawRequest: mockRequest,
});
expect(mockResponse.customError).toBeCalled();
const error: any = mockResponse.customError.mock.calls[0][0];
expect(error.body.message).toBe('oh no');
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/server/search/routes/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function registerSearchRoute(
try {
const { withLongNumeralsSupport, ...response } = await selfStart.search.search(
context,
{ ...searchRequest, id },
{ ...searchRequest, id, rawRequest: request },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wouldn't update this. when it's ppl, the whole request should be passed as long as you just send it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean we check the strategy here, and when it's ppl, we send the whole request instead of request.body?

Copy link
Member Author

@ruanyl ruanyl Jul 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The client.asScoped(request) we call in search strategy requires the http request object, currently I see no better way to get it unless pass it explicitly.

{
abortSignal,
strategy,
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/data/server/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ export interface ISearchSetup {
}

export interface ISearchStart<
SearchStrategyRequest extends IOpenSearchDashboardsSearchRequest = IOpenSearchSearchRequest,
SearchStrategyRequest extends IOpenSearchDashboardsSearchRequest = IOpenSearchSearchRequest & {
rawRequest?: OpenSearchDashboardsRequest;
},
SearchStrategyResponse extends IOpenSearchDashboardsSearchResponse = IOpenSearchSearchResponse
> {
aggs: AggsStart;
Expand Down
1 change: 1 addition & 0 deletions src/plugins/query_enhancements/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const BASE_API = '/api/enhancements';

export const SEARCH_STRATEGY = {
PPL: 'ppl',
PPL_RAW: 'pplraw',
SQL: 'sql',
SQL_ASYNC: 'sqlasync',
};
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/query_enhancements/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ import {
QueryEnhancementsPluginSetup,
QueryEnhancementsPluginSetupDependencies,
QueryEnhancementsPluginStart,
QueryEnhancementsPluginStartDependencies,
} from './types';
import { OpenSearchObservabilityPlugin, OpenSearchPPLPlugin } from './utils';
import { pplRawSearchStrategyProvider } from './search/ppl_raw_search_strategy';

export class QueryEnhancementsPlugin
implements Plugin<QueryEnhancementsPluginSetup, QueryEnhancementsPluginStart> {
Expand All @@ -51,6 +53,7 @@ export class QueryEnhancementsPlugin
}

const pplSearchStrategy = pplSearchStrategyProvider(this.config$, this.logger, client);
const pplRawSearchStrategy = pplRawSearchStrategyProvider(this.config$, this.logger, client);
const sqlSearchStrategy = sqlSearchStrategyProvider(this.config$, this.logger, client);
const sqlAsyncSearchStrategy = sqlAsyncSearchStrategyProvider(
this.config$,
Expand All @@ -59,6 +62,7 @@ export class QueryEnhancementsPlugin
);

data.search.registerSearchStrategy(SEARCH_STRATEGY.PPL, pplSearchStrategy);
data.search.registerSearchStrategy(SEARCH_STRATEGY.PPL_RAW, pplRawSearchStrategy);
data.search.registerSearchStrategy(SEARCH_STRATEGY.SQL, sqlSearchStrategy);
data.search.registerSearchStrategy(SEARCH_STRATEGY.SQL_ASYNC, sqlAsyncSearchStrategy);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { SharedGlobalConfig, Logger, ILegacyClusterClient } from 'opensearch-dashboards/server';
import { Observable } from 'rxjs';
import { ISearchStrategy, SearchUsage } from '../../../data/server';
import { shimSchemaRow } from '../utils';

export const pplRawSearchStrategyProvider = (
config$: Observable<SharedGlobalConfig>,
logger: Logger,
client: ILegacyClusterClient,
usage?: SearchUsage
): ISearchStrategy => {
return {
search: async (context, request: any, options) => {
const runSearch = request.dataSourceId
? context.dataSource.opensearch.legacy.getClient(request.dataSourceId).callAPI
: client.asScoped(request.rawRequest).callAsCurrentUser;

try {
const rawResponse: any = await runSearch('ppl.pplQuery', { body: request.params.body });
const data = shimSchemaRow(rawResponse);
rawResponse.jsonData = data.jsonData;

return {
rawResponse,
};
} catch (e) {
logger.error(`pplSearchStrategy: ${e.message}`);
if (usage) usage.trackError();
throw e;
}
},
};
};
57 changes: 57 additions & 0 deletions src/plugins/vis_type_vega/public/data_model/ppl_parser.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be possible to put like a parsers map in the search service? ideally. all non-out of the box experience is encapsulated within the query enhancements. so the parsers service can host the parser and take in a search strategy id. then in query enhancements we register the ppl

avoids having to re-write shared logic too

Copy link
Member Author

@ruanyl ruanyl Jul 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, let's explore the reusability, especially with considering the vis builder+vega, but that requires to clarify the requirement from both technical and design perspective, I'll leave it for future iteration, this PR is just the first step to add ppl support in vega. For now, I'll keep the scope as small as possible :)

Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { i18n } from '@osd/i18n';
import { Data, UrlObject, PPLQueryRequest } from './types';
import { SearchAPI } from './search_api';

const getRequestName = (request: PPLQueryRequest, index: number) =>
request.dataObject.name ||
i18n.translate('visTypeVega.opensearchQueryParser.unnamedRequest', {
defaultMessage: 'Unnamed request #{index}',
values: { index },
});

export class PPLQueryParser {
searchAPI: SearchAPI;

constructor(searchAPI: SearchAPI) {
this.searchAPI = searchAPI;
}

parseUrl(dataObject: Data, url: UrlObject) {
// data.url.body.query must be defined
if (!url.body || !url.body.query || typeof url.body.query !== 'string') {
throw new Error(

Check warning on line 27 in src/plugins/vis_type_vega/public/data_model/ppl_parser.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/data_model/ppl_parser.ts#L27

Added line #L27 was not covered by tests
i18n.translate('visTypeVega.pplQueryParser.dataUrl.PPL.queryCannotBeEmpty', {
defaultMessage: '{dataUrlParam} must have query specified',
values: {
dataUrlParam: '"data.url"',
},
})
);
}

return { dataObject, url };

Check warning on line 37 in src/plugins/vis_type_vega/public/data_model/ppl_parser.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/data_model/ppl_parser.ts#L37

Added line #L37 was not covered by tests
}

async populateData(requests: PPLQueryRequest[]) {
const searchRequests = requests.map((r, index) => ({

Check warning on line 41 in src/plugins/vis_type_vega/public/data_model/ppl_parser.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/data_model/ppl_parser.ts#L41

Added line #L41 was not covered by tests
...r.url,
name: getRequestName(r, index),
}));

const data$ = await this.searchAPI.search(searchRequests, { strategy: 'pplraw' });
const results = await data$.toPromise();
results.forEach((data, index) => {
const requestObject = requests.find((item) => getRequestName(item, index) === data.name);

Check warning on line 49 in src/plugins/vis_type_vega/public/data_model/ppl_parser.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/data_model/ppl_parser.ts#L46-L49

Added lines #L46 - L49 were not covered by tests

if (requestObject) {
requestObject.dataObject.url = requestObject.url;
requestObject.dataObject.values = (data.rawResponse as any).jsonData;

Check warning on line 53 in src/plugins/vis_type_vega/public/data_model/ppl_parser.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/data_model/ppl_parser.ts#L52-L53

Added lines #L52 - L53 were not covered by tests
}
});
}
}
24 changes: 17 additions & 7 deletions src/plugins/vis_type_vega/public/data_model/search_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
public readonly inspectorAdapters?: VegaInspectorAdapters
) {}

async search(searchRequests: SearchRequest[]) {
async search(searchRequests: SearchRequest[], options?: { strategy?: string }) {
const { search } = this.dependencies.search;
const requestResponders: any = {};

Expand Down Expand Up @@ -87,8 +87,13 @@
? { params, dataSourceId }
: { params };

return search(searchApiParams, { abortSignal: this.abortSignal }).pipe(
tap((data) => this.inspectSearchResult(data, requestResponders[requestId])),
return search(searchApiParams, {
abortSignal: this.abortSignal,
strategy: options?.strategy,
}).pipe(
tap((data) =>
this.inspectSearchResult(data, requestResponders[requestId], options?.strategy)

Check warning on line 95 in src/plugins/vis_type_vega/public/data_model/search_api.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/data_model/search_api.ts#L95

Added line #L95 was not covered by tests
),
map((data) => ({
name: requestId,
rawResponse: data.rawResponse,
Expand Down Expand Up @@ -137,12 +142,17 @@

private inspectSearchResult(
response: IOpenSearchSearchResponse,
requestResponder: RequestResponder
requestResponder: RequestResponder,
strategy?: string
) {
if (requestResponder) {
requestResponder
.stats(dataPluginSearch.getResponseInspectorStats(response.rawResponse))
.ok({ json: response.rawResponse });
if (!strategy) {
requestResponder

Check warning on line 150 in src/plugins/vis_type_vega/public/data_model/search_api.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/data_model/search_api.ts#L150

Added line #L150 was not covered by tests
.stats(dataPluginSearch.getResponseInspectorStats(response.rawResponse))
.ok({ json: response.rawResponse });
} else {
requestResponder.ok({ json: response.rawResponse });

Check warning on line 154 in src/plugins/vis_type_vega/public/data_model/search_api.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_type_vega/public/data_model/search_api.ts#L154

Added line #L154 was not covered by tests
}
}
}
}
1 change: 1 addition & 0 deletions src/plugins/vis_type_vega/public/data_model/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export type OpenSearchQueryRequest = Requests;
export type EmsQueryRequest = Requests & {
obj: UrlObject;
};
export type PPLQueryRequest = Requests;

export interface ContextVarsObject {
[index: string]: any;
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/vis_type_vega/public/data_model/vega_parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import { euiThemeVars } from '@osd/ui-shared-deps/theme';
import { i18n } from '@osd/i18n';
// @ts-ignore
import { Signal } from 'vega';

import { HttpSetup } from 'opensearch-dashboards/public';
import { vega, vegaLite } from '../lib/vega';
import { OpenSearchQueryParser } from './opensearch_query_parser';
import { Utils } from './utils';
Expand All @@ -59,6 +61,7 @@ import {
ControlsDirection,
OpenSearchDashboards,
} from './types';
import { PPLQueryParser } from './ppl_parser';

// Set default single color to match other OpenSearch Dashboards visualizations
const defaultColor: string = euiPaletteColorBlind()[0];
Expand Down Expand Up @@ -653,6 +656,7 @@ The URL is an identifier only. OpenSearch Dashboards and your browser will never
opensearch: new OpenSearchQueryParser(this.timeCache, this.searchAPI, this.filters, onWarn),
emsfile: new EmsFileParser(serviceSettings),
url: new UrlParser(onWarn),
ppl: new PPLQueryParser(this.searchAPI),
};
}
const pending: PendingType = {};
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/visualizations/public/legacy/build_pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,8 @@ export const buildVislibDimensions = async (

export const buildPipeline = async (vis: Vis, params: BuildPipelineParams) => {
const { indexPattern, searchSource } = vis.data;
const query = searchSource!.getField('query');
const filters = searchSource!.getField('filter');
const query = searchSource?.getField('query');
const filters = searchSource?.getField('filter');
const { uiState, title } = vis;

// context
Expand Down
Loading