Promote extension log API to stable #43276

roblourens · 2018-02-09T01:55:37Z

What's the protocol for this? Discussion here and merge it if I get 👍s?

Earlier discussion at 98c58df

Fixes #43275

nzec

Looks good to me!

nzec · 2018-02-09T09:45:16Z

Check Travis though. Seems like all good there.

weinand · 2018-02-09T10:24:52Z

@roblourens I'm fine with the new API but Joh has the final say as he is the API police ;-)

mjbvz

New interfaces look good to me. We should add comments on the Logger interface properties too before shipping this

alexdima

With this addition, we will have two somewhat competing APIs, the output channel and the logger.

IMHO, we should make it clear which one an extension author is supposed to use.

alexdima · 2018-02-15T11:10:35Z

src/vs/vscode.d.ts

+	export interface Logger {
+		readonly onDidChangeLogLevel: Event<LogLevel>;
+		readonly currentLevel: LogLevel;
+		readonly logDirectory: Thenable<string>;


Typically, we make read API synchronous. Why is logDirectory a Thenable ?

The directory is only created when this is called or when the logger is used. Otherwise there will be a ton of empty folders sitting around.

@roblourens IMHO that is super weird. I've never seen a lazy getter field that creates a directory when accessed. Is there no other way to solve the problem of the empty folders ? Why does each extension need to get its own folder ? Can't we use the extension id as the filename and thus avoid collisions ?

If an extension owns it own log file(s), where would it write to? The main logging directory? Then it would have to know about the window id or rotate files itself.

It could be a get() method too.

alexdima · 2018-02-15T11:11:07Z

src/vs/vscode.d.ts

+	 */
+	export interface Logger {
+		readonly onDidChangeLogLevel: Event<LogLevel>;
+		readonly currentLevel: LogLevel;


Why is the log level be exposed as API ?

One use case might be when for when an extension is managing its own logging and writing to a file on its own.

Has anyone asked for this? There are very many things we could add to the API, but IMHO it is a good practice to be as limited as possible and add something only if really needed...

If we expose the log directory, we have to expose this, it makes no sense to only provide one.

alexdima · 2018-02-16T12:58:01Z

@roblourens There is still my question:

As an extension author when should I use an output channel and when should I use the logger?

Are we deprecating the output channel API ?
The output channel API allows to reveal the output, for example; How can one reveal the extension log via API ?
Is it possible to evolve the output channel API rather than deprecate it and replace it with something else ? i.e.:
- can we add the .trace, .debug, .log etc methods on the output channel
- can we implement the output channel to use spdlog? (I believe @sandy081 has done some work in this direction)

roblourens · 2018-02-18T15:45:06Z

I see these as different things, output channel for transient info that users would want to monitor, and logging for bug reports and info for developers.

I'm actually not sure why we show all the log files in the output pane - do we expect users to want to look at these? Even as a developer I don't see generally useful info there unless I'm looking at a specific component or issue.

But if my view is wrong then maybe there's room to merge them instead.

sandy081 · 2018-02-20T14:14:33Z

Yes, we use spdlog internally for implementing output channels.

We already use Output channel to show extension logs, example: Typescript log. May be there are more extensions doing the same. So, I see the point that it might confuse the authors when to use Output channel and when to use log api. To me, log api looks more as a utility than an API to the extensions.

Also, we can enhance Output to stream the log files from extensions into output channel.

roblourens · 2018-02-20T14:33:47Z

TypeScript has a verbose logging mode which writes tons of info to a file, but doesn't use the output channel. Spamming the output channel in that case would just slow things down.

I assume that the output channel API transfers all data to the renderer process whether or not the output channel is visible. So that's a big difference between the two.

sandy081 · 2018-02-20T16:59:43Z

I do not think there will be any slow down or data transfer. If we provide an api to create a file based output channel and if extension has a log file and wanted to show in output channel, then it can create an output channel using the new API passing the log file URI.

Streaming of data happens in the renderer which will be similar to our internal log channels inside product. I did several improvements to output channels in last milestone and one of those is that no streaming happens when a file based channel is not visible.

alexdima · 2018-02-21T09:53:55Z

I see these as different things, output channel for transient info that users would want to monitor, and logging for bug reports and info for developers.

I, personally, as a user, never ever want to see output channels, unless we're talking about the output of executing my program or the output from testing my program. I find it hard to imagine there are people that want to monitor output channels. Take, for example, the following output channels: git, tslint or TypeScript. I could care less what they are writing in the output channel. Also, all the messages in there are crafted by the developers of those components and I have no clue what they mean. The only time I would care is if I would file an issue and the developer asks me to paste that information.

My point is that nobody wants to monitor output channels as part of their regular flow, similar to how nobody wants to monitor log files as part of their regular flow. So, IMHO, output channels are log files. They simply miss some tracing level, some special methods to avoid writing ifs all over the place, and perhaps could benefit from more optimization. I am not sure if the output is written to disk directly from the extension host process or it travels to the renderer process; we could write it to disk from the extension host process for all output channels, as they would all benefit from this optimization.

If I understand you correctly, we have the requirement that extensions maybe want to log from a separate process than the extension host. In such a case, I believe they would use a logging library available in their runtime's programming language. e.g. OmniSharp would use a logger written in C#, TypeScript could also use spdlog, etc. I understand how it is useful in such a case for them to write their log files close to the other VS Code log files, and us giving them a folder path and log level API makes good sense. It's just that these should be synchronous, not asynchronous, and should be pure. Getting a property should, IMHO, have no side effects, such as creating a folder on disk...

jrieken · 2018-03-01T11:29:17Z

What's the protocol for this?

@roblourens Glad you are asking..

start are the api guidelines, then
propose something that doesn't conflict with the guidelines, then
discuss.

In this PR we are still on phase 2 (propose something that doesn't conflict with the guidelines) and maybe on something like phase 0 - in what shape do we want/need this api.

I agree with @alexandrudima that there is too much overlap with output channels (be minimal) and if we untangle the proposal into two pieces we might end up with a better solution. One part of this proposal is being part of the vs code logging infrastructure and using (improved) output channels seems like a promising idea/exploration. The other part is exposing log config data, like level and file location. If those aren't extension-specific we should look into adding something to vscode.env, if they are extension specific something on the ExtensionContext is required.

jrieken · 2018-03-15T16:52:37Z

Outcome of discussing this

have logDirectory a property of the extension context (in the spirit of storagePath)
move log level information and corresponding event to vscode.env as there is only a global log level in vscode
move the logging-related methods to the output channel api.

We are aiming to deliver the first two items during this milestone and consider the last as stretch.

roblourens · 2018-03-22T03:39:52Z

This PR is now n/a

roblourens added the api label Feb 9, 2018

roblourens added this to the February 2018 milestone Feb 9, 2018

roblourens self-assigned this Feb 9, 2018

roblourens requested review from jrieken, weinand and alexdima February 9, 2018 01:55

nzec approved these changes Feb 9, 2018

View reviewed changes

mjbvz approved these changes Feb 9, 2018

View reviewed changes

roblourens added 2 commits February 12, 2018 16:32

Promote extension log API to stable - Fix #43275

2e45f55

Add comments for Logger methods

1fd0f50

roblourens force-pushed the roblou/promoteExtLogAPI branch from 8351e06 to 1fd0f50 Compare February 13, 2018 00:34

kieferrm mentioned this pull request Feb 14, 2018

Iteration Plan for February 2018 #43361

Closed

45 tasks

alexdima reviewed Feb 15, 2018

View reviewed changes

roblourens modified the milestones: February 2018, March 2018 Feb 22, 2018

joaomoreno mentioned this pull request Feb 23, 2018

Question: Logging in Extensions #44212

Closed

jrieken added the log VS Code log issues label Mar 1, 2018

kieferrm mentioned this pull request Mar 13, 2018

Iteration Plan for March 2018 #45632

Closed

70 tasks

roblourens closed this Mar 22, 2018

kieferrm mentioned this pull request Aug 7, 2018

Iteration Plan for August 2018 #55927

Closed

39 tasks

jrieken mentioned this pull request Aug 9, 2018

Promote extension log API to stable #43275

Closed

weinand mentioned this pull request Aug 17, 2018

Do not use deprecated logger API #56658

Closed

bryceitoc9 mentioned this pull request Feb 22, 2019

Log Implementation aws/aws-toolkit-vscode#302

Merged

20 tasks

github-actions bot locked and limited conversation to collaborators Mar 31, 2020

roblourens deleted the roblou/promoteExtLogAPI branch June 29, 2020 19:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Promote extension log API to stable #43276

Promote extension log API to stable #43276

roblourens commented Feb 9, 2018

nzec left a comment

nzec commented Feb 9, 2018 •

edited

Loading

weinand commented Feb 9, 2018

mjbvz left a comment

alexdima left a comment

alexdima Feb 15, 2018

roblourens Feb 15, 2018 •

edited

Loading

alexdima Feb 16, 2018

roblourens Feb 18, 2018

alexdima Feb 15, 2018

roblourens Feb 15, 2018

alexdima Feb 16, 2018

roblourens Feb 18, 2018

alexdima commented Feb 16, 2018 •

edited

Loading

roblourens commented Feb 18, 2018

sandy081 commented Feb 20, 2018

roblourens commented Feb 20, 2018

sandy081 commented Feb 20, 2018

alexdima commented Feb 21, 2018 •

edited

Loading

jrieken commented Mar 1, 2018 •

edited

Loading

jrieken commented Mar 15, 2018

roblourens commented Mar 22, 2018

Promote extension log API to stable #43276

Promote extension log API to stable #43276

Conversation

roblourens commented Feb 9, 2018

nzec left a comment

Choose a reason for hiding this comment

nzec commented Feb 9, 2018 • edited Loading

weinand commented Feb 9, 2018

mjbvz left a comment

Choose a reason for hiding this comment

alexdima left a comment

Choose a reason for hiding this comment

alexdima Feb 15, 2018

Choose a reason for hiding this comment

roblourens Feb 15, 2018 • edited Loading

Choose a reason for hiding this comment

alexdima Feb 16, 2018

Choose a reason for hiding this comment

roblourens Feb 18, 2018

Choose a reason for hiding this comment

alexdima Feb 15, 2018

Choose a reason for hiding this comment

roblourens Feb 15, 2018

Choose a reason for hiding this comment

alexdima Feb 16, 2018

Choose a reason for hiding this comment

roblourens Feb 18, 2018

Choose a reason for hiding this comment

alexdima commented Feb 16, 2018 • edited Loading

roblourens commented Feb 18, 2018

sandy081 commented Feb 20, 2018

roblourens commented Feb 20, 2018

sandy081 commented Feb 20, 2018

alexdima commented Feb 21, 2018 • edited Loading

jrieken commented Mar 1, 2018 • edited Loading

jrieken commented Mar 15, 2018

roblourens commented Mar 22, 2018

nzec commented Feb 9, 2018 •

edited

Loading

roblourens Feb 15, 2018 •

edited

Loading

alexdima commented Feb 16, 2018 •

edited

Loading

alexdima commented Feb 21, 2018 •

edited

Loading

jrieken commented Mar 1, 2018 •

edited

Loading