[MDS] TSVB Support #6298

huyaboo · 2024-03-29T17:40:50Z

Description

Currently, multiple data source (MDS) does not support TSVB visualizations. This PR adds a data source picker component to the TSVB Panel options page so users can specify which datasource to get index patterns from.

Issues Resolved

Partially addresses #6290 and closes #6005

Screenshot

Screen.Recording.2024-03-29.at.10.26.07.AM.mov

Testing the changes

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

Signed-off-by: Huy Nguyen <[email protected]>

codecov · 2024-03-29T17:54:46Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.40%. Comparing base (e785fd3) to head (710eaf7).
Report is 8 commits behind head on main.

❗ Current head 710eaf7 differs from pull request most recent head fe2b794. Consider uploading reports for the commit fe2b794 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #6298       +/-   ##
===========================================
+ Coverage   49.55%   67.40%   +17.84%     
===========================================
  Files        2670     2639       -31     
  Lines       54290    50276     -4014     
  Branches     8878     9086      +208     
===========================================
+ Hits        26906    33889     +6983     
+ Misses      25720    14236    -11484     
- Partials     1664     2151      +487

Flag	Coverage Δ
Linux_2	`55.62% <100.00%> (?)`
Linux_3	`45.11% <100.00%> (?)`
Linux_4	`34.96% <100.00%> (?)`
Windows_1	`?`
Windows_3	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Huy Nguyen <[email protected]>

bandinib-amzn · 2024-04-01T18:50:07Z

Hi @huyaboo I re-run failed Build and test / Build and Verify on Linux (ciGroup1) check but it is still failing. Can you please take a look?

bandinib-amzn · 2024-04-01T19:04:35Z

As we are adding a new file src/plugins/vis_type_timeseries/public/application/components/data_source_picker.tsx, let's refrain from using 'ts-expect-error'. Instead, I would encourage addressing type-level errors directly.

bandinib-amzn · 2024-04-11T20:03:09Z

src/plugins/vis_type_timeseries/server/lib/get_fields.ts

@@ -44,21 +44,24 @@ export async function getFields(
  requestContext: RequestHandlerContext,
  request: OpenSearchDashboardsRequest,
  framework: Framework,
-  indexPattern: string
+  indexPattern: string,
+  dataSourceId: string | null


+1 to @BionIT. Instead of null can we make optional parameter? If this is just because of router schema, then I would prefer fixing in router schema instead of workaround to support that. How much is the effort?

bandinib-amzn · 2024-04-11T20:06:04Z

src/plugins/data_source/server/util/decide_client.ts

+
+export const decideLegacyClient = (
+  requestContext: RequestHandlerContext,
+  dataSourceId: string | null


nit: let's use optional parameter instead of null.

We have decideClient in https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/data/server/search/opensearch_search/decide_client.ts.

Can we use same file for decideLegacyClient ?

Can we use request to get dataSourceId similar to decideClient?
OR
maybe refactor decideClient to return legacy client base on additional parameter?

I can see in decideClient we are supporting LongNumerals. Is it something we need to take for legacy client? Can you double check?

I found this function

OpenSearch-Dashboards/src/plugins/data/server/index_patterns/routes.ts

Line 158 in b4130aa

const decideClient = async (

for legacy client.

Looks like we have added this function while adding MDS feature. can we use same one?

Wrote a workaround to use decideClient() but this will need a refactor because other plugins may need this function as well. Raised a new issue to address this #6417

src/plugins/vis_type_timeseries/public/application/components/annotations_editor.js

...gins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts

Signed-off-by: Huy Nguyen <[email protected]>

bandinib-amzn · 2024-04-15T20:53:09Z

65 successful and 1 skipped checks before rebase.

bandinib-amzn · 2024-04-15T20:54:21Z

@huyaboo Thank you for making this change and patiently hearing everyone's comments and feedback and addressing them. Greatly appreciate your contribution!

zhongnansu · 2024-04-16T01:06:11Z

src/plugins/vis_type_timeseries/public/application/components/index_pattern.js

+                fullWidth={false}
+                removePrepend={true}
+                isClearable={false}
+                hideLocalCluster={!!getHideLocalCluster().hideLocalCluster}


I don't think we need to set hideLocalCluster any more. As #6361 merged in, this prop will be decided by the data source management plugin. Whatever get passed in explicitly by the props, will be overwritten anyways.

Removed all instances of hideLocalCluster

zhongnansu · 2024-04-16T01:07:48Z

src/plugins/vis_type_timeseries/public/services.ts

+  dataSourceManagement: DataSourceManagementPluginSetup | undefined;
+}>('DataSourceManagementSetup');
+
+export const [getHideLocalCluster, setHideLocalCluster] = createGetterSetter<{


similar to my previous comment, this also can be removed

Removed all instances of hideLocalCluster

Signed-off-by: Huy Nguyen <[email protected]>

BionIT · 2024-04-16T04:27:51Z

src/plugins/vis_type_timeseries/public/application/components/index_pattern.js

+                savedObjectsClient={getSavedObjectsClient().client}
+                notifications={getNotifications().toasts}
+                onSelectedDataSource={handleDataSourceSelectChange}
+                defaultOption={model.data_source_id ? [{ id: model.data_source_id }] : [{ id: '' }]}


shall we let the selector to decide the default data source if not provided?

Yeah fixed it. I realized there could be a case where the data_source_id can be undefined and the local cluster is disabled.

BionIT · 2024-04-16T04:28:52Z

...s/vis_type_timeseries/public/application/components/lib/create_data_source_change_handler.ts

+import { PanelSchema } from 'src/plugins/vis_type_timeseries/common/types';
+import { DATA_SOURCE_ID_KEY } from '../../../../common/constants';
+
+export const createDataSourcePickerHandler = (handleChange: (e: PanelSchema) => void) => {


do we have a test for this function?

Added test cases.

…d be local cluster Signed-off-by: Huy Nguyen <[email protected]>

Signed-off-by: Huy Nguyen <[email protected]>

* Add MDS support to TSVB Signed-off-by: Huy Nguyen <[email protected]> * Refactor datasource picker component Signed-off-by: Huy Nguyen <[email protected]> * Allow picker to persist state Signed-off-by: Huy Nguyen <[email protected]> * Refactored picker component params Signed-off-by: Huy Nguyen <[email protected]> * Add unit tests Signed-off-by: Huy Nguyen <[email protected]> * Add to CHANGELOG Signed-off-by: Huy Nguyen <[email protected]> * Refactor components to use hideLocalCluster Signed-off-by: Huy Nguyen <[email protected]> * Remove Picker wrapper Signed-off-by: Huy Nguyen <[email protected]> * Update selector component and rename field to index name Signed-off-by: Huy Nguyen <[email protected]> * Address comments Signed-off-by: Huy Nguyen <[email protected]> * Refactor to use different decideClient Signed-off-by: Huy Nguyen <[email protected]> * Add optional arg Signed-off-by: Huy Nguyen <[email protected]> * Remove hidelocalcluster as a setting Signed-off-by: Huy Nguyen <[email protected]> * Fixed case where local cluster is disabled but the datasource id could be local cluster Signed-off-by: Huy Nguyen <[email protected]> * Add test for create data source picker handler Signed-off-by: Huy Nguyen <[email protected]> --------- Signed-off-by: Huy Nguyen <[email protected]> (cherry picked from commit be0f9d5) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md

* Add MDS support to TSVB Signed-off-by: Huy Nguyen <[email protected]> * Refactor datasource picker component Signed-off-by: Huy Nguyen <[email protected]> * Allow picker to persist state Signed-off-by: Huy Nguyen <[email protected]> * Refactored picker component params Signed-off-by: Huy Nguyen <[email protected]> * Add unit tests Signed-off-by: Huy Nguyen <[email protected]> * Add to CHANGELOG Signed-off-by: Huy Nguyen <[email protected]> * Refactor components to use hideLocalCluster Signed-off-by: Huy Nguyen <[email protected]> * Remove Picker wrapper Signed-off-by: Huy Nguyen <[email protected]> * Update selector component and rename field to index name Signed-off-by: Huy Nguyen <[email protected]> * Address comments Signed-off-by: Huy Nguyen <[email protected]> * Refactor to use different decideClient Signed-off-by: Huy Nguyen <[email protected]> * Add optional arg Signed-off-by: Huy Nguyen <[email protected]> * Remove hidelocalcluster as a setting Signed-off-by: Huy Nguyen <[email protected]> * Fixed case where local cluster is disabled but the datasource id could be local cluster Signed-off-by: Huy Nguyen <[email protected]> * Add test for create data source picker handler Signed-off-by: Huy Nguyen <[email protected]> --------- Signed-off-by: Huy Nguyen <[email protected]> (cherry picked from commit be0f9d5) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: ZilongX <[email protected]>

huyaboo added 6 commits March 26, 2024 22:29

Add MDS support to TSVB

b9434b1

Signed-off-by: Huy Nguyen <[email protected]>

Refactor datasource picker component

6a00949

Signed-off-by: Huy Nguyen <[email protected]>

Allow picker to persist state

910e93a

Signed-off-by: Huy Nguyen <[email protected]>

Refactored picker component params

9b9c681

Signed-off-by: Huy Nguyen <[email protected]>

Add unit tests

7be618d

Signed-off-by: Huy Nguyen <[email protected]>

Merge branch 'main' into tsvb

7b7da2f

github-actions bot added the repeat-contributor label Mar 29, 2024

huyaboo marked this pull request as ready for review March 29, 2024 20:14

huyaboo requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, BSFishy, curq, bandinib-amzn, SuZhou-Joe, ruanyl, BionIT and xinruiba as code owners March 29, 2024 20:14

Add to CHANGELOG

cae4f7f

Signed-off-by: Huy Nguyen <[email protected]>

bandinib-amzn reviewed Apr 11, 2024

View reviewed changes

huyaboo added 6 commits April 11, 2024 13:38

Merge branch 'main' into tsvb

aadcd35

Refactor to use different decideClient

64c45e8

Signed-off-by: Huy Nguyen <[email protected]>

Add optional arg

817e342

Signed-off-by: Huy Nguyen <[email protected]>

Merge branch 'main' into tsvb

d4617d3

Merge branch 'main' into tsvb

710eaf7

Merge branch 'main' into tsvb

a0635bf

bandinib-amzn previously approved these changes Apr 15, 2024

View reviewed changes

huyaboo added 2 commits April 15, 2024 14:02

Merge branch 'main' into tsvb

106a09e

Merge branch 'main' into tsvb

f4f237f

zhongnansu reviewed Apr 16, 2024

View reviewed changes

Merge branch 'main' into tsvb

2a611a1

BionIT dismissed bandinib-amzn’s stale review via 2a611a1 April 16, 2024 02:09

Remove hidelocalcluster as a setting

1436a53

Signed-off-by: Huy Nguyen <[email protected]>

BionIT reviewed Apr 16, 2024

View reviewed changes

BionIT previously approved these changes Apr 16, 2024

View reviewed changes

huyaboo added 2 commits April 16, 2024 09:44

Merge branch 'main' into tsvb

a6300f4

Fixed case where local cluster is disabled but the datasource id coul…

76ed1a4

…d be local cluster Signed-off-by: Huy Nguyen <[email protected]>

huyaboo dismissed BionIT’s stale review via 76ed1a4 April 16, 2024 17:33

Add test for create data source picker handler

fe2b794

Signed-off-by: Huy Nguyen <[email protected]>

BionIT approved these changes Apr 16, 2024

View reviewed changes

zhongnansu approved these changes Apr 16, 2024

View reviewed changes

BionIT merged commit be0f9d5 into opensearch-project:main Apr 16, 2024
66 checks passed

opensearch-trigger-bot bot mentioned this pull request Apr 16, 2024

[Backport 2.x] [MDS] TSVB Support #6491

Merged

huyaboo deleted the tsvb branch April 18, 2024 00:15

BionIT mentioned this pull request Apr 22, 2024

[Meta] [2.14] Security, bug fix and enhancemanet for dashboards anywhere #5872

Closed

77 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MDS] TSVB Support #6298

[MDS] TSVB Support #6298

huyaboo commented Mar 29, 2024 •

edited

Loading

codecov bot commented Mar 29, 2024 •

edited

Loading

bandinib-amzn commented Apr 1, 2024

bandinib-amzn commented Apr 1, 2024

bandinib-amzn Apr 11, 2024

bandinib-amzn Apr 11, 2024

bandinib-amzn Apr 11, 2024

bandinib-amzn Apr 11, 2024 •

edited

Loading

huyaboo Apr 11, 2024

bandinib-amzn commented Apr 15, 2024

bandinib-amzn commented Apr 15, 2024

zhongnansu Apr 16, 2024

huyaboo Apr 16, 2024

zhongnansu Apr 16, 2024

huyaboo Apr 16, 2024

BionIT Apr 16, 2024

huyaboo Apr 16, 2024

BionIT Apr 16, 2024

huyaboo Apr 16, 2024

[MDS] TSVB Support #6298

[MDS] TSVB Support #6298

Conversation

huyaboo commented Mar 29, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Check List

codecov bot commented Mar 29, 2024 • edited Loading

Codecov Report

bandinib-amzn commented Apr 1, 2024

bandinib-amzn commented Apr 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bandinib-amzn Apr 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bandinib-amzn commented Apr 15, 2024

bandinib-amzn commented Apr 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huyaboo commented Mar 29, 2024 •

edited

Loading

codecov bot commented Mar 29, 2024 •

edited

Loading

bandinib-amzn Apr 11, 2024 •

edited

Loading