eclipse-cdt-cloud / theia-trace-extension

Eclipse Theia trace viewer extension using the Trace Server Protocol (TSP), through the tsp-typescript-client. Also the home for reusable JavaScript libraries: traceviewer-base, traceviewer-react-components
MIT License
49 stars 60 forks source link

Refactor server communication to JSON-RPC #976

Closed sgraband closed 1 year ago

sgraband commented 1 year ago

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.

bhufmann commented 1 year ago

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:

Theia FE <--JSON RPC--> Theia BE <--TSP over HTTP--> Trace Server

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 Bernd

sgraband commented 1 year ago

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 TspClient. On a first glance most parameters and return values seem to be serializable, so we should be able to integrate with the Theia RPC mechanism without requiring too many modifications or customizations. The proxied TspClient is then handed over to the rendering code instead of a "real" instance.

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 https://github.com/eclipse-theia/theia/issues/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.

bhufmann commented 1 year ago

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 bigint to allow long timestamps. bigint can't be serialised/deserialised with the standard JSON converter. I just wanted to mention that in case you run into this issue. If the deserialisation is only done once at the end of the chain then there shouldn't be an issue.

bhufmann commented 1 year ago

Do you have any time frame when this can be contributed? Please let me know. Thanks.

sgraband commented 1 year ago

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?

bhufmann commented 1 year ago

Perfect. That's fast. Looking forward seeing the code.

sgraband commented 1 year ago

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.

bhufmann commented 1 year ago

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.