Migrate delve calls to v2 API

[lggomez]

Some debugging limitations/issues such as #868 can be solved by using the new delve API:
https://godoc.org/github.com/derekparker/delve/service/rpc2

As a reference, this is the version we're using right now:
https://godoc.org/github.com/derekparker/delve/service/rpc1

The following calls would have to be migrated:

• ClearBreakpoint
• CreateBreakpoint
• ListGoroutines
• StacktraceGoroutine
• ListLocalVars
• ListFunctionArgs
• ListGoroutines
• Command
• EvalSymbol

The main challenge here is that the migration cannot be done partially (on a per call basis) since the delve server starts either in v1 or v2 mode, as stated here, and the signature changes will involve a partial rewrite of goDebug

[ramya-rao-a]

I agree that its about time we did this.

[lggomez]

FWIW, I'm giving it a try here: master...lggomez:delve_v2, it's still a work in progress

The diff between v1 and v2 seems to be mostly on the side of wrapper types (for both in/out params) and the inclusion of the LoadConfig type which gives us the customization options for #868, among others). These are obviously breaking changes but it looks a bit less painful than I thought originally

With these changes, the debug mode blows up upon launch with the following errors:

Do you have any tips on how to debug this? breakpoints on goDebug don't get bound and I find the verbose mode being not really helpful

[lggomez]

Some more info: This is the diff I get by logging the delve API calls from the extension

Example code is the same as #1385 (comment) for the package github.com/go-training/helloworld which I modified

v1:

2018/03/05 21:54:15 server.go:73: Using API v1
2018/03/05 21:54:15 debugger.go:98: launching process with args: [/home/razael/go/src/github.com/go-training/helloworld/debug]
API server listening at: 127.0.0.1:2345
CALL - RPCServer.CreateBreakpoint [{"file":"/home/razael/go/src/github.com/go-training/helloworld/main.go","line":14}]
2018/03/05 21:54:15 debugger.go:340: created breakpoint: &api.Breakpoint{ID:1, Name:"", Addr:0x49c34c, File:"/home/razael/go/src/github.com/go-training/helloworld/main.go", Line:14, FunctionName:"main.helloworld", Cond:"", Tracepoint:false, Goroutine:false, Stacktrace:0, Variables:[]string(nil), LoadArgs:(*api.LoadConfig)(nil), LoadLocals:(*api.LoadConfig)(nil), HitCount:map[string]uint64{}, TotalHitCount:0x0}
CALL - RPCServer.Command [{"name":"continue"}]
CALL - RPCServer.ListGoroutines []
2018/03/05 21:54:15 debugger.go:497: continuing
CALL - RPCServer.ListGoroutines []
CALL - RPCServer.ListGoroutines []
CALL - RPCServer.ListGoroutines []
CALL - RPCServer.ListGoroutines []
CALL - RPCServer.ListGoroutines []
CALL - RPCServer.ListGoroutines []
CALL - RPCServer.StacktraceGoroutine [{"id":1,"depth":20}]
CALL - RPCServer.ListLocalVars [{"goroutineID":1,"frame":0}]

v2:

2018/03/05 21:55:18 debugger.go:98: launching process with args: [/home/razael/go/src/github.com/go-training/helloworld/debug]
API server listening at: 127.0.0.1:2345
CALL - RPCServer.CreateBreakpoint [{"file":"/home/razael/go/src/github.com/go-training/helloworld/main.go","line":14}]
CALL - RPCServer.Command [{"name":"continue"}]
CALL - RPCServer.ListGoroutines []
2018/03/05 21:55:19 debugger.go:497: continuing
Hello World!!
CALL - RPCServer.ListGoroutines []
Failed to get threads.
Failed to get threads.

The ListGoroutines call at handleReenterDebug is what fails in last place, but the error received in the callback is undefined

[JCzz]

Sounds great.

How will I know when it will support api v2?

Thanks

[lggomez]

I've been swamped with work these last weeks so unfortunately I couldn't put any further effort into this. The WIP branch I have it this one: master...lggomez:delve_v2, but since the new apicalls aren't working I can't consider it PR ready yet. There's also the difficulty of debugging the debug adapter in a fast and reliable way

A tentative roadmap would be the following:

• Make it work
• Test against regressions
• Refactor the v2 and v1 apicalls so the user can switch between them via config or command (for those are still stuck with v1)

If someone can give me a hand with the debugging part I can continue with this migration

[aarzilli]

@lggomez I think your problem is that the argument to CreateBreakpoint isn't wrapped, which means no breakpoint gets created and when you call the continue Command the program executes completely.
I think that when we can't execute a request we return some kind of error response, you should probably print that.

[lggomez]

@aarzilli indeed, that was the problem. I just verified it so I will continue with the rest of a basic debug case during the following days. Thanks for the tip

[lggomez]

@ramya-rao-a I made progress to the point of having a working adapter. I conducted very basic tests but so far all of the basic commands seem to work

I've some questions regarding the next steps to take

• I recall you saying that the extension config is cluttered enough so adding any new settings is heavily discouraged. We have to support the configuration of this parameter in order to allow users to configure the debugger (should fix Long strings in variable inspection get truncated #868, among others). What do you suggest?
• Version 2 of the api seems to be oficially available since v0.12.0 (Jan 11, 2017), although the v2 services were already present since april 2016 (correct me if I'm wrong @aarzilli). Should we preserve support for the v1 api? this would warrant compatibility for users with old versions who cannot update delve, but will require a somewhat large refactor of the debug adapter. Also, it should be configurable, bringing us back to the previous question
• Do you have any test program that you use to test the debugger and may be willing to share? I just want to test this thoroughly and make sure that I'm not introducing regressions. Having some unit tests would be a plus but the effort required (if plausible) seems huge

[aarzilli]

Do you have any test program that you use to test the debugger and may be willing to share?

If you want to manually run the tests we do automatically for delve:

https://github.com/derekparker/delve/blob/master/service/test/integration2_test.go
https://github.com/derekparker/delve/blob/master/service/test/variables_test.go

look for things that call withTestClient2 and withTestProcess in this two files. Also https://github.com/derekparker/delve/blob/master/_fixtures/testvariables2.go.

[ramya-rao-a]

@lggomez Regarding your questions

• LoadConfig: Instead of having the 5 properties in the LoadConfig as 5 different settings, let's have it as a single complex setting
• v1 Backward compatibility: I think we can safely assume that most users have the newer version of delve that supports v2 of the apis. My one concern is for users doing remote debugging. When they start a debug session, we should have a way to identify that the headless dlv being connected to is using v1 and then notify the user to run v2 instead

[ramya-rao-a]

@lggomez I've pushed a commit to make this backward compatible. Here is my intended plan

• For a month, we will run with v1 as default, and get a few folks to start using v2 and provide feedback
• For another month, we will run with v2 as default, with the provision to fall back to v1 for anybody who needs time to move on to v2
• Sometime in July/August, we will stop supporting v1

As I mentioned before, we would still need to take care of the scenario of remote debugging. Are you aware of any way the client can figure out what version of api is the headless server running on?

[lggomez]

@ramya-rao-a thanks for the help, this work is finally taking shape and the plan sounds good to me

I'll start debugging tests tomorrow, on my work projects. Regarding remote debugging, I just found the api calls needed to achieve it: https://godoc.org/github.com/derekparker/delve/service/rpccommon. These should be common across v1 and v2 so the compat is already granted

I added the check during launchRequest, upon successful connection to delve: bcb89e4

The api also supports changing the version served by the remote server, but I prefer failing fast instead of doing that as I deem it too aggressive to be a default behavior

[ramya-rao-a]

To use v2 apis add the below in your debug configuration:

             "useApiV1": false,
            "dlvLoadConfig": {
                "followPointers": true,
                "maxVariableRecurse": 1,
                "maxStringLen": 64,
                "maxArrayValues": 64,
                "maxStructFields": -1
            }

Change the 64 to the say 300 to see longer strings and arrays when debugging

[ramya-rao-a]

This feature is now out in the latest update to the Go extension (0.6.81)

[giulianob]

Is there anyway to use v2 when debugging tests by clicking the "Debug test" in the IDE? It looks like there is a hard coded debug config in vscode-go.

[lggomez]

I believe that the debug tests command uses the currently selected debug configuration, so as long as you have v2 activated on that config with these settings you should be able to use it

[ramya-rao-a]

@lggomez As I mentioned in #1647 (comment), the codelens for "debug tests" uses hard coded configuration from https://github.com/Microsoft/vscode-go/blob/0.6.82/src/goRunTestCodelens.ts and doesn't read from the launch.json file.

@giulianob What is your specific use case for using the v2 apis? Do you intend to configure any of the settings from #1555 (comment)? If yes, then feel free to log a feature request for the same.

[lggomez]

@ramya-rao-a my bad. We can add an option that, if set to true, will bypass that default launch cfg and use the currently selected one forcing (or validating) that it has mode === 'test'

Can we use the configurationmanager from the native debugService to retrieve the launch cfg from the codelens provier?

[ramya-rao-a]

We can add an option that, if set to true, will bypass that default launch cfg and use the currently selected one forcing

VS Code also lets you debug files with default config even when there is no launch.json file. To support that as well as the debug codelens, we will have to have a separate setting which mimics the dlvLoadConfig

Can we use the configurationmanager from the native debugService to retrieve the launch cfg from the codelens provider?

I don't see how that is possible though. The debug adapters from extensions are called by VS Code in a separate process that has no access to settings as far as I know. If you find anything different, do share.

I was thinking of reading the settings in https://github.com/Microsoft/vscode-go/blob/master/src/goDebugConfiguration.ts. provideDebugConfigurations here is used when there are no debug configurations. resolveDebugConfiguration gets called for every debug session

[ramya-rao-a]

I've logged #1735 to track the pending item from the above discussion

[kzhui125]

@ramya-rao-a

            "dlvLoadConfig": {
                "followPointers": true,
                "maxVariableRecurse": 1,
                "maxStringLen": 64000,
                "maxArrayValues": 64000,
                "maxStructFields": -1
            }

Debugging is very very slow now. It's unusable.

[lggomez]

@kzhui125 never tried such large limits but I can't say it's unexpected though, you'll have to settle down with a lower value

We could measure which part has the major slowdown: delve, the rpc apicall itself or vs code