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

Feature: MDS client enhancement #706

Draft
wants to merge 6 commits into
base: 2.x
Choose a base branch
from
Draft
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
3 changes: 2 additions & 1 deletion opensearch_dashboards.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
],
"requiredPlugins": [
"navigation",
"opensearchDashboardsReact"
"opensearchDashboardsReact",
"dataSource"
],
"optionalPlugins": [
"managementOverview"
Expand Down
111 changes: 111 additions & 0 deletions server/client/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { get } from "lodash";
import {
IContextProvider,
ILegacyScopedClusterClient,
OpenSearchDashboardsRequest,
RequestHandler,
RequestHandlerContext,
} from "opensearch-dashboards/server";
import { IRequestHandlerContentWithDataSource, IGetClientProps, DashboardRequestEnhancedWithContext } from "./interface";

export const getClientSupportMDS = (props: IGetClientProps) => {
const originalAsScoped = props.client.asScoped;
const handler: IContextProvider<RequestHandler<unknown, unknown, unknown>, "core"> = (
context: RequestHandlerContext,
request: OpenSearchDashboardsRequest
) => {
(request as DashboardRequestEnhancedWithContext)[`${props.pluginId}_context`] = context as IRequestHandlerContentWithDataSource;
Copy link
Member

Choose a reason for hiding this comment

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

I remember you mentioned the concern of putting _data_source_id_ in http headers and the challenge of moving it to url search parameters.

Since we modified the original request here anyway(though it looks a bit hacky as registerRouteHandlerContext is not intent for this). If we end up with modifying the request object, perhaps we can also do this to handle data source id in search parameters:

    core.http.registerRouteHandlerContext(`${props.pluginId}_MDS_CTX_SUPPORT`, (ctx, req) => {
      req[`${props.pluginId}_context`] = ctx;
      if (req.url.searchParams.has('__data_source_id__')) {
        // add data source id to request object so later access
        req[`${props.pluginId}_data_source_id`] = req.url.searchParams.get('__data_source_id__');
        // then we can remove it from search params so it won't bother router validators.
        req.url.searchParams.delete('__data_source_id__');
      }
      return {};
    });

return {} as any;
};

/**
* asScoped can not get the request context,
* add _context to request
*/
props.core.http.registerRouteHandlerContext(`${props.pluginId}_MDS_CTX_SUPPORT` as "core", handler);

/**
* it is not a good practice to rewrite the method like this
* but JS does not provide a method to copy a class instance
*/
props.client.asScoped = function (request: DashboardRequestEnhancedWithContext): ILegacyScopedClusterClient {
Copy link
Member

@ruanyl ruanyl May 19, 2023

Choose a reason for hiding this comment

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

What do you think of using Proxy instead of overwriting the original method? Something like this:

    const proxy = new Proxy(legacyClient, {
      get(target, prop) {
        if (prop === 'asScoped') {
          return (request) => {
            if (/* data source id presents */) {
              return {
                callAsCurrentUser: async () => {/.../},
                callAsInternalUser: async () => {/.../},
              };
            }
            return target.asScoped(request);
          };
        }
        return Reflect.get(...arguments);
      },
    });
    // then use legacyClient proxy
    const indexService = new IndexService(proxy);

const context = request[`${props.pluginId}_context`];

/**
* If the context can not be found
* reject the request and add a log
*/
if (!context) {
const errorMessage = "There is some error between dashboards and your remote data source, please retry again.";
props.logger.error(errorMessage);
return {
callAsCurrentUser: () => Promise.reject(errorMessage),
callAsInternalUser: () => Promise.reject(errorMessage),
};
}

const dataSourceId = props.getDataSourceId?.(context, request);
/**
* If no dataSourceId provided
* use the original client
*/
if (!dataSourceId) {
props.logger.debug("No dataSourceId, using original client");
return originalAsScoped.call(props.client, request);
}

const callApi: ILegacyScopedClusterClient["callAsCurrentUser"] = async (...args) => {
const [endpoint, clientParams, options] = args;
return new Promise(async (resolve, reject) => {
props.logger.debug(`Call api using the data source: ${dataSourceId}`);
try {
const dataSourceClient = await context.dataSource.opensearch.getClient(dataSourceId);

/**
* extend client if needed
**/
Object.assign(dataSourceClient, { ...props.onExtendClient?.(dataSourceClient) });

/**
* Call the endpoint by providing client
* The logic is much the same as what callAPI does in Dashboards
*/
const clientPath = endpoint.split(".");
const api: any = get(dataSourceClient, clientPath);
let apiContext = clientPath.length === 1 ? dataSourceClient : get(dataSourceClient, clientPath.slice(0, -1));
const request = api.call(apiContext, clientParams);

/**
* In case the request is aborted
*/
if (options?.signal) {
options.signal.addEventListener("abort", () => {
request.abort();
reject(new Error("Request was aborted"));
});
}
const result = await request;
resolve(result.body || result);
} catch (e: any) {
/**
* TODO
* ask dashboard team to add original error to DataSourceError
* so that we can make the client behave exactly the same as legacy client
*/
reject(e);
}
});
};

/**
* Return a legacy-client-like client
* so that the callers no need to change their code.
*/
const client: ILegacyScopedClusterClient = {
callAsCurrentUser: callApi,
callAsInternalUser: callApi,
};
return client;
};
return props.client;
};
41 changes: 41 additions & 0 deletions server/client/interface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { OpenSearchDashboardsClient } from "@opensearch-project/opensearch/api/opensearch_dashboards";
import {
CoreSetup,
ILegacyCustomClusterClient,
LegacyCallAPIOptions,
Logger,
OpenSearchDashboardsRequest,
RequestHandlerContext,
} from "opensearch-dashboards/server";

export interface IRequestHandlerContentWithDataSource extends RequestHandlerContext {
Copy link
Member

Choose a reason for hiding this comment

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

data source plugin registered client objects at https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/data_source/server/plugin.ts#L99 , not sure if that can be reused

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point and plugin should add dataSource plugin as requiredPlugins, add this change into this PR.

dataSource: {
opensearch: {
getClient: (dataSourceId: string) => OpenSearchDashboardsClient;
legacy: {
getClient: (
dataSourceId: string
) => {
callAPI: (endpoint: string, clientParams?: Record<string, any>, options?: LegacyCallAPIOptions) => Promise<unknown>;
};
};
};
};
}

export interface IGetClientProps {
core: CoreSetup;
/**
* We will rewrite the asScoped method of your client
* It would be better that create a new client before you pass in one
*/
client: ILegacyCustomClusterClient;
onExtendClient?: (client: OpenSearchDashboardsClient) => Record<string, any> | undefined;
getDataSourceId?: (context: RequestHandlerContext, request: OpenSearchDashboardsRequest) => string | undefined;
pluginId: string;
logger: Logger;
}

export type DashboardRequestEnhancedWithContext = OpenSearchDashboardsRequest & {
[contextKey: string]: IRequestHandlerContentWithDataSource;
};