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.

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

[MDS] Add support for register data sources during the absence of local cluster #2140

Merged
merged 4 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
94 changes: 49 additions & 45 deletions public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ export class ObservabilityPlugin
constructor(initializerContext: PluginInitializerContext) {
this.config = initializerContext.config.get<PublicConfig>();
}
private featureFlagStatus: boolean = false;
Copy link
Member

Choose a reason for hiding this comment

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

can this variable naming be more explicit? looking at featureFlagStatus it's hard to tell what the features is..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I will change that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since the backport is failing, I will manually change it in 2.x first and backport. I will push another commit for change it on main.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have updated here #2144.


public setup(
core: CoreSetup<AppPluginStartDependencies>,
Expand All @@ -184,6 +185,7 @@ export class ObservabilityPlugin
});

setupOverviewPage(setupDeps.contentManagement!);
this.featureFlagStatus = !!setupDeps.dataSource;

// redirect legacy notebooks URL to current URL under observability
if (window.location.pathname.includes('notebooks-dashboards')) {
Expand Down Expand Up @@ -459,55 +461,57 @@ export class ObservabilityPlugin
return `${type.charAt(0).toUpperCase()}${type.slice(1)}`;
};

// register all s3 datasources
const registerDataSources = () => {
try {
core.http.get(`${DATACONNECTIONS_BASE}`).then((s3DataSources) => {
s3DataSources.map((s3ds) => {
dataSourceService.registerDataSource(
dataSourceFactory.getDataSourceInstance(S3_DATA_SOURCE_TYPE, {
id: htmlIdGenerator(OBS_S3_DATA_SOURCE)(),
name: s3ds.name,
type: s3ds.connector.toLowerCase(),
metadata: {
...s3ds.properties,
ui: {
label: s3ds.name,
typeLabel: getDataSourceTypeLabel(s3ds.connector.toLowerCase()),
groupType: s3ds.connector.toLowerCase(),
selector: {
displayDatasetsAsSource: false,
// register all s3 datasources only if mds feature flag is disabled
if (!this.featureFlagStatus) {
Copy link
Member

Choose a reason for hiding this comment

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

can we directly use setupDeps.dataSource here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we can't access the setupDeps from the start function, we can only access to the startDeps, However, the startDeps.dataSource.dataSourceEnabled is also not accessible, when the dataSource plugin is not registered.

Copy link
Member

Choose a reason for hiding this comment

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

got it. this makes sense for now. Ideally with should stop the registration at setup itself. Adding if condition here: https://github.com/opensearch-project/dashboards-observability/blob/3ad2bf0448f6a705f449f42e62be8398f840ea37/public/plugin.tsx#L264C11-L264C31

const registerDataSources = () => {
try {
core.http.get(`${DATACONNECTIONS_BASE}`).then((s3DataSources) => {
s3DataSources.map((s3ds) => {
dataSourceService.registerDataSource(
dataSourceFactory.getDataSourceInstance(S3_DATA_SOURCE_TYPE, {
id: htmlIdGenerator(OBS_S3_DATA_SOURCE)(),
name: s3ds.name,
type: s3ds.connector.toLowerCase(),
metadata: {
...s3ds.properties,
ui: {
label: s3ds.name,
typeLabel: getDataSourceTypeLabel(s3ds.connector.toLowerCase()),
groupType: s3ds.connector.toLowerCase(),
selector: {
displayDatasetsAsSource: false,
},
},
},
},
})
);
})
);
});
});
});
} catch (error) {
console.error('Error registering S3 datasources', error);
}
};

dataSourceService.registerDataSourceFetchers([
{ type: S3_DATA_SOURCE_TYPE, registerDataSources },
]);

if (startDeps.securityDashboards) {
core.http
.get(SECURITY_PLUGIN_ACCOUNT_API)
.then(() => {
registerDataSources();
})
.catch((e) => {
if (e?.response?.status !== 401) {
// accounts api should not return any error status other than 401 if security installed,
// this datasource register is included just in case
} catch (error) {
console.error('Error registering S3 datasources', error);
}
};

dataSourceService.registerDataSourceFetchers([
{ type: S3_DATA_SOURCE_TYPE, registerDataSources },
]);

if (startDeps.securityDashboards) {
core.http
.get(SECURITY_PLUGIN_ACCOUNT_API)
.then(() => {
registerDataSources();
}
});
} else {
registerDataSources();
})
.catch((e) => {
if (e?.response?.status !== 401) {
// accounts api should not return any error status other than 401 if security installed,
// this datasource register is included just in case
registerDataSources();
}
});
} else {
registerDataSources();
}
}

core.http.intercept({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Compatible with OpenSearch and OpenSearch Dashboards version 2.17.0
* [FEATURE] MDS support in Integrations for observability plugin ([#2051](https://github.com/opensearch-project/dashboards-observability/pull/2051))
* [Feature] Logs UI update ([#2092](https://github.com/opensearch-project/dashboards-observability/pull/2092))
* feat: make createAssets API compatible with workspace ([#2101](https://github.com/opensearch-project/dashboards-observability/pull/2101))
* [Page Header] New page header for notebooks and UI updates ([#2099](https://github.com/opensearch-project/dashboards-observability/pull/2099), [#2103](https://github.com/opensearch-project/dashboards-observability/pull/2203))
* [Page Header] New page header for notebooks and UI updates ([#2099](https://github.com/opensearch-project/dashboards-observability/pull/2099), [#2103](https://github.com/opensearch-project/dashboards-observability/pull/2099))
* [Feature] OverviewPage made with Content Management ([#2077](https://github.com/opensearch-project/dashboards-observability/pull/2077))

### Enhancement
Expand All @@ -24,6 +24,7 @@ Compatible with OpenSearch and OpenSearch Dashboards version 2.17.0
* [query assist] update api handler to accommodate new ml-commons config response ([#2111](https://github.com/opensearch-project/dashboards-observability/pull/2111))
* Update trace analytics landing page ([#2125](https://github.com/opensearch-project/dashboards-observability/pull/2125))
* [query assist] update ml-commons response schema ([#2124](https://github.com/opensearch-project/dashboards-observability/pull/2124))
* [MDS] Add support for register data sources during the absence of local cluster ([#2140](https://github.com/opensearch-project/dashboards-observability/pull/2140))

### Bug Fixes
* [Bug] Trace Analytics bug fix for local cluster being rendered ([#2006](https://github.com/opensearch-project/dashboards-observability/pull/2006))
Expand Down
Loading