[MD] Concatenate data source name with index pattern name and change delimiter to double colon #5907

BionIT · 2024-02-21T04:21:27Z

Description

This change adds data source name as a prefix to the index pattern name for the index pattern selector. This change also changed the delimiter for the datasource name and index pattern name to be double colons by changing getIndexPatternTitle function which is used by index pattern service(

OpenSearch-Dashboards/src/plugins/data/common/index_patterns/index_patterns/index_patterns.ts

Line 127 in eff7cb5

result.attributes.title = await getIndexPatternTitle(

) and saved object management page(

OpenSearch-Dashboards/src/plugins/saved_objects_management/server/routes/find.ts

Line 99 in eff7cb5

const savedObjects = await Promise.all(

).

Issues Resolved

Fixes #5900

Screenshot

concatdsnameandindexname.mp4

Testing the changes

The following steps were performed to test the change in the recording:

disable data source, goes to maps plugin page
click add layer, then see the index pattern selector with options populated
go to visualization and select Controls , then click Add, will see index pattern selector with options populated correctly
enable data source, goes to the same visualization page, and will see the options that have data source name prefixed before index pattern name, the delimiter is double colon
go to maps plugin, click add layer, then see the index pattern selector with options populated, data source name is added as prefix before the index pattern name and the delimiter is double colon

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: Lu Yu <[email protected]>

codecov · 2024-02-21T04:35:20Z

Codecov Report

Attention: Patch coverage is 93.33333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 67.01%. Comparing base (4081154) to head (5e30698).

Files	Patch %	Lines
...lugins/data/common/index_patterns/lib/get_title.ts	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5907      +/-   ##
==========================================
+ Coverage   66.98%   67.01%   +0.02%     
==========================================
  Files        3305     3305              
  Lines       63574    63585      +11     
  Branches    10153    10155       +2     
==========================================
+ Hits        42587    42609      +22     
+ Misses      18520    18506      -14     
- Partials     2467     2470       +3

Flag	Coverage Δ
Linux_1	`35.21% <13.33%> (-0.01%)`	⬇️
Linux_2	`55.09% <13.33%> (-0.02%)`	⬇️
Linux_3	`43.58% <93.33%> (+0.01%)`	⬆️
Linux_4	`35.20% <13.33%> (-0.01%)`	⬇️
Windows_1	`35.27% <13.33%> (+0.02%)`	⬆️
Windows_2	`55.06% <13.33%> (-0.02%)`	⬇️
Windows_3	`43.58% <93.33%> (+0.02%)`	⬆️
Windows_4	`35.20% <13.33%> (-0.01%)`	⬇️

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: Lu Yu <[email protected]>

bandinib-amzn · 2024-02-21T22:49:14Z

Can we add local cluster prefix for index pattern created from local cluster index for consistent look? cc: @kgcreative

kgcreative · 2024-02-21T23:18:15Z

Can we add local cluster prefix for index pattern created from local cluster index for consistent look? cc: @kgcreative

I think that makes sense. What do you think, @BionIT?

kgcreative · 2024-02-21T23:22:08Z

With concatenated names, would that also resolve the current bug of not being able to create an index pattern with the same name against two clusters?

Today, if I create an index pattern named sample_logs* on my local cluster, I am forced to get creative and create an index pattern named sample_log* against cluster 2.

We can update the unique names requirement to only check for unique index pattern names against the same data source

BionIT · 2024-02-21T23:43:30Z

Can we add local cluster prefix for index pattern created from local cluster index for consistent look? cc: @kgcreative

I think that makes sense. What do you think, @BionIT?

I think that would make it confusing if user doesn't know what does local cluster represents and they have no control of it since it is hard coded there and they might wonder where does this local cluster name came from. The reason we add data source name as prefix is to make it easy to distinguish between index patterns created using different data sources and the data source name was created by user. To keep consistency, I didn't see local cluster being added in the dashboards today to emphasize the index pattern or saved object is from local cluster.

bandinib-amzn · 2024-02-22T00:18:45Z

src/plugins/data/public/ui/index_pattern_select/index_pattern_select.tsx

+      const dataSourcesToFetch: Array<{ type: string; id: string }> = [];
+      savedObjects.map((indexPatternSavedObject: SimpleSavedObject<any>) => {
+        const dataSourceReference = getDataSourceReference(indexPatternSavedObject.references);
+        if (dataSourceReference && !this.state.dataSourceIdToTitle.has(dataSourceReference.id)) {
+          dataSourcesToFetch.push({ type: 'data-source', id: dataSourceReference.id });
+        }
+      });
+
+      const dataSourceIdToTitleToUpdate = new Map();
+
+      if (dataSourcesToFetch.length > 0) {
+        const resp = await savedObjectsClient.bulkGet(dataSourcesToFetch);
+        resp.savedObjects.map((dataSourceSavedObject: SimpleSavedObject<any>) => {
+          dataSourceIdToTitleToUpdate.set(
+            dataSourceSavedObject.id,
+            dataSourceSavedObject.attributes.title
+          );
+        });
+      }
+


Can we move this code to getTitle? We have index pattern id, saved object client to find reference, right? We don't have to pass third parameter. Otherwise consumer of getTitle always needs to pass map with data source id and title? Instead can getTitle function handle that?

This part of code would perform bulk get index patterns(existing behavior), the added behavior is if the referred data source doesn't exist in the current state, it will fetch the data sources in bulk. Note there is only 2 requests here. The getTitle function is only used for selected index pattern which is a single index pattern, the existing behavior is it use an index pattern id to make individual call to saved object API to get the index pattern title, and if we use this function to handle all index patterns, this would multiple by number of index patterns, then to add data source title would double the requests.
If we want to refactor this, I can extract the logic here to a function to perform bulk operation and to produce the idToTitle map

Got it. No need to extract for now. We need to unify the pickers from all screen and that would be another effort. Thanks for the change.

bandinib-amzn · 2024-02-22T00:23:04Z

Can we add local cluster prefix for index pattern created from local cluster index for consistent look? cc: @kgcreative

I think that makes sense. What do you think, @BionIT?

I think that would make it confusing if user doesn't know what does local cluster represents and they have no control of it since it is hard coded there and they might wonder where does this local cluster name came from. The reason we add data source name as prefix is to make it easy to distinguish between index patterns created using different data sources and the data source name was created by user. To keep consistency, I didn't see local cluster being added in the dashboards today to emphasize the index pattern or saved object is from local cluster.

But we do have Local cluster option in cluster selector. Do you mean when multi data source feature is disabled?

BionIT · 2024-02-22T00:59:02Z

Can we add local cluster prefix for index pattern created from local cluster index for consistent look? cc: @kgcreative

I think that makes sense. What do you think, @BionIT?

I think that would make it confusing if user doesn't know what does local cluster represents and they have no control of it since it is hard coded there and they might wonder where does this local cluster name came from. The reason we add data source name as prefix is to make it easy to distinguish between index patterns created using different data sources and the data source name was created by user. To keep consistency, I didn't see local cluster being added in the dashboards today to emphasize the index pattern or saved object is from local cluster.

But we do have Local cluster option in cluster selector. Do you mean when multi data source feature is disabled?

That's the place for user to select the source and not for adding to the existing title, if we look at 1) data picker in discover, 2) in index pattern management page, 3) in saved objects management, none of them show the local cluster in the title. If we plan to be explicit about which resource belong to which cluster or local cluster, then I would suggest to make the change in all the pages. If we just need it to distinguish from resources from local cluster, then the current practice of handling is to not have local cluster in the title unless we plan to change the existing behavior which can be done in a separate PR? Any suggestions @kgcreative ?

kgcreative · 2024-02-22T18:18:53Z

Let's keep this issue scoped. I would prefer we don't change the way we display local cluster just yet, especially if that will require a lot of additional work and scope. We can have a sepparate conversation about enhancements and ergonomics of index patterns, including potentially a display name and other concepts in order to make this more fully featured in the future.

bandinib-amzn · 2024-02-22T19:57:44Z

Approved the change. But can you check why several checks are failing?

…delimiter to double colon (#5907) * concatenate data source name with index pattern name Signed-off-by: Lu Yu <[email protected]> * add changelog Signed-off-by: Lu Yu <[email protected]> * add tests Signed-off-by: Lu Yu <[email protected]> --------- Signed-off-by: Lu Yu <[email protected]> (cherry picked from commit 4ab0ca8) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…delimiter to double colon (#5907) (#5930) * concatenate data source name with index pattern name * add changelog * add tests --------- (cherry picked from commit 4ab0ca8) Signed-off-by: Lu Yu <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…delimiter to double colon (opensearch-project#5907) * concatenate data source name with index pattern name Signed-off-by: Lu Yu <[email protected]> * add changelog Signed-off-by: Lu Yu <[email protected]> * add tests Signed-off-by: Lu Yu <[email protected]> --------- Signed-off-by: Lu Yu <[email protected]>

concatenate data source name with index pattern name

4cadb9b

Signed-off-by: Lu Yu <[email protected]>

BionIT requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, kristenTian, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, BSFishy, curq, bandinib-amzn and SuZhou-Joe as code owners February 21, 2024 04:21

github-actions bot added the repeat-contributor label Feb 21, 2024

add changelog

2e8fc90

Signed-off-by: Lu Yu <[email protected]>

add tests

e9cb414

Signed-off-by: Lu Yu <[email protected]>

bandinib-amzn reviewed Feb 22, 2024

View reviewed changes

Merge branch 'main' into indexpattern

86239ff

bandinib-amzn approved these changes Feb 22, 2024

View reviewed changes

Merge branch 'main' into indexpattern

5e30698

ZilongX approved these changes Feb 22, 2024

View reviewed changes

ZilongX added backport 2.x v2.13.0 labels Feb 23, 2024

ZilongX merged commit 4ab0ca8 into opensearch-project:main Feb 23, 2024

opensearch-trigger-bot bot mentioned this pull request Feb 23, 2024

[Backport 2.x] [MD] Concatenate data source name with index pattern name and change delimiter to double colon #5930

Merged

BionIT mentioned this pull request Mar 5, 2024

[Meta][2.13] Security, bug fix and enhancemanet for dashboards anywhere #5752

Open

28 tasks

yujin-emma mentioned this pull request Mar 13, 2024

[BUG] when enable multiple data source, import saved objects of index pattern display UUID as prefix, what expected is using data source connection name #5901

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MD] Concatenate data source name with index pattern name and change delimiter to double colon #5907

[MD] Concatenate data source name with index pattern name and change delimiter to double colon #5907

BionIT commented Feb 21, 2024 •

edited

Loading

codecov bot commented Feb 21, 2024 •

edited

Loading

bandinib-amzn commented Feb 21, 2024

kgcreative commented Feb 21, 2024

kgcreative commented Feb 21, 2024

BionIT commented Feb 21, 2024

bandinib-amzn Feb 22, 2024

BionIT Feb 22, 2024

bandinib-amzn Feb 22, 2024

bandinib-amzn commented Feb 22, 2024

BionIT commented Feb 22, 2024 •

edited

Loading

kgcreative commented Feb 22, 2024

bandinib-amzn commented Feb 22, 2024

[MD] Concatenate data source name with index pattern name and change delimiter to double colon #5907

[MD] Concatenate data source name with index pattern name and change delimiter to double colon #5907

Conversation

BionIT commented Feb 21, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Check List

codecov bot commented Feb 21, 2024 • edited Loading

Codecov Report

bandinib-amzn commented Feb 21, 2024

kgcreative commented Feb 21, 2024

kgcreative commented Feb 21, 2024

BionIT commented Feb 21, 2024

bandinib-amzn Feb 22, 2024

Choose a reason for hiding this comment

BionIT Feb 22, 2024

Choose a reason for hiding this comment

bandinib-amzn Feb 22, 2024

Choose a reason for hiding this comment

bandinib-amzn commented Feb 22, 2024

BionIT commented Feb 22, 2024 • edited Loading

kgcreative commented Feb 22, 2024

bandinib-amzn commented Feb 22, 2024

BionIT commented Feb 21, 2024 •

edited

Loading

codecov bot commented Feb 21, 2024 •

edited

Loading

BionIT commented Feb 22, 2024 •

edited

Loading