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

Consider adopting the proposed Port Attributes API #21292

Closed
alexr00 opened this issue May 24, 2023 · 7 comments · Fixed by microsoft/vscode-python-debugger#140
Closed

Consider adopting the proposed Port Attributes API #21292

alexr00 opened this issue May 24, 2023 · 7 comments · Fixed by microsoft/vscode-python-debugger#140
Assignees
Labels
author-verification-requested Issues potentially verifiable by issue author feature-request Request for new features or functionality needs PR Ready to be worked on

Comments

@alexr00
Copy link
Member

alexr00 commented May 24, 2023

Original post: microsoft/vscode#115616 (comment)

In a remote environment (Codespaces, Dev Containers, etc.) VS Code will automatically detect and forward ports that it finds processes listening on so that users can access the remote application they're developing from their local machine. This includes debugees, such as a Python program that the user is debugging.

The auto port forwarding can't distinguish between a useful port to forward (ex. the app that you're debugging) and a useless port (ex. the port that the debugger itself uses for communication). This results in noisy extra ports being forwarded. We've seeing this in other extensions, such as the JS Debug extension.

The proposed solution for this is to allow extension to provide port attributes for ports that they know about. The JS Debug extension has done this for the debug ports it uses: https://github.com/microsoft/vscode-js-debug/blob/0d0704da3726653e2d806be528b8ab51fa952cf0/src/ui/portAttributesProvider.ts#L39-L55

We're working on finalizing the port attributes API, so I don't expect too many more changes. In support of finalizing the API, and to remove the port-clutter that comes from forwarding Python extension debug ports, it would be great if the Python extension adopted the proposed port attributes API.

@alexr00 alexr00 added the feature-request Request for new features or functionality label May 24, 2023
@github-actions github-actions bot added the triage-needed Needs assignment to the proper sub-team label May 24, 2023
@alexr00
Copy link
Member Author

alexr00 commented May 24, 2023

Python folks, if you want I can specifically assign a Python extension dev to my port attributes test plan item for endgame next week.

@paulacamargo25 paulacamargo25 added needs PR Ready to be worked on and removed triage-needed Needs assignment to the proper sub-team labels May 24, 2023
@karthiknadig
Copy link
Member

Here are few scenarios we are creating ports, and where I think forwarding will be required and where it is not.

Debugger scenarios

I had a discussion with debugpy team, the underlying debugger where we were planning on using this API. The debugger itself communicates with subprocess that it starts over ports, but those don't need to forward. Those are ephemeral and can be ignored for this case.

Cases to consider:

  1. debugpy listen mode: In this mode VS Code launches the Debug Adapter with the intent that the Debug adapter will listen for connections from the debugger. This is a scenario where the debugger is installed independently of VS Code or Python extension, say like in a container that the user wants to debug. The client that launched debug adapter will have a port provided by the user that it opens in a listening mode, and the debugger in the container connects to the debugger as an outgoing connection from the container.

  2. debugpy connect mode: In this mode VS Code directly connects to debugpy which is running in server mode. This is another case where typically it is a container or a remoter server that is running debugpy in it and may not contain VS Code server in it.

I don't think the above two cases are where the API will be used.

Web app scenarios

These are cases that involve running some web app framework, like django, fastapi, flask etc. There are two possibilities here:

  1. Run without debugging/run in terminal case: This is where the web app is run either without debugger or is being run in terminal directly by user or run in terminal using a command. If we can detect the port that is opened by the web app framework then we could provide the right attributes for it to be forwarded.

  2. Debugging a web app launched with server ready: Server ready is a VS Code launch json configuration that automatically detects web server string in the terminal using patterns provided by the user. This is often used to trigger browser launch after the server has started. This seems like a case where, VS Code itself should be able to detect and forward the port without requiring input from extension?

  3. Debugging a web app launched without out server ready: This is another case where we will have to monitor the content being sent to the terminal, but a key difference here is that debugger can capture stdout and redirect it as a DAP message. So extension can use DAP tracker API from VS Code to monitor and provide port attributes.

Data Science

  1. Tensor Board: I think tensor board uses ports in some form so this API could be used there to provide the right attributes.

@alexr00
Copy link
Member Author

alexr00 commented Jun 7, 2023

The debugger scenarios sound reasonable to me.

For the web app scenarios:

  1. Run without debugging/run in terminal case: This is where the web app is run either without debugger or is being run in terminal directly by user or run in terminal using a command. If we can detect the port that is opened by the web app framework then we could provide the right attributes for it to be forwarded.

VS Code will automatically forward this anyway. VS Code detects ports in two possible ways based on what settings the user has: watching the proc filesystem and/or watching terminal/debug output. It sounds like this is also desirable behavior.

  1. Debugging a web app launched with server ready: Server ready is a VS Code launch json configuration that automatically detects web server string in the terminal using patterns provided by the user. This is often used to trigger browser launch after the server has started. This seems like a case where, VS Code itself should be able to detect and forward the port without requiring input from extension?

Yes, the port will be automatically forwarded.

  1. Debugging a web app launched without out server ready: This is another case where we will have to monitor the content being sent to the terminal, but a key difference here is that debugger can capture stdout and redirect it as a DAP message. So extension can use DAP tracker API from VS Code to monitor and provide port attributes.

If you want to have the port forwarded in this case then you shouldn't have to do anything.

@alexr00
Copy link
Member Author

alexr00 commented Aug 16, 2023

@paulacamargo25 I'm ready to finalize the port attributes API. I'd love to get feedback from you or someone else on the Python team before I do so. Is adopting this API work that is planned for September?

@karthiknadig
Copy link
Member

@alexr00 This is planned for September Milestone.

Adding debugger owners to provide any input they have. @int19h @AdamYoblick

@int19h
Copy link

int19h commented Aug 16, 2023

@alexr00 I had a look, and my main concern is the inherent asynchrony between the debugger processes and VSCode. For internal ports, to avoid clashing with anything else, the debugger typically does listen(0) to let the OS allocate an ephemeral port - so it doesn't actually know the port number until it starts listening. On the other hand, it needs to communicate that port number back to the Python extension for the latter to be able to respond to providePortAttributes correctly. We can communicate it using a custom DAP message or through some other mechanism, but the problem is that, so far as I can tell, there's no guarantee that providePortAttributes call is scheduled after that custom message has been parsed. I would expect similar issues with any out-of-proc DAP implementation that uses sockets internally to communicate between its own components.

@alexr00
Copy link
Member Author

alexr00 commented Aug 17, 2023

@int19h thank you for looking at the API and providing feedback! You are correct that there's no guarantee that providePortAttributes will happen after your custom message. However, this will likely not be an issue because VS Code will not be very quick to call providePortAttributes:

  • First, we have to actually discover the port. For the purposes of this issue, debug ports will only ever be discovered through polling, which means we likely already have a delay. The minimum polling interval is 2 seconds, but we use a moving average to calculate it based on how fast the machine is, so it can be greater.
  • Next, VS Code has to communicate the discovered port from the remote machine to the local machine. This will add whatever the network latency is to the delay.
  • Finally, VS Code's call to providePortAttributes will always be a round trip cross-process, and depending on where the extension is running it could add round trip network latency again, so that will also add delay. This last one is why we have the portSelector, so that we can prevent this round trip for every port attribute providing extension.

Given these inherent delays, which can easily be > 1 second, and will almost always be hundreds of milliseconds, I don't think we should let perfect be the enemy of better here.

@joaomoreno joaomoreno removed this from the December 2023 milestone Dec 11, 2023
@paulacamargo25 paulacamargo25 added the author-verification-requested Issues potentially verifiable by issue author label Feb 20, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
author-verification-requested Issues potentially verifiable by issue author feature-request Request for new features or functionality needs PR Ready to be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants