[Multiple Datasource][Version Decoupling] Add data source engine type to data source saved object #7026

ZilongX · 2024-06-13T23:36:27Z

Description

This one is to enrich the data source saved object by adding a new meta data dataSourceEngineType
This feature is required per discussion and feedback collected from RFC [RFC] Plugins Version Decoupling #5877.
In order to decouple the plugin versions with OS cluster versions, we will need to specify a supported data source version range for example >= 2.1.0, yet given we support both OpenSearch and ElasticSearch clusters, the version range could be overlapped since only the numeric part is recorded for now, so for an ES 7.10.2 data source the version would be recorded as 7.10.2, which would break the support range check as it would also satisfy the semver check of >=2.1.0. This extra piece of engine type information would get this issue resolved.

Issues Resolved

#7021

Screenshot

Newly created OS datasource with dataSourceEngineType as OpenSearch
Newly created ES datasource with dataSourceEngineType as Elasticsearch
Newly created OSS datasource with dataSourceEngineType as OpenSearch Serverless

Testing the changes

All related test cases passed as

Changelog

fix: [MDS] Add data source engine type to data source saved object

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

… to data source saved object Signed-off-by: Zilong Xia <zilongx@amazon.com>

github-actions · 2024-06-13T23:37:02Z

❌ Changelog Entry Missing Hyphen

Changelog entries must begin with a hyphen (-).

github-actions · 2024-06-13T23:39:04Z

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "feat". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

github-actions · 2024-06-13T23:41:58Z

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "feat". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

github-actions · 2024-06-13T23:43:28Z

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "fix". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

codecov · 2024-06-13T23:50:18Z

Codecov Report

Attention: Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.

Project coverage is 67.44%. Comparing base (a4aa682) to head (26c64e4).

Files	Patch %	Lines
.../server/routes/data_source_connection_validator.ts	60.00%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7026      +/-   ##
==========================================
- Coverage   67.44%   67.44%   -0.01%     
==========================================
  Files        3444     3444              
  Lines       67865    67873       +8     
  Branches    11027    11029       +2     
==========================================
+ Hits        45772    45777       +5     
- Misses      19429    19431       +2     
- Partials     2664     2665       +1

Flag	Coverage Δ
Linux_1	`33.07% <ø> (ø)`
Linux_2	`55.11% <ø> (ø)`
Linux_3	`45.22% <71.42%> (-0.01%)`	⬇️
Linux_4	`34.86% <ø> (ø)`
Windows_1	`33.10% <ø> (ø)`
Windows_2	`55.06% <ø> (ø)`
Windows_3	`45.24% <71.42%> (+<0.01%)`	⬆️
Windows_4	`34.86% <ø> (ø)`

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.

kavilla · 2024-06-14T08:49:01Z

src/plugins/data_source/common/data_sources/types.ts

@@ -52,3 +53,10 @@ export enum SigV4ServiceName {
 }

 export { DataSourceError } from './error';
+
+export enum DataSourceEngineType {
+  OpenSearch = 'OpenSearch',


How come not default?

If the data source then is undefined then it's assumed default. One thing I noticed related to the flint stuff passing around types with spaces causes some issues if the global url states.

vs using default which is the implied OpenSearch and is BWC if we assume default if no type explicitly defined.

I know in the repo it's a mixed casing with enums but not sure if using default will cause some issues lowercase so maybe

export enum DATA_SOURCE_ENGINE_TYPE { DEFAULT = 'default', SERVERLESS = 'serverless', LEGACY = 'legacy', NONE = 'none' }

the serverless banking off that we assume it's OpenSearch. The legacy because internally we reference it as legacy internally [example]

and NONE because it's matches the pattern to other enums in the repo [example]

Why is default needed?

This engine type is retrieved when the data source object is pulled together and assuming a default just makes no sense to the users where the default just mean OS cluster ? This info is not leveraged in global url states but maybe share the flint issues link if any so we can double check. (is flint even open sourced ?)

Per the legacy it's pretty vague where it could be meaning either Elasticsearch or Legacy OpenSearch (say when we have OpenSearch 3.0 out and OpenSearch 1.x retired which become Legacy OpenSearch). For supported data source types there are three now explicitly so given three exact type rather than giving general terms looks more accurate.

(is flint even open sourced ?)

but i dont think elasticsearch is either which is why internally codewise we refer to elasticsearch references as legacy

a default just makes no sense to the users where the default just mean OS cluster

do we display this info to the user in functional purposes? have we verified this with legal? previous requirements were no trademark displays from that legacy engine. so if we plan on displaying this info could be an issue. if not then i would consider that internally codewise since forking we reference elasticsearch as legacy. so it would be an antipattern

this info is not displayed to user, would like to follow up on this legal verification mentioned here, do you have any previous ref or poc from legal ? have no problem to switch all reference of elasticsearch to legacy

kavilla · 2024-06-14T08:55:04Z

src/plugins/data_source/common/data_sources/types.ts

@@ -10,6 +10,7 @@ export interface DataSourceAttributes extends SavedObjectAttributes {
  description?: string;
  endpoint: string;
  dataSourceVersion?: string;
+  dataSourceEngineType?: DataSourceEngineType;


do we need a migration? i dont think so right

OpenSearch-Dashboards/src/plugins/data_source/server/saved_objects/data_source.ts

Line 44 in a4aa682

'2.4.0': flow(migrateDataSource), // 2.4.0 is the version that introduces the datasource

it's optional field. so think it doesn't need it.

that's why this field is marked optional yes

kavilla · 2024-06-14T09:01:16Z

src/plugins/data_source/common/data_sources/types.ts

@@ -10,6 +10,7 @@ export interface DataSourceAttributes extends SavedObjectAttributes {
  description?: string;
  endpoint: string;
  dataSourceVersion?: string;


nit: prolly keeping in line with version which makes sense too.

but might be redundant to put dataSource in the field name since it's apart of the DataSource attributes. but i also understand the concept of data sources has different meanings even within this repo so might make sense to specify.

yeah you got the point, the dataSource prefix was added intentionally just to distinguish with other data source terms

kavilla · 2024-06-14T09:02:07Z

just an opinion on the enums: #7026 (comment)

but this is great thank you!

ZilongX · 2024-06-14T16:25:38Z

just an opinion on the enums: #7026 (comment)

but this is great thank you!

Thanks @kavilla, all great feedbacks ~~~

yujin-emma

Great, hope it can resolve the version filter well

ZilongX · 2024-06-14T18:49:38Z

Great, hope it can resolve the version filter well

Thanks @yujin-emma , yeah first baby step ...

xinruiba · 2024-06-14T18:52:09Z

src/plugins/data_source/server/routes/data_source_connection_validator.ts

+          if (
+            body.version.distribution !== null &&
+            body.version.distribution !== undefined &&
+            body.version.distribution === 'opensearch'


nit:
body.version.distribution === 'opensearch' should be enough here.
But no hurts to add !== null and !== undefined explicitly.

bandinib-amzn · 2024-06-14T18:56:22Z

src/plugins/data_source/server/routes/fetch_data_source_metadata.test.ts

    expect(result.body).toEqual({
      dataSourceVersion: '2.11.0',
+      dataSourceEngineType: 'OpenSearch',
      installedPlugins: ['opensearch-ml', 'opensearch-sql'],


Is this is the only place where we are comparing result.body during testing? What if we are testing result.body at other places like functional, integration? It will break, right? We will need to include dataSourceEngineType everyehwhere.

yes as dataSourceEngineType is marked as optional to ensure existing test won't fail

zhongnansu · 2024-06-14T19:03:08Z

src/plugins/data_source/server/routes/data_source_connection_validator.ts

+          if (
+            body.version.distribution !== null &&
+            body.version.distribution !== undefined &&
+            body.version.distribution === 'opensearch'


Suggested change

if (

body.version.distribution !== null &&

body.version.distribution !== undefined &&

body.version.distribution === 'opensearch'

if (

body.version.distribution === 'opensearch'

This can be simplified

… to data source saved object (#7026) * [Multiple Datasource][Version Decoupling] Add data source engine type to data source saved object Signed-off-by: Zilong Xia <zilongx@amazon.com> * Changeset file for PR #7026 created/updated --------- Signed-off-by: Zilong Xia <zilongx@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 190dab0) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

… to data source saved object (#7026) (#7033) * [Multiple Datasource][Version Decoupling] Add data source engine type to data source saved object * Changeset file for PR #7026 created/updated --------- (cherry picked from commit 190dab0) Signed-off-by: Zilong Xia <zilongx@amazon.com> 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> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

[Multiple Datasource][Version Decoupling] Add data source engine type…

09ac45c

… to data source saved object Signed-off-by: Zilong Xia <zilongx@amazon.com>

ZilongX requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, Flyingliuhub, curq, bandinib-amzn, SuZhou-Joe, ruanyl, BionIT, xinruiba, zhyuanqi and mengweieric as code owners June 13, 2024 23:36

github-actions bot added the seasoned-contributor label Jun 13, 2024

ZilongX added multiple datasource multiple datasource project version decoupling v2.16.0 and removed seasoned-contributor labels Jun 13, 2024

github-actions bot added the failed changeset label Jun 13, 2024

ZilongX removed the failed changeset label Jun 13, 2024

github-actions bot added the failed changeset label Jun 13, 2024

ZilongX removed the failed changeset label Jun 13, 2024

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

649c161

Merge branch 'main' into main

26c64e4

kavilla reviewed Jun 14, 2024

View reviewed changes

yujin-emma approved these changes Jun 14, 2024

View reviewed changes

xinruiba reviewed Jun 14, 2024

View reviewed changes

xinruiba approved these changes Jun 14, 2024

View reviewed changes

bandinib-amzn reviewed Jun 14, 2024

View reviewed changes

bandinib-amzn approved these changes Jun 14, 2024

View reviewed changes

zhongnansu reviewed Jun 14, 2024

View reviewed changes

ZilongX merged commit 190dab0 into opensearch-project:main Jun 14, 2024
71 checks passed

ZilongX added the backport 2.x label Jun 14, 2024

opensearch-trigger-bot bot mentioned this pull request Jun 14, 2024

[Backport 2.x] [Multiple Datasource][Version Decoupling] Add data source engine type to data source saved object #7033

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Multiple Datasource][Version Decoupling] Add data source engine type to data source saved object #7026

[Multiple Datasource][Version Decoupling] Add data source engine type to data source saved object #7026

ZilongX commented Jun 13, 2024 •

edited

Loading

github-actions bot commented Jun 13, 2024

github-actions bot commented Jun 13, 2024

github-actions bot commented Jun 13, 2024

github-actions bot commented Jun 13, 2024

codecov bot commented Jun 13, 2024 •

edited

Loading

kavilla Jun 14, 2024

ZilongX Jun 14, 2024 •

edited

Loading

kavilla Jun 14, 2024 •

edited

Loading

ZilongX Jun 24, 2024

kavilla Jun 14, 2024

ZilongX Jun 14, 2024

kavilla Jun 14, 2024

ZilongX Jun 14, 2024 •

edited

Loading

kavilla commented Jun 14, 2024

ZilongX commented Jun 14, 2024

yujin-emma left a comment

ZilongX commented Jun 14, 2024

xinruiba Jun 14, 2024

bandinib-amzn Jun 14, 2024

ZilongX Jun 14, 2024

zhongnansu Jun 14, 2024

[Multiple Datasource][Version Decoupling] Add data source engine type to data source saved object #7026

[Multiple Datasource][Version Decoupling] Add data source engine type to data source saved object #7026

Conversation

ZilongX commented Jun 13, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

github-actions bot commented Jun 13, 2024

❌ Changelog Entry Missing Hyphen

github-actions bot commented Jun 13, 2024

❌ Invalid Prefix For Manual Changeset Creation

github-actions bot commented Jun 13, 2024

❌ Invalid Prefix For Manual Changeset Creation

github-actions bot commented Jun 13, 2024

❌ Invalid Prefix For Manual Changeset Creation

codecov bot commented Jun 13, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

ZilongX Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

kavilla Jun 14, 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

ZilongX Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

kavilla commented Jun 14, 2024

ZilongX commented Jun 14, 2024

yujin-emma left a comment

Choose a reason for hiding this comment

ZilongX commented Jun 14, 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

ZilongX commented Jun 13, 2024 •

edited

Loading

codecov bot commented Jun 13, 2024 •

edited

Loading

ZilongX Jun 14, 2024 •

edited

Loading

kavilla Jun 14, 2024 •

edited

Loading

ZilongX Jun 14, 2024 •

edited

Loading