Use asExternalUri For Server Url #173

williamsyang-work · 2023-08-05T02:03:54Z

~~Requires: eclipse-cdt-cloud/vscode-trace-server#11~~

This allows for automatic port forwarding of trace-server api calls when using remote.

Signed-off-by: Will Yang [email protected]

williamsyang-work · 2023-08-09T19:13:40Z

Sometimes after the port is forwarded, the explorer webviews do not refresh to load/show the new data from the trace-server. The user has to navigate away and return to the trace-extension to refresh the views.

There may be other UX flow issues with these changes. I can address them if needed but thought the bugs could be documented and addressed as a separate issue.

williamsyang-work · 2023-08-09T19:20:34Z

I tested this by:

building the .vsix files
installing them onto vscode while connected via remote-ssh.
using plugin while connected vis remote-ssh.

Using the [Extension Development Host] while connected with remote-ssh also seems to work.
EDIT: You will need to install the .vsix for one plugin since AFAIK only one can be tested at a time using the debugger.
Just make sure that the port is not being forwarded on another open editor.

marco-miller

Thanks!

vscode-trace-extension/src/extension.ts

marco-miller

LGTM, yet letting @bhufmann cover the final assessment.

bhufmann · 2023-08-09T20:15:40Z

vscode-trace-extension/src/extension.ts

+        myAnalysisProvider.updateTraceServerUrl(extUriString);
+        TraceViewerPanel.updateTraceServerUrl(extUriString);
+    };
+


@williamsyang-work a question, what if a user executes the command Trace Server: Start (if stopped) manually using the command palette, shouildn't updateUris() be called as well? Maybe when command serverStatus.started is executed? Please let me know.

@bhufmann I tested starting and stopping the trace server via command palette and it worked as-is for me.
My understanding is: as long as server url remains the same we don't need to update it. The process on that port can stop, start, restart, etc.

@williamsyang-work When connecting to the remote host and executing the command Trace Server: Start (if stopped) then the server is started, but the port forwarding is not working because updateUris() and hencevscode.asExternalUrl is not executed. Start and stop is still working in this case. I tested that case with a remote running in a virtual machine and I saw it not working.

However, when using the open trace command that starts the server and updateUris() is executed will enable port forwarding for the manual start command.

I think we need to address the manual start case as well.

Per our discussion updateUris() is now called in the serverStatus.started command. This updates the Trace-Explorer webviews and starts port forwarding.

There seems to be another bug where it shows the server's data accurately, but then changes back to 'No Traces Open: Open Trace' state after some time.

bhufmann · 2023-08-10T18:43:46Z

@williamsyang-work I noticed that when I close the remote vscode window, the server is not automatically stopped and keeps running afterwards. I had to go to a shell and kill the process. Do you see this behaviour as well?

williamsyang-work · 2023-08-10T20:02:55Z

@williamsyang-work I noticed that when I close the remote vscode window, the server is not automatically stopped and keeps running afterwards. I had to go to a shell and kill the process. Do you see this behaviour as well?

Yes, this is the behavior I get as well.

williamsyang-work · 2023-08-10T20:04:28Z

The url that's returned from server.start-if-stopped command is no longer used. Therefore, there is no longer a need for the changes in vscode-trace-server repo.

bhufmann · 2023-08-11T14:35:07Z

vscode-trace-extension/src/extension.ts

@@ -151,7 +157,7 @@ async function startTraceServerIfAvailable(): Promise<void> {
    if (!traceServerExtension || await isUp()) {
        return;
    }
-    await vscode.commands.executeCommand(extensionId + '.start-if-stopped');
+    vscode.commands.executeCommand(extensionId + '.start-if-stopped');


This breaks the local installation. You need to keep the await here. Otherwise opening from file explorer (right mouse click on trace -> Open with Trace Viewer) will fail because server is not ready when continuing.

bhufmann · 2023-08-11T14:39:26Z

vscode-trace-extension/src/extension.ts

@@ -151,7 +157,7 @@ async function startTraceServerIfAvailable(): Promise<void> {
    if (!traceServerExtension || await isUp()) {


So, the port forwarding is only configured after startTraceServerIfAvailable() is executed. However, in this function isUp() is executed which does a healthCheck() http call to the server. So, it will fail if port forwarding is not done.

So, we need to re-think the execution, i.e. what should be done when and in what order.

bhufmann

I noticed that once the asExternalUri is called the port forwarding is configured and visible in the Ports view. Stopping the trace server doesn't affect that this setting. What can happen that there is a TSP call to the trace server and the forwarded port is not served by the remote server. The HTTP message is pending (use developer tools to reproduce that). Even when starting the sever, these pending messages block all subsequent http requests which makes the trace viewer unusable. To unblock that, the use has to open the Ports view and stop the port forwarding. Doing that the pending message are marked as failed, and afterwards the trace viewer is working again.

I think, we need to investigate a bit more and find a away to avoid the situation with hanging messages and blocked application.

bhufmann · 2023-08-11T19:08:24Z

@williamsyang-work I noticed that when I close the remote vscode window, the server is not automatically stopped and keeps running afterwards. I had to go to a shell and kill the process. Do you see this behaviour as well?

Yes, this is the behavior I get as well.

Thanks for confirming. I did some investigation and with some logging (to file) I was able to confirm, that the deactivate() method of the vscode-trace-server is being called. However, the call to tree-kill(pid) has no effect. More investigation needed to fix stopping the trace server on exit.

bhufmann · 2023-08-14T18:36:18Z

@williamsyang-work I noticed that when I close the remote vscode window, the server is not automatically stopped and keeps running afterwards. I had to go to a shell and kill the process. Do you see this behaviour as well?

Yes, this is the behavior I get as well.

Thanks for confirming. I did some investigation and with some logging (to file) I was able to confirm, that the deactivate() method of the vscode-trace-server is being called. However, the call to tree-kill(pid) has no effect. More investigation needed to fix stopping the trace server on exit.

I did some more digging to figure out what's going on. I think what's happening is that VsCode exits before the treeKill() command is executed. treeKill() spawns external processes to get all the children pids from the parent pid of the trace server launcher. It reads the stdout of the command ps -o pid --no-headers --ppid $ppid and then uses the collected list of pids to call process.kill on it. This is done in a callback, but the parent VsCode application already exited and the hence the no process is killed. I was able to make it work by adding some async/wait/timeout call to delay the execution.

Please note that this can also happen in the local case. I guess, that the shutdown of a local VsCode application takes longer than closing the remote processes.

This allows for automatic port forwarding of trace-server api calls when using remote. Signed-off-by: Will Yang <[email protected]>

williamsyang-work marked this pull request as draft August 5, 2023 02:09

williamsyang-work force-pushed the remote-fixes branch from 24dfdd2 to 1cb4fe3 Compare August 9, 2023 17:57

williamsyang-work marked this pull request as ready for review August 9, 2023 17:58

williamsyang-work force-pushed the remote-fixes branch from 1cb4fe3 to 0fd19c9 Compare August 9, 2023 18:25

marco-miller self-requested a review August 9, 2023 19:06

marco-miller requested changes Aug 9, 2023

View reviewed changes

vscode-trace-extension/src/extension.ts Outdated Show resolved Hide resolved

vscode-trace-extension/src/extension.ts Outdated Show resolved Hide resolved

vscode-trace-extension/src/extension.ts Outdated Show resolved Hide resolved

marco-miller requested a review from bhufmann August 9, 2023 19:41

williamsyang-work force-pushed the remote-fixes branch from 0fd19c9 to 351d3dc Compare August 9, 2023 19:49

marco-miller approved these changes Aug 9, 2023

View reviewed changes

bhufmann reviewed Aug 9, 2023

View reviewed changes

williamsyang-work force-pushed the remote-fixes branch from 351d3dc to ad01fce Compare August 10, 2023 19:58

williamsyang-work force-pushed the remote-fixes branch 2 times, most recently from 543eea1 to 8857562 Compare August 11, 2023 16:48

bhufmann reviewed Aug 11, 2023

View reviewed changes

bhufmann requested changes Aug 11, 2023

View reviewed changes

bhufmann mentioned this pull request Aug 15, 2023

Trace Server not stopped when deactivating extension during shutdown eclipse-cdt-cloud/vscode-trace-server#13

Closed

hriday-panchasara mentioned this pull request Aug 23, 2023

Fix stopping trace server during shutdown eclipse-cdt-cloud/vscode-trace-server#14

Merged

williamsyang-work force-pushed the remote-fixes branch from 8857562 to fd9094d Compare October 17, 2023 19:31

Use asExternalUri For Server Url

3c7802d

This allows for automatic port forwarding of trace-server api calls when using remote. Signed-off-by: Will Yang <[email protected]>

williamsyang-work force-pushed the remote-fixes branch from fd9094d to 3c7802d Compare October 18, 2023 18:08

williamsyang-work added 2 commits October 23, 2023 22:24

kind of working. can't open traces

193c3ef

commiting last code bits for analysis

d84eb76

williamsyang-work closed this Jan 25, 2024

williamsyang-work mentioned this pull request Jan 30, 2024

Use asExternalUri For Server Url #196

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use asExternalUri For Server Url #173

Use asExternalUri For Server Url #173

williamsyang-work commented Aug 5, 2023 •

edited

Loading

williamsyang-work commented Aug 9, 2023 •

edited

Loading

williamsyang-work commented Aug 9, 2023 •

edited

Loading

marco-miller left a comment

marco-miller left a comment

bhufmann Aug 9, 2023

williamsyang-work Aug 9, 2023 •

edited

Loading

bhufmann Aug 10, 2023

williamsyang-work Aug 10, 2023 •

edited

Loading

bhufmann commented Aug 10, 2023

williamsyang-work commented Aug 10, 2023

williamsyang-work commented Aug 10, 2023 •

edited

Loading

bhufmann Aug 11, 2023

bhufmann Aug 11, 2023

bhufmann left a comment

bhufmann commented Aug 11, 2023 •

edited

Loading

bhufmann commented Aug 14, 2023

		@@ -151,7 +157,7 @@ async function startTraceServerIfAvailable(): Promise<void> {
		if (!traceServerExtension \|\| await isUp()) {

Use asExternalUri For Server Url #173

Use asExternalUri For Server Url #173

Conversation

williamsyang-work commented Aug 5, 2023 • edited Loading

williamsyang-work commented Aug 9, 2023 • edited Loading

williamsyang-work commented Aug 9, 2023 • edited Loading

marco-miller left a comment

Choose a reason for hiding this comment

marco-miller left a comment

Choose a reason for hiding this comment

bhufmann Aug 9, 2023

Choose a reason for hiding this comment

williamsyang-work Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

bhufmann Aug 10, 2023

Choose a reason for hiding this comment

williamsyang-work Aug 10, 2023 • edited Loading

Choose a reason for hiding this comment

bhufmann commented Aug 10, 2023

williamsyang-work commented Aug 10, 2023

williamsyang-work commented Aug 10, 2023 • edited Loading

bhufmann Aug 11, 2023

Choose a reason for hiding this comment

bhufmann Aug 11, 2023

Choose a reason for hiding this comment

bhufmann left a comment

Choose a reason for hiding this comment

bhufmann commented Aug 11, 2023 • edited Loading

bhufmann commented Aug 14, 2023

williamsyang-work commented Aug 5, 2023 •

edited

Loading

williamsyang-work commented Aug 9, 2023 •

edited

Loading

williamsyang-work commented Aug 9, 2023 •

edited

Loading

williamsyang-work Aug 9, 2023 •

edited

Loading

williamsyang-work Aug 10, 2023 •

edited

Loading

williamsyang-work commented Aug 10, 2023 •

edited

Loading

bhufmann commented Aug 11, 2023 •

edited

Loading