Skip to content

Commit

Permalink
debt - remove IWindowService#log in favor of a logger that can log to…
Browse files Browse the repository at this point in the history
… the main side
  • Loading branch information
bpasero committed Sep 17, 2019
1 parent 36e6149 commit 7ecb582
Show file tree
Hide file tree
Showing 17 changed files with 155 additions and 69 deletions.
6 changes: 3 additions & 3 deletions src/vs/code/electron-browser/issue/issueReporterMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { EnvironmentService } from 'vs/platform/environment/node/environmentServ
import { IssueReporterModel, IssueReporterData as IssueReporterModelData } from 'vs/code/electron-browser/issue/issueReporterModel';
import { IssueReporterData, IssueReporterStyles, IssueType, ISettingsSearchIssueReporterData, IssueReporterFeatures, IssueReporterExtensionData } from 'vs/platform/issue/node/issue';
import BaseHtml from 'vs/code/electron-browser/issue/issueReporterPage';
import { LogLevelSetterChannelClient, FollowerLogService } from 'vs/platform/log/common/logIpc';
import { LoggerChannelClient, FollowerLogService } from 'vs/platform/log/common/logIpc';
import { ILogService, getLogLevel } from 'vs/platform/log/common/log';
import { OcticonLabel } from 'vs/base/browser/ui/octiconLabel/octiconLabel';
import { normalizeGitHubUrl } from 'vs/code/electron-browser/issue/issueReporterUtil';
Expand Down Expand Up @@ -300,8 +300,8 @@ export class IssueReporter extends Disposable {
this.environmentService = new EnvironmentService(configuration, configuration.execPath);

const logService = new SpdLogService(`issuereporter${configuration.windowId}`, this.environmentService.logsPath, getLogLevel(this.environmentService));
const logLevelClient = new LogLevelSetterChannelClient(mainProcessService.getChannel('loglevel'));
this.logService = new FollowerLogService(logLevelClient, logService);
const loggerClient = new LoggerChannelClient(mainProcessService.getChannel('logger'));
this.logService = new FollowerLogService(loggerClient, logService);

const sharedProcess = (<IWindowsService>serviceCollection.get(IWindowsService)).whenSharedProcessReady()
.then(() => connectNet(this.environmentService.sharedIPCHandle, `window:${configuration.windowId}`));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { IWindowsService } from 'vs/platform/windows/common/windows';
import { WindowsService } from 'vs/platform/windows/electron-browser/windowsService';
import { ipcRenderer } from 'electron';
import { ILogService, LogLevel } from 'vs/platform/log/common/log';
import { LogLevelSetterChannelClient, FollowerLogService } from 'vs/platform/log/common/logIpc';
import { LoggerChannelClient, FollowerLogService } from 'vs/platform/log/common/logIpc';
import { LocalizationsService } from 'vs/platform/localizations/node/localizations';
import { ILocalizationsService } from 'vs/platform/localizations/common/localizations';
import { LocalizationsChannel } from 'vs/platform/localizations/node/localizationsIpc';
Expand Down Expand Up @@ -96,8 +96,8 @@ async function main(server: Server, initData: ISharedProcessInitData, configurat
const environmentService = new EnvironmentService(initData.args, process.execPath);

const mainRouter = new StaticRouter(ctx => ctx === 'main');
const logLevelClient = new LogLevelSetterChannelClient(server.getChannel('loglevel', mainRouter));
const logService = new FollowerLogService(logLevelClient, new SpdLogService('sharedprocess', environmentService.logsPath, initData.logLevel));
const loggerClient = new LoggerChannelClient(server.getChannel('logger', mainRouter));
const logService = new FollowerLogService(loggerClient, new SpdLogService('sharedprocess', environmentService.logsPath, initData.logLevel));
disposables.add(logService);
logService.info('main', JSON.stringify(configuration));

Expand Down Expand Up @@ -135,7 +135,7 @@ async function main(server: Server, initData: ISharedProcessInitData, configurat
const services = new ServiceCollection();
const environmentService = accessor.get(IEnvironmentService);
const { appRoot, extensionsPath, extensionDevelopmentLocationURI: extensionDevelopmentLocationURI, isBuilt, installSourcePath } = environmentService;
const telemetryLogService = new FollowerLogService(logLevelClient, new SpdLogService('telemetry', environmentService.logsPath, initData.logLevel));
const telemetryLogService = new FollowerLogService(loggerClient, new SpdLogService('telemetry', environmentService.logsPath, initData.logLevel));
telemetryLogService.info('The below are logs for every telemetry event sent from VS Code once the log level is set to trace.');
telemetryLogService.info('===========================================================');

Expand Down
8 changes: 4 additions & 4 deletions src/vs/code/electron-main/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import { DarwinUpdateService } from 'vs/platform/update/electron-main/updateServ
import { IIssueService } from 'vs/platform/issue/node/issue';
import { IssueChannel } from 'vs/platform/issue/electron-main/issueIpc';
import { IssueMainService } from 'vs/platform/issue/electron-main/issueMainService';
import { LogLevelSetterChannel } from 'vs/platform/log/common/logIpc';
import { LoggerChannel } from 'vs/platform/log/common/logIpc';
import { setUnexpectedErrorHandler, onUnexpectedError } from 'vs/base/common/errors';
import { ElectronURLListener } from 'vs/platform/url/electron-main/electronUrlListener';
import { serve as serveDriver } from 'vs/platform/driver/electron-main/driver';
Expand Down Expand Up @@ -567,9 +567,9 @@ export class CodeApplication extends Disposable {
const storageChannel = this._register(new GlobalStorageDatabaseChannel(this.logService, storageMainService));
electronIpcServer.registerChannel('storage', storageChannel);

const logLevelChannel = new LogLevelSetterChannel(accessor.get(ILogService));
electronIpcServer.registerChannel('loglevel', logLevelChannel);
sharedProcessClient.then(client => client.registerChannel('loglevel', logLevelChannel));
const loggerChannel = new LoggerChannel(accessor.get(ILogService));
electronIpcServer.registerChannel('logger', loggerChannel);
sharedProcessClient.then(client => client.registerChannel('logger', loggerChannel));

// ExtensionHost Debug broadcast service
electronIpcServer.registerChannel(ExtensionHostDebugBroadcastChannel.ChannelName, new ExtensionHostDebugBroadcastChannel());
Expand Down
51 changes: 50 additions & 1 deletion src/vs/platform/log/common/log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { IDisposable, Disposable } from 'vs/base/common/lifecycle';
import { isWindows } from 'vs/base/common/platform';
import { Event, Emitter } from 'vs/base/common/event';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
import { LoggerChannelClient } from 'vs/platform/log/common/logIpc';

export const ILogService = createServiceDecorator<ILogService>('logService');

Expand Down Expand Up @@ -183,6 +184,54 @@ export class ConsoleLogService extends AbstractLogService implements ILogService
dispose(): void { }
}

export class ConsoleLogInMainService extends AbstractLogService implements ILogService {

_serviceBrand: undefined;

constructor(private readonly client: LoggerChannelClient, logLevel: LogLevel = DEFAULT_LOG_LEVEL) {
super();
this.setLevel(logLevel);
}

trace(message: string, ...args: any[]): void {
if (this.getLevel() <= LogLevel.Trace) {
this.client.consoleLog('trace', [message, ...args]);
}
}

debug(message: string, ...args: any[]): void {
if (this.getLevel() <= LogLevel.Debug) {
this.client.consoleLog('debug', [message, ...args]);
}
}

info(message: string, ...args: any[]): void {
if (this.getLevel() <= LogLevel.Info) {
this.client.consoleLog('info', [message, ...args]);
}
}

warn(message: string | Error, ...args: any[]): void {
if (this.getLevel() <= LogLevel.Warning) {
this.client.consoleLog('warn', [message, ...args]);
}
}

error(message: string, ...args: any[]): void {
if (this.getLevel() <= LogLevel.Error) {
this.client.consoleLog('error', [message, ...args]);
}
}

critical(message: string, ...args: any[]): void {
if (this.getLevel() <= LogLevel.Critical) {
this.client.consoleLog('critical', [message, ...args]);
}
}

dispose(): void { }
}

export class MultiplexLogService extends AbstractLogService implements ILogService {
_serviceBrand: undefined;

Expand Down Expand Up @@ -326,4 +375,4 @@ export function getLogLevel(environmentService: IEnvironmentService): LogLevel {
}
}
return DEFAULT_LOG_LEVEL;
}
}
31 changes: 27 additions & 4 deletions src/vs/platform/log/common/logIpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { IChannel, IServerChannel } from 'vs/base/parts/ipc/common/ipc';
import { LogLevel, ILogService, DelegatedLogService } from 'vs/platform/log/common/log';
import { Event } from 'vs/base/common/event';

export class LogLevelSetterChannel implements IServerChannel {
export class LoggerChannel implements IServerChannel {

onDidChangeLogLevel: Event<LogLevel>;

Expand All @@ -26,13 +26,32 @@ export class LogLevelSetterChannel implements IServerChannel {
call(_: unknown, command: string, arg?: any): Promise<any> {
switch (command) {
case 'setLevel': this.service.setLevel(arg); return Promise.resolve();
case 'consoleLog': this.consoleLog(arg[0], arg[1]); return Promise.resolve();
}

throw new Error(`Call not found: ${command}`);
}

private consoleLog(severity: string, args: string[]): void {
let consoleFn = console.log;

switch (severity) {
case 'error':
consoleFn = console.error;
break;
case 'warn':
consoleFn = console.warn;
break;
case 'info':
consoleFn = console.info;
break;
}

consoleFn.call(console, ...args);
}
}

export class LogLevelSetterChannelClient {
export class LoggerChannelClient {

constructor(private channel: IChannel) { }

Expand All @@ -43,17 +62,21 @@ export class LogLevelSetterChannelClient {
setLevel(level: LogLevel): void {
this.channel.call('setLevel', level);
}

consoleLog(severity: string, args: string[]): void {
this.channel.call('consoleLog', [severity, args]);
}
}

export class FollowerLogService extends DelegatedLogService implements ILogService {
_serviceBrand: undefined;

constructor(private master: LogLevelSetterChannelClient, logService: ILogService) {
constructor(private master: LoggerChannelClient, logService: ILogService) {
super(logService);
this._register(master.onDidChangeLogLevel(level => logService.setLevel(level)));
}

setLevel(level: LogLevel): void {
this.master.setLevel(level);
}
}
}
1 change: 0 additions & 1 deletion src/vs/platform/windows/common/windows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ export interface IWindowsService {
openExtensionDevelopmentHostWindow(args: ParsedArgs, env: IProcessEnvironment): Promise<void>;
getWindows(): Promise<{ id: number; workspace?: IWorkspaceIdentifier; folderUri?: ISingleFolderWorkspaceIdentifier; title: string; filename?: string; }[]>;
getWindowCount(): Promise<number>;
log(severity: string, args: string[]): Promise<void>;
showItemInFolder(path: URI): Promise<void>;
getActiveWindowId(): Promise<number | undefined>;

Expand Down
1 change: 0 additions & 1 deletion src/vs/platform/windows/common/windowsIpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ export class WindowsChannel implements IServerChannel {
case 'whenSharedProcessReady': return this.service.whenSharedProcessReady();
case 'toggleSharedProcess': return this.service.toggleSharedProcess();
case 'quit': return this.service.quit();
case 'log': return this.service.log(arg[0], arg[1]);
case 'showItemInFolder': return this.service.showItemInFolder(URI.revive(arg));
case 'getActiveWindowId': return this.service.getActiveWindowId();
case 'openExternal': return this.service.openExternal(arg);
Expand Down
4 changes: 0 additions & 4 deletions src/vs/platform/windows/electron-browser/windowsService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,6 @@ export class WindowsService implements IWindowsService {
return this.channel.call('getWindowCount');
}

log(severity: string, args: string[]): Promise<void> {
return this.channel.call('log', [severity, args]);
}

showItemInFolder(path: URI): Promise<void> {
return this.channel.call('showItemInFolder', path);
}
Expand Down
18 changes: 0 additions & 18 deletions src/vs/platform/windows/electron-main/windowsService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,24 +336,6 @@ export class WindowsService extends Disposable implements IWindowsService, IURLH
return this.windowsMainService.getWindows().length;
}

async log(severity: string, args: string[]): Promise<void> {
let consoleFn = console.log;

switch (severity) {
case 'error':
consoleFn = console.error;
break;
case 'warn':
consoleFn = console.warn;
break;
case 'info':
consoleFn = console.info;
break;
}

consoleFn.call(console, ...args);
}

async showItemInFolder(resource: URI): Promise<void> {
this.logService.trace('windowsService#showItemInFolder');

Expand Down
9 changes: 5 additions & 4 deletions src/vs/workbench/api/browser/mainThreadConsole.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers';
import { MainContext, MainThreadConsoleShape, IExtHostContext } from 'vs/workbench/api/common/extHost.protocol';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
import { IRemoteConsoleLog, log, parse } from 'vs/base/common/console';
import { IRemoteConsoleLog, log } from 'vs/base/common/console';
import { logRemoteEntry } from 'vs/workbench/services/extensions/common/remoteConsoleUtil';
import { parseExtensionDevOptions } from 'vs/workbench/services/extensions/common/extensionDevOptions';
import { IWindowsService } from 'vs/platform/windows/common/windows';
import { ILogService } from 'vs/platform/log/common/log';
import { IExtensionHostDebugService } from 'vs/platform/debug/common/extensionHostDebug';

@extHostNamedCustomer(MainContext.MainThreadConsole)
Expand All @@ -20,7 +21,7 @@ export class MainThreadConsole implements MainThreadConsoleShape {
constructor(
extHostContext: IExtHostContext,
@IEnvironmentService private readonly _environmentService: IEnvironmentService,
@IWindowsService private readonly _windowsService: IWindowsService,
@ILogService private readonly _logService: ILogService,
@IExtensionHostDebugService private readonly _extensionHostDebugService: IExtensionHostDebugService,
) {
const devOpts = parseExtensionDevOptions(this._environmentService);
Expand All @@ -40,7 +41,7 @@ export class MainThreadConsole implements MainThreadConsoleShape {

// Log on main side if running tests from cli
if (this._isExtensionDevTestFromCli) {
this._windowsService.log(entry.severity, parse(entry).args);
logRemoteEntry(this._logService, entry);
}

// Broadcast to other windows if we are in development mode
Expand Down
4 changes: 0 additions & 4 deletions src/vs/workbench/browser/web.simpleservices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,10 +508,6 @@ export class SimpleWindowsService implements IWindowsService {
return Promise.resolve(this.windowCount);
}

log(_severity: string, _args: string[]): Promise<void> {
return Promise.resolve();
}

showItemInFolder(_path: URI): Promise<void> {
return Promise.resolve();
}
Expand Down
8 changes: 4 additions & 4 deletions src/vs/workbench/contrib/remote/common/remote.contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { OperatingSystem, isWeb } from 'vs/base/common/platform';
import { Schemas } from 'vs/base/common/network';
import { IRemoteAgentService, RemoteExtensionLogFileName } from 'vs/workbench/services/remote/common/remoteAgentService';
import { ILogService } from 'vs/platform/log/common/log';
import { LogLevelSetterChannelClient } from 'vs/platform/log/common/logIpc';
import { LoggerChannelClient } from 'vs/platform/log/common/logIpc';
import { IOutputChannelRegistry, Extensions as OutputExt, } from 'vs/workbench/contrib/output/common/output';
import { localize } from 'vs/nls';
import { joinPath } from 'vs/base/common/resources';
Expand Down Expand Up @@ -79,9 +79,9 @@ class RemoteChannelsContribution extends Disposable implements IWorkbenchContrib
super();
const connection = remoteAgentService.getConnection();
if (connection) {
const logLevelClient = new LogLevelSetterChannelClient(connection.getChannel('loglevel'));
logLevelClient.setLevel(logService.getLevel());
this._register(logService.onDidChangeLogLevel(level => logLevelClient.setLevel(level)));
const loggerClient = new LoggerChannelClient(connection.getChannel('logger'));
loggerClient.setLevel(logService.getLevel());
this._register(logService.onDidChangeLogLevel(level => loggerClient.setLevel(level)));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { ILogService } from 'vs/platform/log/common/log';
import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
import { DialogChannel } from 'vs/platform/dialogs/electron-browser/dialogIpc';
import { DownloadServiceChannel } from 'vs/platform/download/common/downloadIpc';
import { LogLevelSetterChannel } from 'vs/platform/log/common/logIpc';
import { LoggerChannel } from 'vs/platform/log/common/logIpc';
import { ipcRenderer as ipc } from 'electron';
import { IDiagnosticInfoOptions, IRemoteDiagnosticInfo } from 'vs/platform/diagnostics/common/diagnostics';
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
Expand Down Expand Up @@ -229,7 +229,7 @@ class RemoteChannelsContribution implements IWorkbenchContribution {
if (connection) {
connection.registerChannel('dialog', new DialogChannel(dialogService));
connection.registerChannel('download', new DownloadServiceChannel(downloadService));
connection.registerChannel('loglevel', new LogLevelSetterChannel(logService));
connection.registerChannel('logger', new LoggerChannel(logService));
}
}
}
Expand Down
27 changes: 20 additions & 7 deletions src/vs/workbench/electron-browser/desktop.main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ import { KeyboardMapperFactory } from 'vs/workbench/services/keybinding/electron
import { IWindowConfiguration } from 'vs/platform/windows/common/windows';
import { webFrame } from 'electron';
import { ISingleFolderWorkspaceIdentifier, IWorkspaceInitializationPayload, ISingleFolderWorkspaceInitializationPayload, reviveWorkspaceIdentifier } from 'vs/platform/workspaces/common/workspaces';
import { ConsoleLogService, MultiplexLogService, ILogService } from 'vs/platform/log/common/log';
import { ConsoleLogService, MultiplexLogService, ILogService, ConsoleLogInMainService } from 'vs/platform/log/common/log';
import { StorageService } from 'vs/platform/storage/node/storageService';
import { LogLevelSetterChannelClient, FollowerLogService } from 'vs/platform/log/common/logIpc';
import { LoggerChannelClient, FollowerLogService } from 'vs/platform/log/common/logIpc';
import { Schemas } from 'vs/base/common/network';
import { sanitizeFilePath } from 'vs/base/common/extpath';
import { GlobalStorageDatabaseChannelClient } from 'vs/platform/storage/node/storageIpc';
Expand Down Expand Up @@ -345,12 +345,25 @@ class CodeRendererMain extends Disposable {
}

private createLogService(mainProcessService: IMainProcessService, environmentService: IWorkbenchEnvironmentService): ILogService {
const spdlogService = new SpdLogService(`renderer${this.environmentService.configuration.windowId}`, environmentService.logsPath, this.environmentService.configuration.logLevel);
const consoleLogService = new ConsoleLogService(this.environmentService.configuration.logLevel);
const logService = new MultiplexLogService([consoleLogService, spdlogService]);
const logLevelClient = new LogLevelSetterChannelClient(mainProcessService.getChannel('loglevel'));
const loggerClient = new LoggerChannelClient(mainProcessService.getChannel('logger'));

// Extension development test CLI: forward everything to main side
const loggers: ILogService[] = [];
if (environmentService.isExtensionDevelopment && !!environmentService.extensionTestsLocationURI) {
loggers.push(
new ConsoleLogInMainService(loggerClient, this.environmentService.configuration.logLevel)
);
}

// Normal logger: spdylog and console
else {
loggers.push(
new ConsoleLogService(this.environmentService.configuration.logLevel),
new SpdLogService(`renderer${this.environmentService.configuration.windowId}`, environmentService.logsPath, this.environmentService.configuration.logLevel)
);
}

return new FollowerLogService(logLevelClient, logService);
return new FollowerLogService(loggerClient, new MultiplexLogService(loggers));
}
}

Expand Down
Loading

0 comments on commit 7ecb582

Please sign in to comment.