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

Request for Comments: Debugging dotnetcore Lambda Functions #759

Merged
merged 13 commits into from
Jan 9, 2019

Conversation

ndobryanskyy
Copy link
Contributor

@ndobryanskyy ndobryanskyy commented Nov 12, 2018

Original Issue

#568 - Feature request

Design document

⚡ Rendered document is available here

Doc is updated and utilizes new docker ps filter approach. @jfuss, @thesriram, please take a look on it and my updated POC -as always comments and questions are welcome!

If you are okay with the new approach, the only thing left is merging PR to lamci/lambda.

As requested I provide design document and example which uses modified runner from PR which I would also appreciate reviewing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sanathkr sanathkr changed the title WIP: #568 Design doc and example WIP: Debugging dotnetcore lambda functions Nov 12, 2018
_From SAM code - container.py:_

```python
for frame_type, data in output_itr:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I believe this : #729

We do have unbuffered streams, whenever a debug port is set explicitly. but in this case there is no debugger port. because of the .net debugging works here, so we may need to slightly rethink that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the port is set, and environment variable is set as expected, but it just won't work on python 3.7

As you see above with flush, everything works. Without it all is buffered, help needed, maybe this [link](https://docs.python.org/3/using/cmdline.html#cmdoption-u) will give something (see note for python 3.7),

3. Help needed in investigation of how to adopt that approach for VS 2017. See this [link](https://github.com/Microsoft/MIEngine/wiki/Offroad-Debugging-of-.NET-Core-on-Linux---OSX-from-Visual-Studio) for some information. Maybe we can try to use Docker as a pipe there too.
4. Docker in VS Code reports _"The pipe program 'docker' exited unexpectedly with code 137."_ after stopping or hitting continue which leads to quitting the program. It seems, that 137 is **killed** exit code, which seems a bit strange, maybe you guys, have some ideas on that? Haven't investigated yet.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is strange and definitely needs to be investigated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I've described a bit incorrectly (so a bit of clarification), the debugging process works fine and smooth, but as soon as it ends, VS Code just shows the notification, it is just annoying, but the functionality works just fine.

@ndobryanskyy ndobryanskyy changed the title WIP: Debugging dotnetcore lambda functions Request for Comments: Debugging dotnetcore Lambda Functions Dec 3, 2018

### What is the problem

Currently SAM CLI does not provide debugging support for .NET Core 2.0 and 2.1 because `dotnet` command does **not** support _start and wait for the debugger_ configuration out of the box and neither does current AWS Lambda runner ([2.0](https://github.com/lambci/docker-lambda/tree/master/dotnetcore2.0) / [2.1](https://github.com/lambci/docker-lambda/tree/master/dotnetcore2.1)) has debugging support flag or anything.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some talk here that it might make it to 3.0

https://github.com/dotnet/core-setup/issues/198

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've mentioned this issue in the document too. But I don't think it has some priority there now


Visual Studio 2017 (can be a target, but it has some other very different Docker container debugging support - needs additional investigation). But some trick with `launch.json` [might be possible](https://github.com/Microsoft/MIEngine/wiki/Offroad-Debugging-of-.NET-Core-on-Linux---OSX-from-Visual-Studio). Help on this area is highly appreciated.

Rider support also needs separate investigation, as it does not support `vsdbg` by any means (licensing [issue](https://github.com/dotnet/core/issues/505)) and therefore they have [their own](https://blog.jetbrains.com/dotnet/2017/02/23/rider-eap-18-coreclr-debugging-back-windows/) debugger implementation and possibly UX around .NET Core remote debugging. If support is required - I think we should open another issue.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rider may be using a ssh based transport, so we need to be extensible of adding that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The worst thing, is that they are not using vsdbg 😞 because of the licensing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aaaah. :|


### CLI Changes

None required. We would happily use `debugger_port` and `debugger_path` like other runtimes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is brilliant. 💯

Copy link
Contributor

@jfuss jfuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of this has been merged for some time now. Approving and will wait for the build to finish before merging in.

@jfuss jfuss added the stage/accepted Accepted and will be fixed label Jan 8, 2019
@jfuss jfuss merged commit ef13b1f into aws:develop Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/accepted Accepted and will be fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants