Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Add support for LogPoints in Node #163

Closed
auchenberg opened this issue Jan 28, 2018 · 25 comments
Closed

Add support for LogPoints in Node #163

auchenberg opened this issue Jan 28, 2018 · 25 comments

Comments

@auchenberg
Copy link

auchenberg commented Jan 28, 2018

This is a part of a series of requests to enable "custom breakpoints" support in VS Code. See microsoft/vscode#38568

We want to light up LogPoints for the local Node debugging scenario in VS Code, so developers can get comfortable with LogPoints while debugging locally, and because we believe LogPoints will enable more people to get comfortable by using a debugger, as it's an experience that's closer to using console.log's.

Functionality

  • I imagine LogPoints can be enabled as a conditional breakpoint over CDP, where the passed expression simply is console.log(<x>) which will get evaluated in the scope of where the breakpoint hauls execution. The log message should then be surfaced in the Debug Console, as this is already working.

  • LogPoints should be set using a UX that is similar to the conditional breakpoint logic, but use a different gutter icon.

  • The expression of a LogPoint should be validated to make sure it can be executed

Asks outside of this DA:

VS code:

  1. Support an extension point for customizing gutter interaction.. Support an extension point for customizing gutter interaction. vscode#38568
  2. Visualize breakpoint types in breakpoints view to enable LogPoints to show up as LogPoints instead of a conditional breakpoint, Visualize breakpoint types in breakpoints viewlet vscode#42266

VS Code Debug Protocol

  1. Enable debug adaptors to enable/disable certain breakpoint types. Enable debug adaptors to enable/disable certain breakpoint types vscode-debugadapter-node#155

Considerations:

  1. Should we treat LogPoints as a special kind of breakpoint or something new? Do we want do introduce a Debugger.addLogPoint in CDP and move the responsibility on level deeper, or have the implementation in inside this DA? Does the Debug Protocol (DAP) need to be extended?

(@dhanvikapila, @digeff, @changsi-an, @roblourens, @weinand )

@weinand
Copy link
Contributor

weinand commented Jan 28, 2018

@auchenberg I'm confused by your usage of "DAP" (e.g in "Asks outside of this DAP").
For us DAP means Debug Adapter Protocol and here you probably mean "DA" for Debug Adapter, correct?

@auchenberg
Copy link
Author

@weinand I meant DA. Updated.

@weinand
Copy link
Contributor

weinand commented Feb 14, 2018

@auchenberg @changsi-an A fundamental question:

Is the LogPoints concept really something that we want to add to VS Code as an extension or is it a built-in feature of VS Code that is available for every debugger?

E.g. VS for Mac supports LogPoints as an orthogonal feature for all breakpoint types:

2018-02-14_12-01-48

or VS's peek UI:

2018-02-14_15-49-50

I'm asking because adding LogPoints as a builtin feature to VS Code is really simple, whereas opening up VS Code for adding LogPoint functionality as an extension will increase our API surface area substantially.

@changsi-an If LogPoints would be a built-in feature of VS Code, what would this mean for your request microsoft/vscode#38568 and your planned work?

@changsi-an
Copy link

So far we have designed LogPoints to be enabled in an extension. When you say it's easier to make this feature built-in, is it just UI interface or your "built-in" suggest that work will be done on your side to implement LogPoints for all langauges, all frameworks? If it is just interface, who will be responsible for implementing LogPoints, for example, C#?

Our LogPoints implementation is not trivial, it involves remote procedures to set up one log point. And it only works for a scoped scenario for Azure. I don't think it is a good idea to have this implementation piece built-in to VS Code.

Please provide more details on the API in your mind when we say "adding LogPoints as a builtin feature to VS Code"

@weinand
Copy link
Contributor

weinand commented Feb 14, 2018

If VS and VS for Mac support already LogPoint generically, I do not see why VS Code should not be allowed to do the same.

UI-wise the LogPoint feature could be added on top of a VS Code SourceBreakpoint (like VS's UI suggests). On VS for Mac a LogPoint is an exclusive option: either the breakpoint breaks or it logs and continues. On VS breaking and logging are independent (non-exclusive).

Datastructure-wise a LogPoint is just another string based attribute of the SourceBreakpoint that contains the message to be logged (basically the expression passed to a console.log(...)).
VS Code does not interpret it in any way and passes it unmodified to the DA (and it is available in every extension via the vscode.breakpoints API).

Node-debug and other "real" debug extensions could just wrap the expression in a "console.log" and combine it with the "condition" and "hitcount" attribute already supported. With this approach we would get local LogPoints basically for free.

For your non-trivial LogPoint implementation involving remote procedures you could create your own Debug Extension and DA that gets the same LogPoints passed and you could do your remote target instrumentation based on the information available in every LogPoint.

With this approach you could first develop and run a program locally and create and try LogPoints with the regular debug extensions. If everything works locally you would then deploy the program, and via your extension apply the LogPoints to the remote program.

@dhanvikapila
Copy link
Member

@changsi-an We are looking to add Logpoints as a local experience and not just as a remote experience so with respect to that we should have something that is native within VSCode.

@weinand We do have our own DA that is does everything you mention above. At the very least, we should have something that allows us to setup a logpoint (with an expression from customer), show it in the gutter with a different icon to differentiate it from a regular breakpoint and allow the DA (launched by the extension) to interact with the logpoint to enable, disable, update the expression.

@weinand
Copy link
Contributor

weinand commented Feb 14, 2018

If we would add builtin LogPoints, the following functionality would become available almost immediately:

  • Peek UI to turn breakpoints into LogPoints similar to VS's peek UI (see screenshot above).
  • Extension API to create, delete, and retrieve LogPoints.
  • LogPoints will show up with their own icon in the gutter and they have edit actions in their context menu and show additional information on hover.
  • LogPoints will show up in the Breakpoints view and they have edit actions in their context menu.
  • When editing LogPoint's their location is updated (so they "stick" to their line).
  • VS Code saves and restores LogPoints per workspace; deletes when resource is deleted, etc.
  • VS Code sends LogPoints to active DA whenever necessary, e.g. when attributes or location are changed.

Please note that this functionality is always available, even if no debug extension and no debug session (DA) is active.

@isidorn did I miss anything?

@dhanvikapila
Copy link
Member

@weinand can you clarify what you mean by Peek UI to turn breakpoints into LogPoints ? Does that mean customers set a breakpoint first and then turn it into a logpoint? If yes, can we have the option to set a logpoint separate from a breakpoint? Otherwise, setting a logpoint would not be easily discover-able I fear.

@weinand
Copy link
Contributor

weinand commented Feb 14, 2018

Sure, for instance in the (already exisiting) gutter menu there would be an additional action "Add LogPoint" between the first and second action:

2018-02-14_23-09-57

And this action opens the Peek UI in the editor with the focus on the text box where the "LogPoint" expression is entered:

2018-02-14_23-16-05

Imagine that "Hit Count" would read "Log Point Expression".

@dhanvikapila
Copy link
Member

Thanks @weinand for the details.

@auchenberg If we think of logpoints as a springboard for customers to discover debug adapters and debugging of Node apps/programs, I think we should have a way to turn logpoints into breakpoints. I imagine that we can auto launch a DA with logpoints experience by default which can be thought of as a lightweight debugging experience where you dont break your app execution but are also able to log state easily. When you set a breakpoint (or convert a log point to a breakpoint), we automatically pause the program and give the full blown debugging experience.

What happens when a logpoint <-> breakpoint conversion happens should be something that the DA should be able to determine I think as some breakpoint scenarios might not make sense in remote contexts.

@auchenberg
Copy link
Author

At this point in time we don't have enough validation to make the decision to ship LogPoints built-into our Node.js debugger and provide this as a default local experience.

I imagine LogPoints could be functionality that many debug adaptors would like to implement if proven useful, but first I would like to us to find a way for us to explore the LogPoints concept without taking a big investment.

This is the reason for idea of having extension contributing new breakpoint types instead of building in into the core, as we can ship and iterate independently.

So it's interesting that it's cheaper to skip the extension model altogether. We would want all the behavior described in #163 (comment)

Questions

  1. If we were to make LogPoints a new standard breakpoint type, can we add APIs to disable LogPoints for certain debug adaptors? If not supported, don't enable/show.

  2. Is it possible to introduce APIs to disable specific built-in breakpoint types? An example would be the Node.js Cloud scenario where we don't want to allow regular breakpoints as they halt execution. If a debug adaptor/extension added the breakpoint type dynamically this functionality would already be provided.

  3. Would we get a command for "Add LogPoint" like we have "Debug: add conditional breakpoint"? This can be mapped to a keyboard shortcut right?

  4. Is it possible to provide an API or mechanism where we could provide suggestions for the expression used in the Add LogPoint UI? Imagine you click "Add LogPoint" for a given line and VS Code would be able to provide an intelligent suggestion for the expression. This logic could live in an extension or the used DA

  5. To @dhanvikapila's point: We should explore ways to transition between breakpoint types. I could imagine a new action in the context menu for LogPoints that would say "Convert to breakpoint"

Ultimately it all comes down to: What is the easiest way we can ship a prototype of LogPoints with a better UI in VS Code? By being built-in we would get the default UI of VS Code which also would address the UX feedback we already have received in the LogPoints MVP.

To me it sounds like the cheapest way would be:

  1. Add LogPoints as a core breakpoint type to get the UI.
  2. Add LogPoints to the default Node DA or introduce a new one that can be shipped independently
  3. Adjust the existing Node Cloud DA to receive the new VS Code DA events

@weinand
Copy link
Contributor

weinand commented Feb 16, 2018

@auchenberg What is the difference between your notion of (local) LogPoints and the concept that already exists in VS and VS for Mac?
I was under the impression that being available in VS and VS for Mac is enough evidence that log point functionality is indeed a generally useful concept across debuggers.

Yes, exploring new concepts (or introducing debugger specific functionality) as an VS Code extension is the preferred approach and I did this for the "loaded scripts" functionality (which only waits for the last step of being pushed into core).

But this approach only works if either the API already exists or is at least "in reach." If most of the needed API doesn't exist (e.g. most of the functionality listed in
#163 (comment)) then the effort for creating the API is a bigger investment than actually using the API in an exploration.

APIs are not just a few interfaces and structure definitions but they involve basically an RCP implementation that works between different processes (i.e. address spaces) and does not break down in a remote EH setup. In addition, if adding new APIs is already hard, removing APIs is almost impossible. So we do not want to introduce new APIs only for the purpose of a one-shot exploration.

Answers to your questions:

  1. Breakpoints are "owned" by VS Code and not by individual extensions. Breakpoints are not associated in any way with a debug type. They live independent from extensions and can be created and deleted without a debugger extension or DA being active. Since we've introduced additional breakpoint features (e.g. conditions, hit count) and types ("FunctionBreakpoint") later, we had to introduce the corresponding capabilities "supportsConditionalBreakpoints", "supportsHitConditionalBreakpoints", "supportsFunctionBreakpoint" capability in the DAP. VS Code will only call API related to these features if the DA has opted-into it. For LogPoints we would add another capability "supportsLogPoints". But please note that this capability does not control whether VS Code shows the UI for LogPoints or not (if you have two debug extensions installed and only one of them supports LogPoints, the LogPoint UI must still be enabled all the time).
  2. In general VS Code will never provide APIs that allow to disable VS Code's built-in functionality. So there will be no API to disable the built-in source- or function breakpoints. However, the interpretation (and implementation) of any type of breakpoint is the sole responsibility of the DAs. So the DA has the ultimate control whether a breakpoint "breaks" the target or just adds a log entry. If the DA detects that it is connect to a cloud service in production, it should better ignore regular source breakpoints and only install non-breaking log points.
  3. Yes. All builtin features get full support including keyboard shortcuts.
  4. Yes, Intellisense will be supported, but not initially. Currently we do not even have Intellisense for breakpoint conditions. But we are interested in adding this soon. And the same implementation can be used for the LogPoint text box. Please note that most likely this implementation will live in the VS Code core and not come from an extension or DA because LogPoints can be added at any time even if no debug extension or DA is active. Since intellisense support for the underlying language is not available in the extension or DA anyway, this simplifies the extension/DA a lot. For LogPoints we would support the curly-bracket message syntax used by MS and Google, e.g. New session created on for user {customer.name}..
  5. Since the only difference between a LogPoint and a Breakpoint is the value (or existence) of a new attribute, our planned UI could support in-place conversion easily. Setting or unsetting a checkbox in the already existing "Edit Breakpoint" Peek UI would do the trick.

Our proposal:

  • Add LogPoints as a built-in breakpoint type with the functionality listed in Add support for LogPoints in Node #163 (comment).
    • this includes the addition of one or two attributes to the DAP type SourceBreakpoint (e.g. "logMessage" and "action") and a new capability "supportsLogPoints"
    • surface the attributes in the extension API
    • add language Intellisense support for conditions and logMessage
  • make node-debug (both protocols) opt into LogPoints and implement "local" LogPoint functionality based on node's existing breakpoint condition: console.log(logMessage); return false
  • Adjust the existing Node Cloud DA to opt into LogPoints and interpret the "logMessage" attribute accordingly.

@weinand
Copy link
Contributor

weinand commented Feb 22, 2018

I've created microsoft/vscode-debugadapter-node#162 for the corresponding DAP changes.

@auchenberg
Copy link
Author

auchenberg commented Feb 23, 2018

@weinand I would agree that we could look to VS and VS for Mac as validation of LogPoints as a debugging concept, but in relation to LogPoints for Node.js we are yet to verify that as as valid concept.

Today we are shipping "tracepoint" in Edge DevTools that essentially are LogPoints, and we don't see much usage. Whether this is cased by little awareness, Edge dynamics or the concept simply doesn't resonate is yet to be identified.

I want us to explore LogPoints for Node.js specifically, and if adding LogPoints as a general concept to VS Code is the cheapest way to explore this, I'm all in. This would also as you mentioned bring parity for VS Code with VS/VS Mac.

Comments

  1. We should explore ways to disable built-in functionality in VS Code. Example: It will be highly confusing for our users that they in the UI can set breakpoints, but they aren't working when connecting to a cloud/production context. I understand the technical architecture and constraints, but we should explore ways to make this possible. Maybe via extension APIs?

  2. I bet Having intellisense for expressions in LogPoints is going to be a key request, as developers today have intellisense when using console.log within the editor. If we can't provide intelligent suggestions for the expressions, developers will continue to use console.log. We need to be better with LogPoints, and having away to intelligently propose the most likely expression could be a differentiator.

@weinand
Copy link
Contributor

weinand commented Feb 23, 2018

  1. Instead of introducing API for disabling existing features, I suggest that we find a way that "breakpoints actually do work when connecting to a cloud/production context". But of course not as breakpoints but as log points. What debug extension is actually used for the cloud/production context? This could turn all breakpoints into log points automatically so that they would work.
  2. As I said: we are interested to add intellisense for breakpoint conditions and logMessage expressions. But I do not understand why we only make proposals intelligent in the context of logPoints, but we keep them dumb everywhere else. Wouldn't it be better to have the same intelligent proposals everywhere? What kind of intelligent logMessage expression proposals are you envisioning?

@auchenberg
Copy link
Author

  1. In a Cloud Context we would probably expose a CDP-like endpoint in a gateway component that would receive the Breakpoint/LogPoints commands and orchestrate them across instances, etc. So we could indeed automatically turn BreakPoints into LogPoints, but would this be a weird UX as a user? You set a breakpoint, connect to the cloud, and suddenly they change?

  2. We should take a general approach to provide completions for expressions, but I think we would need to enable a DAs or Extension to provide language specific completions for the language specific expressions. Example: LogPoints in Node would be a "better console.log", so developers would most likely expect to be able to write "onFocus", e.target, new Date().now as the expression, ( console.log("onFocus", e.target, new Date().now)), where a C# developer might have a different expectation to the syntax.

@weinand
Copy link
Contributor

weinand commented Mar 6, 2018

I've created:

@weinand
Copy link
Contributor

weinand commented Mar 16, 2018

@roblourens we got the first glimpse of Log Point UI.

I've already implemented minimal Log Point support in "legacy" node with these two commits: microsoft/vscode-node-debug@f6f2317 and microsoft/vscode-node-debug@ddd4fbe.

It would be great if you could do something similar for "inspector".

What is still missing on our side is microsoft/vscode#45127 and the code that extracts and evaluates embedded {expression} in the logMessage . I'll let you know if I've finished this for node-debug.

roblourens added a commit to microsoft/vscode-chrome-debug-core that referenced this issue Mar 16, 2018
roblourens added a commit that referenced this issue Mar 16, 2018
@roblourens
Copy link
Member

Added those changes

@weinand
Copy link
Contributor

weinand commented Mar 19, 2018

@roblourens I've closed this issue because there is nothing left to do for node-debug2.

@weinand weinand closed this as completed Mar 19, 2018
@weinand
Copy link
Contributor

weinand commented Mar 19, 2018

@auchenberg let's continue further LogPoint discussions in microsoft/vscode#45128

@weinand
Copy link
Contributor

weinand commented Mar 22, 2018

@roblourens I've added support for interpolating expressions in logMessages. Please see microsoft/vscode-node-debug@1233378

@roblourens
Copy link
Member

I'm lost, what does that change do vs @auchenberg's PR?

@weinand
Copy link
Contributor

weinand commented Mar 22, 2018

The log message is plain text (as in VS and VS for Mac), not an expression. Expressions are only interpolated within curly brackets. @auchenberg's PR assumed that the logMessage is an evaluatable language construct (which is not the case).

Please see the VS and "VS for Mac" screenshots above in my comment #163 (comment)

@roblourens
Copy link
Member

roblourens commented Mar 22, 2018

Got it, added the same.

roblourens added a commit to microsoft/vscode-chrome-debug-core that referenced this issue Mar 22, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants