Refactor server communication to JSON-RPC #976
Comments
|
Thank you very much for this proposal. We are interested. I have some comment and questions about this. The idea to directly communicate to the Trace Server was to minimize the latency for back-end calls as well as to allow the trace server to be deployed outside from the place where the traces are location. The latter case is for future and not supported at all. However, I see the advantage of not having to do special environment setup in the cloud deployment case. First of all, I as far as I understand, the communication with the trace server will be basically like this:
Not know the implementation details, I'm wondering how the translation from JSON RPC to the HTTP message will be done and how the additional latency can be minimized. Could you please elaborate about this? Secondly, we are currently working on making a VsCode trace extension that can be installed in Theia as well as VsCode. Any suggestion on how a similar JSON RPC communication could be achieved when installing the vscode-trace-extension in Theia and VsCode? I mean the back-end / front-end separation for cloud deployment. Regards |
|
Thank you for the response! First of all, your understanding of the suggested communication flow is correct. Regarding your first question: Generally speaking the idea is to encapsulate the communication into a client whose implementation then differs according to the environment. For an example of this you could take a look at the ModelServerClient for the eclipse-emfcloud/emfcloud-modelserver. It is basically the architecure that we would propose. It consists of:
Looking at the existing Trace Compass code we realized that you already use a client pattern with the Regarding performance: Generally speaking the new communication will have a higher latency because there is an additional call in the communication flow. However this additional call is then usually performed locally as the Theia backend and TraceCompass Server likely run on the same machine. Theia's JSON-RPC mechanism is already optimized to not introduce too much latency (see eclipse-theia/theia#10684). Clicking around in the example Trace Viewer doesn't seem to create too many requests so naively we would expect the additional latency to not be noticeable by an user. In any case, the behavior can be made easily customizable by running the TspClient in the frontend instead of the backend. So if any developer would prefer the client side communication for their use cases this would still be supported. Within the VSCode extension the same patterns can be used. Just instead of using Theia's RPC utils to create a TspClient proxy you would need to implement an own proxy running in the webview. This proxy then uses the message mechanism between webview and extension to delegate the calls to the TspClient running within the extension (i.e. the VS Code / Theia Node backend). Should it be necessary you could make this configurable too and instantiate the real client in the webview if you see this as a valid use case. |
|
Thanks a lot for all the information. I think some additional questions might pop up when reviewing and integrating this proposal. About the latency, right now the biggest contributors to the latency is the serialisation server-side and deserialisation client-side of larger messages. The transport latency was a only small percentage in that case. As long as there is no additional serialisation/deserialisation then the added latency should be ok. Please note, that in the tsp-typescript-client there are conversions to variables of type |
|
Do you have any time frame when this can be contributed? Please let me know. Thanks. |
|
I will start on monday, but i have short week next week. I can probably provide a PR for this the week after, if this is fine by you? |
|
Perfect. That's fast. Looking forward seeing the code. |
|
Just a quick update: I was able to migrate the communication to JSON-RPC, however i still need some time to test and take a look at some of the specifics of the extension, like changing the url of the server via a setting and so on. So i will likely be able to open a PR sometime next week, if this is fine. |
Thanks for the update. |
Base package: - created a new TspFrontendClient interface that follows the TspClient - each tspClient is now either a TspFrontendClient or a TspClient - this is also true for all the react components - this way all tspClients can be used with or without JSON-RPC - note that this interface could be moved to tsp-typescript-client - then the TspClient could also follow the same interface Theia-extension: - a implementation for the TspFrontendClient is provided - it calls the JSON-RPC proxy on the backend and transforms the responses to TspClientResponses so they can be handled as before - the backend proxy simply the urlPromise and returns a lazyTspClient - the trace-server-url-provider-impl was also moved to the backend - this way the env variable can directly be read out from process.env - for the port preference a proxy was setup in the url-provider - with it the url provider gets notified about port changes - the trace-server-connection-status was also moved to the backend - the frontend client for the connection-status registers on the backend - the backend then pings all registered clients when the status changed Fixes eclipse-cdt-cloud#976 Contributed on behalf of STMicroelectronics
The `TspClient` is now an interface that can be used for other implementations. The existing functionality was moved to the `DefaultTspClient`. This means adopters now need to initialize the `DefaultTspClient` type instead of `TspClient` to get the default behavior. However this makes the package more flexible as different implementations can be provided, e.g. a proxy client in a Theia frontend. Part of eclipse-cdt-cloud/theia-trace-extension#976 Contributed on behalf of STMicroelectronics
The new interface is called `ITspClient`. `ITspClient` can be used for typing and other implementations. The existing functionality was moved to the `HttpTspClient`. To not break the existing behavior `TspClient` is still exported as an alias for the `HttpTspClient`, but is being deprecated. This way adopters should be notified to migrate to `HttpTspClient`. This makes the package more flexible as different implementations can be provided, e.g. a proxy client in a Theia frontend. Part of eclipse-cdt-cloud/theia-trace-extension#976 Contributed on behalf of STMicroelectronics
The new interface is called `ITspClient`. `ITspClient` can be used for typing and other implementations. The existing functionality was moved to the `HttpTspClient`. To not break the existing behavior `TspClient` is still exported as an alias for the `HttpTspClient`, but is being deprecated. This way adopters should be notified to migrate to `HttpTspClient`. This makes the package more flexible as different implementations can be provided, e.g. a proxy client in a Theia frontend. Part of eclipse-cdt-cloud/theia-trace-extension#976 Contributed on behalf of STMicroelectronics
The new interface is called `ITspClient`. `ITspClient` can be used for typing and other implementations. The existing functionality was moved to the `HttpTspClient`. To not break the existing behavior `TspClient` is still exported as an alias for the `HttpTspClient`, but is being deprecated. This way adopters should be notified to migrate to `HttpTspClient`. This makes the package more flexible as different implementations can be provided, e.g. a proxy client in a Theia frontend. Part of eclipse-cdt-cloud/theia-trace-extension#976 Contributed on behalf of STMicroelectronics
TspClient Proxy: Provide an implementation of the ITspClient called `TheiaRpcTspProxy`. The implementation forwards all calls to a JSON RPC proxy called `TheiaClientProxy`. Responses are then mapped to a `TspClientResponse`. On the backend the `TheiaClientProxy` returns a lazyTspClient. The lazyTspClient returns a `HttpTspClient` (default implementation). Trace Server url provider: The implementation of the provider was moved to the backend. This way the env variable can be read out directly. For the port preference a proxy mechanism was added. Changes to the preference are sent via JSON RPC and handled by the url provider. Trace Server connection status: The implementation was moved to the backend. Frontends can register themselves on the backend. Each registered frontend will be pinged when the status has changed. Misc: Use `ITspClient` instead of `TspClient` for types, as `TspClient` is deprecated. Fixes eclipse-cdt-cloud#976 Contributed on behalf of STMicroelectronics
TspClient Proxy: Provide an implementation of the ITspClient called `TheiaRpcTspProxy`. The implementation forwards all calls to a JSON RPC proxy called `TheiaClientProxy`. Responses are then mapped to a `TspClientResponse`. On the backend the `TheiaClientProxy` returns a lazyTspClient. The lazyTspClient returns a `HttpTspClient` (default implementation). Trace Server url provider: The implementation of the provider was moved to the backend. This way the env variable can be read out directly. For the port preference a proxy mechanism was added. Changes to the preference are sent via JSON RPC and handled by the url provider. Trace Server connection status: The implementation was moved to the backend. Frontends can register themselves on the backend. Each registered frontend will be pinged when the status has changed. Misc: Use `ITspClient` instead of `TspClient` for types, as `TspClient` is deprecated. Fixes eclipse-cdt-cloud#976 Contributed on behalf of STMicroelectronics
By default the Theia extension and the React components do not communicate with the Trace Compass server via the Theia backend, but rather connect to a separate exposed port (e.g. 8080) via REST. This approach has some downsides. For example in cloud deployments the additional port needs to be exposed as well.
We would like to suggest to refactor the communication in the Theia extension to delegate the communication via Theia's JSON-RPC from the Theia frontend over the Theia backend to the Trace Compass server and back.
Of course the pure React components should stay independent of Theia and allow to connect to arbitrary end points.
Are you interested in this change? If yes, we will gladly contribute an initial implementation.
The text was updated successfully, but these errors were encountered: