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.

Sign up for GitHub

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(vis_type_vega): support reading time field #9152

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat(vis_type_vega): support reading time field #9152

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a4be69c
feat(vis_type_vega): support reading time field
ruanyl Jan 7, 2025
6e2791c
Merge branch 'main' into vega-ppl-support-timefilter-from-context
ruanyl Jan 8, 2025
0ad3147
Changeset file for PR #9152 created/updated
opensearch-changeset-bot[bot] Jan 10, 2025
5bbbb2d
fix time format to use moment.utc
ruanyl Jan 13, 2025
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
2 changes: 2 additions & 0 deletions changelogs/fragments/9152.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- Vega visualization with ppl now supports reading time field ([#9152](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/9152))
59 changes: 55 additions & 4 deletions src/plugins/vis_type_vega/public/data_model/ppl_parser.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,18 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { timefilterServiceMock } from '../../../data/public/query/timefilter/timefilter_service.mock';
import { PPLQueryParser } from './ppl_parser';
import { TimeCache } from './time_cache';

test('it should throw error if with invalid url object', () => {
const searchApiMock = {
search: jest.fn(() => ({
toPromise: jest.fn(() => Promise.resolve({})),
})),
};
const parser = new PPLQueryParser(searchApiMock);
const timeCache = new TimeCache(timefilterServiceMock.createStartContract().timefilter, 100);
const parser = new PPLQueryParser(timeCache, searchApiMock);
expect(() => parser.parseUrl({}, {})).toThrowError();
expect(() => parser.parseUrl({}, { body: {} })).toThrowError();
expect(() => parser.parseUrl({}, { body: { query: {} } })).toThrowError();
Expand All @@ -23,21 +26,69 @@ test('it should parse url object', () => {
toPromise: jest.fn(() => Promise.resolve({})),
})),
};
const parser = new PPLQueryParser(searchApiMock);
const timeCache = new TimeCache(timefilterServiceMock.createStartContract().timefilter, 100);
const parser = new PPLQueryParser(timeCache, searchApiMock);
const result = parser.parseUrl({}, { body: { query: 'source=test_index' } });
expect(result.dataObject).toEqual({});
expect(result.url).toEqual({ body: { query: 'source=test_index' } });
});

it('should populate data to request', async () => {
test('it should parse url object with %timefield% with injecting time filter to ppl query', () => {
const from = new Date('2024-10-07T05:03:22.548Z');
const to = new Date('2025-01-08T05:03:30.981Z');
jest
.spyOn(TimeCache.prototype, 'getTimeBounds')
.mockReturnValue({ max: from.valueOf(), min: to.valueOf() });

const searchApiMock = {
search: jest.fn(() => ({
toPromise: jest.fn(() => Promise.resolve({})),
})),
};
const timeCache = new TimeCache(timefilterServiceMock.createStartContract().timefilter, 100);
timeCache.setTimeRange({
from: from.toISOString(),
to: to.toISOString(),
mode: 'absolute',
});

const parser = new PPLQueryParser(timeCache, searchApiMock);
const result1 = parser.parseUrl(
{},
{ body: { query: 'source=test_index' }, '%timefield%': 'timestamp' }
);
expect(result1.url).toEqual({
body: {
query:
"source=test_index | where `timestamp` >= '2025-01-08 13:03:30.981' and `timestamp` <= '2024-10-07 13:03:22.548'",
},
});

const result2 = parser.parseUrl(
{},
{
body: { query: 'source=test_index | stats count() as doc_count' },
'%timefield%': 'timestamp',
}
);
expect(result2.url).toEqual({
body: {
query:
"source=test_index | where `timestamp` >= '2025-01-08 13:03:30.981' and `timestamp` <= '2024-10-07 13:03:22.548' | stats count() as doc_count",
},
});
});

test('it should populate data to request', async () => {
const searchApiMock = {
search: jest.fn(() => ({
toPromise: jest.fn(() =>
Promise.resolve([{ name: 'request name', rawResponse: { jsonData: [{ id: 'id1' }] } }])
),
})),
};
const parser = new PPLQueryParser(searchApiMock);
const timeCache = new TimeCache(timefilterServiceMock.createStartContract().timefilter, 100);
const parser = new PPLQueryParser(timeCache, searchApiMock);
const request = {
url: { body: { query: 'source=test_index' } },
dataObject: {
Expand Down
32 changes: 29 additions & 3 deletions src/plugins/vis_type_vega/public/data_model/ppl_parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@
*/

import { i18n } from '@osd/i18n';
import moment from 'moment';

import { Data, UrlObject, PPLQueryRequest } from './types';
import { SearchAPI } from './search_api';
import { TimeCache } from './time_cache';

const TIMEFIELD = '%timefield%';

const getRequestName = (request: PPLQueryRequest, index: number) =>
request.dataObject.name ||
Expand All @@ -15,13 +20,29 @@
});

export class PPLQueryParser {
searchAPI: SearchAPI;

constructor(searchAPI: SearchAPI) {
constructor(private readonly timeCache: TimeCache, private readonly searchAPI: SearchAPI) {
this.searchAPI = searchAPI;
}

injectTimeFilter(query: string, timefield: string) {
if (this.timeCache._timeRange) {
const [source, ...others] = query.split('|');
const bounds = this.timeCache.getTimeBounds();
const from = moment(bounds.min).format('YYYY-MM-DD HH:mm:ss.SSS');
const to = moment(bounds.max).format('YYYY-MM-DD HH:mm:ss.SSS');
const timeFilter = `where \`${timefield}\` >= '${from}' and \`${timefield}\` <= '${to}'`;
if (others.length > 0) {
return `${source.trim()} | ${timeFilter} | ${others.map((s) => s.trim()).join(' | ')}`;

Check warning on line 35 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#L35 

Added line #L35 was not covered by tests
}
return `${source.trim()} | ${timeFilter}`;
}
return query;

Check warning on line 39 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#L39 

Added line #L39 was not covered by tests
}

parseUrl(dataObject: Data, url: UrlObject) {
const timefield = url[TIMEFIELD];
delete url[TIMEFIELD];
Comment on lines +43 to +44
Copy link
Collaborator

@Hailong-am Hailong-am Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

will this synced to other apps(discovery/dashboards)?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not the current browser url, it's the url property of vega-lite config object, the TIMEFIELD defines the time field name of current index, for example, timestamp, and it will used as the time filter of the query.


// data.url.body.query must be defined
if (!url.body || !url.body.query || typeof url.body.query !== 'string') {
throw new Error(
Expand All @@ -34,6 +55,11 @@
);
}

if (timefield) {
const query = this.injectTimeFilter(url.body.query, timefield);
url.body.query = query;
Copy link
Member

Choose a reason for hiding this comment

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

Is this the url that vega uses to query the cluster? I wouldn't expect the url parser to format/mutate the url, but I'm new to this area so it may be a gap in my understanding.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this the url that vega uses to query the cluster?

Yes, url.body.query is the ppl query and will be send to opensearch. We modify it here to encapsulate the time filter from the UI, for example, the time range user selected on a dashboard. The behavior is the same as vega visualization with opensearch DSL which modified the query DSL to add time filter.

I wouldn't expect the url parser to format/mutate the url

Could you please share your concern?

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense. No concerns, my expectation was that the parser is a pure function but it looks like this is more of a parser and formatter of the original url if I'm understanding correctly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I got your point now, I think we can also make it a pure function, for example we can deep copy the url object and then modify the deep copied object. I don’t notice much of a difference in this particular case since the existing parser mutate the url object as well, but I’d love to hear your thoughts on it :)

}

return { dataObject, url };
}

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_type_vega/public/data_model/vega_parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +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),
ppl: new PPLQueryParser(this.timeCache, this.searchAPI),
};
}
const pending: PendingType = {};
Expand Down
Loading