Remove filter out serverless cluster and add support to extract index name #8872

ananzh · 2024-11-15T08:22:19Z

Description

Allow extract index name for both serverless and non-serverless clusters Allow different key formats:

datasource-id::TIMESERIES:::0
datasource-id:::0
(non-serverless case)

Issues Resolved

NA

Screenshot

2024-11-14_17-49-18.mp4

Testing the changes

NA

Changelog

fix: Remove filter out serverless cluster and add support to extract index name

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

… name Allow extract index name for both serverless and non-serverless clusters Allow different key formats: - datasource-id::TIMESERIES::<index-name>:0 - datasource-id::<index-name>:0 - <index-name> (non-serverless case) Signed-off-by: Anan Zhuang <[email protected]>

codecov · 2024-11-15T08:38:17Z

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 60.87%. Comparing base (413697d) to head (cad3eca).
Report is 15 commits behind head on main.

Files with missing lines	Patch %	Lines
...ery/query_string/dataset_service/lib/index_type.ts	77.77%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8872      +/-   ##
==========================================
- Coverage   60.93%   60.87%   -0.07%     
==========================================
  Files        3800     3802       +2     
  Lines       90878    91059     +181     
  Branches    14323    14376      +53     
==========================================
+ Hits        55380    55428      +48     
- Misses      31968    32092     +124     
- Partials     3530     3539       +9

Flag	Coverage Δ
Linux_1	`29.01% <11.11%> (-0.01%)`	⬇️
Linux_2	`56.39% <ø> (ø)`
Linux_3	`37.89% <77.77%> (-0.04%)`	⬇️
Linux_4	`28.99% <20.00%> (-0.01%)`	⬇️
Windows_1	`29.02% <11.11%> (-0.01%)`	⬇️
Windows_2	`56.34% <ø> (ø)`
Windows_3	`37.89% <77.77%> (-0.04%)`	⬇️
Windows_4	`28.99% <20.00%> (-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.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

kavilla · 2024-11-15T11:15:44Z

src/plugins/data/public/query/query_string/dataset_service/lib/index_type.test.ts

+          aggregations: {
+            indices: {
+              buckets: [
+                // Serverless format with TIMESERIES


nit: i think serverless is a product. probably shouldn't mention here

will remove this comment

kavilla · 2024-11-15T11:19:35Z

src/plugins/data/public/query/query_string/dataset_service/lib/index_type.ts

+      // Handle the case of serverless cluster where key format is either:
+      // - datasource-id::TIMESERIES::<index-name>:0
+      // - datasource-id::<index-name>:0
+      // Note: Index names cannot contain ':' or '::' in OpenSearch, so these delimiters


we have verified this correct?

can we get sign off or buy in from the team formatting indices this way? it might even be worth export a const like DELIMINATER and using it. i think we have one in some plugins cant remember if it is global.

ideally in the future collections functions like workspaces accurate results since the datasource id i believe is getting long. and then our dataset takes the id over the parent and appends itself to it. our url is getting quiet long. Discover doesn't really need to know this information if the data source already knows what information.

also might need some insight on the collection concept. but it will hurt if the format changes.

the over engineered option would be to make an advanced settings that is like collection index format or something

kavilla · 2024-11-15T11:20:07Z

src/plugins/data/public/query/query_string/dataset_service/lib/index_type.ts

+      // Note: Index names cannot contain ':' or '::' in OpenSearch, so these delimiters
+      // are guaranteed to be part of the serverless format, not the index name
+      const parts = key.split('::');
+      const lastPart = parts[parts.length - 1] || '';


nit: buckets is being mapped so the key will exist and parts will always have a length

Suggested change

const lastPart = parts[parts.length - 1] || '';

const lastPart = parts[parts.length - 1];

Suggested change

const lastPart = parts[parts.length - 1] || '';

const lastPart = parts[parts.length - 1] || key;

prolly not '' if you. decide to go with havinga fallback

kavilla · 2024-11-15T11:21:04Z

src/plugins/data/public/query/query_string/dataset_service/lib/index_type.ts

+      // Note: Index names cannot contain ':' or '::' in OpenSearch, so these delimiters
+      // are guaranteed to be part of the serverless format, not the index name
+      const parts = key.split('::');
+      const lastPart = parts[parts.length - 1] || '';


just verifying there's no way we would run into an exception here right?

Yes. I verified both : and ::. Error is 400 invalid index name. Was trying to add screenshot but failed.

kavilla · 2024-11-15T11:21:46Z

src/plugins/query_enhancements/public/datasets/s3_type.test.ts

@@ -154,33 +152,6 @@ describe('s3TypeConfig', () => {
      expect(result.children?.[0].title).toBe('DataSource 1');
      expect(result.hasNext).toBe(true);
    });
-
-    it('should filter out data sources with versions lower than 1.0.0', async () => {


nit: perhaps its worth to keep this test to but flip it so it shows that we do NOT filter out data sources

kavilla · 2024-11-15T11:23:16Z

src/plugins/data/public/query/query_string/dataset_service/lib/index_type.ts

+
+    return rawResponse.aggregations.indices.buckets.map((bucket: { key: string }) => {
+      const key = bucket.key;
+      // Handle the case of serverless cluster where key format is either:


nit: same as above i think serverless is a product. so prolly should be taken out

kavilla

awesome! i think we should get buy in and committment on the format not changing if this is how it will be otherwise we should consider adding a new data type collections

Signed-off-by: Anan Zhuang <[email protected]>

ananzh · 2024-11-15T22:19:47Z

loll new commit will wipe out changelog then update changelog will wipe out approval
@kavilla could you re-approve? 🫨

… name (#8872) * Remove filter out serverless cluster and add support to extract index name Allow extract index name for both serverless and non-serverless clusters Allow different key formats: - datasource-id::TIMESERIES::<index-name>:0 - datasource-id::<index-name>:0 - <index-name> (non-serverless case) Signed-off-by: Anan Zhuang <[email protected]> * fix PR comment Signed-off-by: Anan Zhuang <[email protected]> * Changeset file for PR #8872 created/updated --------- Signed-off-by: Anan Zhuang <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 1cb2511) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

ananzh assigned kavilla Nov 15, 2024

github-actions bot added the distinguished-contributor label Nov 15, 2024

opensearch-changeset-bot bot added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Nov 15, 2024

Changeset file for PR opensearch-project#8872 created/updated

3634a45

kavilla added backport 2.x discover for discover reinvent labels Nov 15, 2024

kavilla added the discover-next label Nov 15, 2024

kavilla reviewed Nov 15, 2024

View reviewed changes

kavilla previously approved these changes Nov 15, 2024

View reviewed changes

fix PR comment

ea9a708

Signed-off-by: Anan Zhuang <[email protected]>

ananzh dismissed kavilla’s stale review via ea9a708 November 15, 2024 20:17

ananzh force-pushed the fix-neo-serverless branch from 3634a45 to ea9a708 Compare November 15, 2024 20:17

kavilla previously approved these changes Nov 15, 2024

View reviewed changes

Changeset file for PR opensearch-project#8872 created/updated

cad3eca

opensearch-changeset-bot bot dismissed kavilla’s stale review via cad3eca November 15, 2024 22:12

abbyhu2000 approved these changes Nov 15, 2024

View reviewed changes

kavilla approved these changes Nov 15, 2024

View reviewed changes

ananzh merged commit 1cb2511 into opensearch-project:main Nov 21, 2024
69 checks passed

opensearch-trigger-bot bot mentioned this pull request Nov 21, 2024

[Backport 2.x] Remove filter out serverless cluster and add support to extract index name #8910

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove filter out serverless cluster and add support to extract index name #8872

Remove filter out serverless cluster and add support to extract index name #8872

ananzh commented Nov 15, 2024 •

edited

Loading

codecov bot commented Nov 15, 2024 •

edited

Loading

kavilla Nov 15, 2024

ananzh Nov 15, 2024

kavilla Nov 15, 2024

kavilla Nov 15, 2024 •

edited

Loading

kavilla Nov 15, 2024

kavilla Nov 15, 2024

kavilla Nov 15, 2024

kavilla Nov 15, 2024

ananzh Nov 15, 2024

kavilla Nov 15, 2024

kavilla Nov 15, 2024

kavilla left a comment

ananzh commented Nov 15, 2024

	const lastPart = parts[parts.length - 1] \|\| '';
	const lastPart = parts[parts.length - 1];

Remove filter out serverless cluster and add support to extract index name #8872

Remove filter out serverless cluster and add support to extract index name #8872

Conversation

ananzh commented Nov 15, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

codecov bot commented Nov 15, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kavilla Nov 15, 2024 • edited Loading

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

kavilla left a comment

Choose a reason for hiding this comment

ananzh commented Nov 15, 2024

ananzh commented Nov 15, 2024 •

edited

Loading

codecov bot commented Nov 15, 2024 •

edited

Loading

kavilla Nov 15, 2024 •

edited

Loading