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.

Sign up for GitHub

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

Jump to bottom

debug: enable remote attach with dlv-dap in the extension #1861

Closed
polinasok opened this issue Oct 28, 2021 · 6 comments
Closed

debug: enable remote attach with dlv-dap in the extension #1861

polinasok opened this issue Oct 28, 2021 · 6 comments
Assignees
@polinasok
Labels
Debug Issues related to the debugging functionality of the extension. FrozenDueToAge
Milestone
v0.30.0

Comments

@polinasok
Copy link
Contributor

polinasok commented Oct 28, 2021 •
edited
Loading

Dlv has been upgraded to support remote attach via DAP (available with go install github.com/go-delve/delve/cmd/dlv@HEAD).

In vscode-go currently by default if remote mode is used with a port, the extension just uses the legacy adapter. If dlv-dap is explicitly specified in launch.json or settings.json with remote mode, the following error is displayed

The only way to by-pass these is to not specify the adapter (dlv-dap by default) and use debugServer instead of port with remote attach mode.

We need to roll out the new dap remote attach, but do so gradually as users will be updating their tools at different rates. Ideally we should check the version of dlv used. If the version supports remote-attach, we can use dlv-dap configuration unless the user explicitly specified legacy. If the version is too old, we can either fail or fallback tolegacy while printing a warning that the user should update their tools because remote attach is now available with dlv-dap as well.

@hyangah @suzmue
@gopherbot gopherbot added this to the Untriaged milestone Oct 28, 2021
@polinasok polinasok added the Debug Issues related to the debugging functionality of the extension. label Oct 28, 2021
@hyangah hyangah modified the milestones: Untriaged, On Deck Oct 28, 2021
@polinasok polinasok self-assigned this Nov 1, 2021
@polinasok
Copy link
Contributor Author

I can't think of a good way to check the binary version of a running server. We can't use the same logic we use to check the version of the installed tools because we don't know which binary was used or if it was started on a local machine. There is no rpc method to ask for the binary version and, even if there was, using a special client connection just to check the version seems overly complicated.

The simplest thing would be to continue using legacy default for the foreseeable future if debugAdapter is not specified and only use dlv-dap if it is explicitly requested by the user. Then after a while, when most users have likely updated their tools, we can switch to dlv-dap by default, but continue allowing legacy option on demand.

If the user makes a mistake and requests dlv-dap adapter with a remote server that doesn't support remote attach via DAP, they will get the following error:

2021-11-02T21:43:44Z error layer=rpc rpc:invalid character 'C' looking for beginning of value

Ideally we should translate that into something informing the user that they need a newer version of dlv or at the very least, we should include this detail in our documentation.

@polinasok polinasok changed the title debug: enable remote attach in the extension debug: enable remote attach with dlv-dap in the extension Nov 3, 2021
@polinasok polinasok mentioned this issue Nov 3, 2021
Closed
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/360974 mentions this issue: debug: allow debugAdapter=dlv-dap with remote attach mode

gopherbot pushed a commit that referenced this issue Nov 4, 2021
@polinasok
debug: allow debugAdapter=dlv-dap with remote attach mode
b2c00ac 
If debug adapter is not specified, still default to `legacy` for remote attach in case users haven't updated their tools to newer version of dlv that supports this via DAP. We can flip the default in a couple of months. In the meantime, users will need to specify directly if they want to use `dlv-dap` adapter. There is no good way to detect the version of a running server, so we will always allow them to proceed, while providing a warning about the right version at session start-up and at shutdown via Go Debug output channel if we detect that the connection failed with no responses.

After this change, we will have the following behavior:
attach + remote + debugAdapter=   => legacy
attach + remote + debugAdapter=dlv-dap => dlv-dap + version warnings
attach + remote + debugAdapter=legacy => legacy

attach + !remote + debugAdapter=dlv-dap + port => warning to drop port if not using external server
launch + !remote + debugAdapter=dlv-dap + port => warning to drop port if not using external server
launch + remote  + debugAdapter=dlv-dap + port => error from dlv server

Updates #1861

Change-Id: Ia2e6b35f9d401ea2e719a65c78d9cf8e5ef90c24
GitHub-Last-Rev: 3a0ce53
GitHub-Pull-Request: #1873
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/360974
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Polina Sokolova <polina@google.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
@polinasok polinasok mentioned this issue Nov 30, 2021
Closed
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/367854 mentions this issue: docs/debugging.md: update on newly available traditional remote debugging via DAP

gopherbot pushed a commit that referenced this issue Dec 3, 2021
@polinasok
docs/debugging.md: update on newly available traditional remote debug…
6320e8b 
…ging via DAP

Updates #1861

Change-Id: Icc133623362b98d16fc0cc9fe0f235e4241a928b
GitHub-Last-Rev: 4a15622
GitHub-Pull-Request: #1920
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/367854
Reviewed-by: Polina Sokolova <polina@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
@polinasok polinasok mentioned this issue Dec 3, 2021
Closed
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/368936 mentions this issue: package.json: update remote attach references to reflect dlv-dap support

gopherbot pushed a commit that referenced this issue Dec 6, 2021
@polinasok
package.json: update remote attach references to reflect dlv-dap support
7d023af 
Updates #1861

Change-Id: Ic429043aee93bcb0be34de2be861124bf548cde5
GitHub-Last-Rev: 9195536
GitHub-Pull-Request: #1925
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/368936
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Polina Sokolova <polina@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
@hyangah hyangah modified the milestones: On Deck, v0.30.0 Dec 9, 2021
@hyangah hyangah closed this as completed Dec 9, 2021
@polinasok polinasok mentioned this issue Dec 14, 2021
Closed
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/371494 mentions this issue: readme+welcome: announce remote attach debugging with dlv-dap

gopherbot pushed a commit that referenced this issue Dec 14, 2021
@polinasok @hyangah
readme+welcome: announce remote attach debugging with dlv-dap
15a266b 
Updates #1930
Updates #1861

Will add gophercon details separately.

Change-Id: I87fbe7dc3a41d0c9d3b27e40bd81b84a269a2ef9
GitHub-Last-Rev: 8a1b13f
GitHub-Pull-Request: #1943
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/371494
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Peter Weinberger <pjw@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
@polinasok polinasok mentioned this issue Dec 14, 2021
Closed
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/371974 mentions this issue: src/goDebugFactory: improve start-up and session fail messages for remote dlv-dap

gopherbot pushed a commit that referenced this issue Dec 24, 2021
@polinasok
src/goDebugFactory: improve start-up and session fail messages for re…
691e15f 
…mote dlv-dap

Updates #1861
The informational message alerting the user that they are using new remote dlv-dap mode will always pop-up when remote attach session is started. The user has the option to disable it with "Do not show again". If connection/attach fail, an additional pop-up will occur to high-light that this is dlv-dap mode that requires newer versions of dlv.

Change-Id: If56a9481323c57c11a740d1b61df20631c796bd0
GitHub-Last-Rev: a731107
GitHub-Pull-Request: #1945
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/371974
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Polina Sokolova <polina@google.com>
@golang golang locked and limited conversation to collaborators Dec 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@polinasok polinasok

Labels
Debug Issues related to the debugging functionality of the extension. FrozenDueToAge
Projects
None yet
Milestone
v0.30.0
Development

No branches or pull requests
3 participants
@hyangah @gopherbot @polinasok