Skip to content

Commit

Permalink
Use new logging API for python extension logger and LS logger (micros…
Browse files Browse the repository at this point in the history
…oft#21062)

In this PR:
1. Changes the python extension logging to use LogOutputChannel
2. Changes the language server logger with LogOutputChannel
3. Test output channel uses OutputChannel as it needs to show test
output and not really logging. Also, using logging test output makes it
pretty much unreadable.
4. Simplifies logging channel and output channel registration.

We need to do this now to make it easier for new test work to integrate
with output logging.

For microsoft#20844

This still doesn't get rid of the log level setting.
  • Loading branch information
karthiknadig authored and eleanorjboyd committed Apr 18, 2023
1 parent 6403d20 commit a1f5041
Show file tree
Hide file tree
Showing 39 changed files with 259 additions and 146 deletions.
4 changes: 2 additions & 2 deletions src/client/activation/common/analysisOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { DocumentFilter, LanguageClientOptions, RevealOutputChannelOn } from 'vs
import { IWorkspaceService } from '../../common/application/types';

import { PYTHON, PYTHON_LANGUAGE } from '../../common/constants';
import { IOutputChannel, Resource } from '../../common/types';
import { ILogOutputChannel, Resource } from '../../common/types';
import { debounceSync } from '../../common/utils/decorators';
import { IEnvironmentVariablesProvider } from '../../common/variables/types';
import { traceDecoratorError } from '../../logging';
Expand All @@ -14,7 +14,7 @@ import { ILanguageServerAnalysisOptions, ILanguageServerOutputChannel } from '..

export abstract class LanguageServerAnalysisOptionsBase implements ILanguageServerAnalysisOptions {
protected readonly didChange = new EventEmitter<void>();
private readonly output: IOutputChannel;
private readonly output: ILogOutputChannel;

protected constructor(
lsOutputChannel: ILanguageServerOutputChannel,
Expand Down
6 changes: 3 additions & 3 deletions src/client/activation/common/outputChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
import { inject, injectable } from 'inversify';
import { IApplicationShell, ICommandManager } from '../../common/application/types';
import '../../common/extensions';
import { IDisposableRegistry, IOutputChannel } from '../../common/types';
import { IDisposableRegistry, ILogOutputChannel } from '../../common/types';
import { OutputChannelNames } from '../../common/utils/localize';
import { ILanguageServerOutputChannel } from '../types';

@injectable()
export class LanguageServerOutputChannel implements ILanguageServerOutputChannel {
public output: IOutputChannel | undefined;
public output: ILogOutputChannel | undefined;

private registered = false;

Expand All @@ -22,7 +22,7 @@ export class LanguageServerOutputChannel implements ILanguageServerOutputChannel
@inject(IDisposableRegistry) private readonly disposable: IDisposableRegistry,
) {}

public get channel(): IOutputChannel {
public get channel(): ILogOutputChannel {
if (!this.output) {
this.output = this.appShell.createOutputChannel(OutputChannelNames.languageServer);
this.disposable.push(this.output);
Expand Down
6 changes: 3 additions & 3 deletions src/client/activation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { Event } from 'vscode';
import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient/node';
import type { IDisposable, IOutputChannel, Resource } from '../common/types';
import type { IDisposable, ILogOutputChannel, Resource } from '../common/types';
import { PythonEnvironment } from '../pythonEnvironments/info';

export const IExtensionActivationManager = Symbol('IExtensionActivationManager');
Expand Down Expand Up @@ -110,10 +110,10 @@ export interface ILanguageServerOutputChannel {
/**
* Creates output channel if necessary and returns it
*
* @type {IOutputChannel}
* @type {ILogOutputChannel}
* @memberof ILanguageServerOutputChannel
*/
readonly channel: IOutputChannel;
readonly channel: ILogOutputChannel;
}

export const IExtensionSingleActivationService = Symbol('IExtensionSingleActivationService');
Expand Down
6 changes: 3 additions & 3 deletions src/client/common/application/applicationShell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import {
InputBoxOptions,
languages,
LanguageStatusItem,
LogOutputChannel,
MessageItem,
MessageOptions,
OpenDialogOptions,
OutputChannel,
Progress,
ProgressOptions,
QuickPick,
Expand Down Expand Up @@ -166,8 +166,8 @@ export class ApplicationShell implements IApplicationShell {
public createTreeView<T>(viewId: string, options: TreeViewOptions<T>): TreeView<T> {
return window.createTreeView<T>(viewId, options);
}
public createOutputChannel(name: string): OutputChannel {
return window.createOutputChannel(name);
public createOutputChannel(name: string): LogOutputChannel {
return window.createOutputChannel(name, { log: true });
}
public createLanguageStatusItem(id: string, selector: DocumentSelector): LanguageStatusItem {
return languages.createLanguageStatusItem(id, selector);
Expand Down
4 changes: 2 additions & 2 deletions src/client/common/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ import {
InputBox,
InputBoxOptions,
LanguageStatusItem,
LogOutputChannel,
MessageItem,
MessageOptions,
OpenDialogOptions,
OutputChannel,
Progress,
ProgressOptions,
QuickPick,
Expand Down Expand Up @@ -429,7 +429,7 @@ export interface IApplicationShell {
*
* @param name Human-readable string which will be used to represent the channel in the UI.
*/
createOutputChannel(name: string): OutputChannel;
createOutputChannel(name: string): LogOutputChannel;
createLanguageStatusItem(id: string, selector: DocumentSelector): LanguageStatusItem;
}

Expand Down
2 changes: 0 additions & 2 deletions src/client/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ export namespace ThemeIcons {

export const DEFAULT_INTERPRETER_SETTING = 'python';

export const STANDARD_OUTPUT_CHANNEL = 'STANDARD_OUTPUT_CHANNEL';

export const isCI = process.env.TRAVIS === 'true' || process.env.TF_BUILD !== undefined;

export function isTestExecution(): boolean {
Expand Down
5 changes: 2 additions & 3 deletions src/client/common/installer/moduleInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { IApplicationShell } from '../application/types';
import { wrapCancellationTokens } from '../cancellation';
import { STANDARD_OUTPUT_CHANNEL } from '../constants';
import { IFileSystem } from '../platform/types';
import * as internalPython from '../process/internal/python';
import { IProcessServiceFactory } from '../process/types';
import { ITerminalServiceFactory, TerminalCreationOptions } from '../terminal/types';
import { ExecutionInfo, IConfigurationService, IOutputChannel, Product } from '../types';
import { ExecutionInfo, IConfigurationService, ILogOutputChannel, Product } from '../types';
import { isResource } from '../utils/misc';
import { ProductNames } from './productNames';
import { IModuleInstaller, InstallOptions, InterpreterUri, ModuleInstallFlags } from './types';
Expand Down Expand Up @@ -152,7 +151,7 @@ export abstract class ModuleInstaller implements IModuleInstaller {
const options = {
name: 'VS Code Python',
};
const outputChannel = this.serviceContainer.get<IOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
const outputChannel = this.serviceContainer.get<ILogOutputChannel>(ILogOutputChannel);
const command = `"${execPath.replace(/\\/g, '/')}" ${args.join(' ')}`;

traceLog(`[Elevated] ${command}`);
Expand Down
6 changes: 5 additions & 1 deletion src/client/common/process/rawProcessApis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,10 @@ export function plainExec(
}

const stdoutBuffers: Buffer[] = [];
on(proc.stdout, 'data', (data: Buffer) => stdoutBuffers.push(data));
on(proc.stdout, 'data', (data: Buffer) => {
stdoutBuffers.push(data);
options.outputChannel?.append(data.toString());
});
const stderrBuffers: Buffer[] = [];
on(proc.stderr, 'data', (data: Buffer) => {
if (options.mergeStdOutErr) {
Expand All @@ -130,6 +133,7 @@ export function plainExec(
} else {
stderrBuffers.push(data);
}
options.outputChannel?.append(data.toString());
});

proc.once('close', () => {
Expand Down
3 changes: 2 additions & 1 deletion src/client/common/process/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import { ChildProcess, ExecOptions, SpawnOptions as ChildProcessSpawnOptions } from 'child_process';
import { Observable } from 'rxjs/Observable';
import { CancellationToken, Uri } from 'vscode';
import { CancellationToken, OutputChannel, Uri } from 'vscode';
import { PythonExecInfo } from '../../pythonEnvironments/exec';
import { InterpreterInformation, PythonEnvironment } from '../../pythonEnvironments/info';
import { ExecutionInfo, IDisposable } from '../types';
Expand All @@ -24,6 +24,7 @@ export type SpawnOptions = ChildProcessSpawnOptions & {
mergeStdOutErr?: boolean;
throwOnStdErr?: boolean;
extraVariables?: NodeJS.ProcessEnv;
outputChannel?: OutputChannel;
};

export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean };
Expand Down
9 changes: 6 additions & 3 deletions src/client/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import {
Extension,
ExtensionContext,
Memento,
OutputChannel,
LogOutputChannel,
Uri,
WorkspaceEdit,
OutputChannel,
} from 'vscode';
import { LanguageServerType } from '../activation/types';
import type { InstallOptions, InterpreterUri, ModuleInstallFlags } from './installer/types';
Expand All @@ -29,8 +30,10 @@ export interface IDisposable {
dispose(): void | undefined | Promise<void>;
}

export const IOutputChannel = Symbol('IOutputChannel');
export interface IOutputChannel extends OutputChannel {}
export const ILogOutputChannel = Symbol('ILogOutputChannel');
export interface ILogOutputChannel extends LogOutputChannel {}
export const ITestOutputChannel = Symbol('ITestOutputChannel');
export interface ITestOutputChannel extends OutputChannel {}
export const IDocumentSymbolProvider = Symbol('IDocumentSymbolProvider');
export interface IDocumentSymbolProvider extends DocumentSymbolProvider {}
export const IsWindows = Symbol('IS_WINDOWS');
Expand Down
15 changes: 4 additions & 11 deletions src/client/extensionActivation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,22 @@

'use strict';

import {
debug,
DebugConfigurationProvider,
DebugConfigurationProviderTriggerKind,
languages,
OutputChannel,
window,
} from 'vscode';
import { debug, DebugConfigurationProvider, DebugConfigurationProviderTriggerKind, languages, window } from 'vscode';

import { registerTypes as activationRegisterTypes } from './activation/serviceRegistry';
import { IExtensionActivationManager } from './activation/types';
import { registerTypes as appRegisterTypes } from './application/serviceRegistry';
import { IApplicationDiagnostics } from './application/types';
import { IApplicationEnvironment, ICommandManager, IWorkspaceService } from './common/application/types';
import { Commands, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL, UseProposedApi } from './common/constants';
import { Commands, PYTHON, PYTHON_LANGUAGE, UseProposedApi } from './common/constants';
import { registerTypes as installerRegisterTypes } from './common/installer/serviceRegistry';
import { IFileSystem } from './common/platform/types';
import {
IConfigurationService,
IDisposableRegistry,
IExtensions,
IInterpreterPathService,
IOutputChannel,
ILogOutputChannel,
IPathUtils,
} from './common/types';
import { noop } from './common/utils/misc';
Expand Down Expand Up @@ -173,7 +166,7 @@ async function activateLegacy(ext: ExtensionState): Promise<ActivationResult> {
const dispatcher = new DebugSessionEventDispatcher(handlers, DebugService.instance, disposables);
dispatcher.registerEventHandlers();

const outputChannel = serviceManager.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
const outputChannel = serviceManager.get<ILogOutputChannel>(ILogOutputChannel);
disposables.push(cmdManager.registerCommand(Commands.ViewOutput, () => outputChannel.show()));
cmdManager.executeCommand('setContext', 'python.vscode.channel', applicationEnv.channel).then(noop, noop);

Expand Down
15 changes: 7 additions & 8 deletions src/client/extensionInit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
'use strict';

import { Container } from 'inversify';
import { Disposable, Memento, OutputChannel, window } from 'vscode';
import { Disposable, Memento, window } from 'vscode';
import { instance, mock } from 'ts-mockito';
import { STANDARD_OUTPUT_CHANNEL } from './common/constants';
import { registerTypes as platformRegisterTypes } from './common/platform/serviceRegistry';
import { registerTypes as processRegisterTypes } from './common/process/serviceRegistry';
import { registerTypes as commonRegisterTypes } from './common/serviceRegistry';
Expand All @@ -16,7 +15,8 @@ import {
IDisposableRegistry,
IExtensionContext,
IMemento,
IOutputChannel,
ILogOutputChannel,
ITestOutputChannel,
WORKSPACE_MEMENTO,
} from './common/types';
import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry';
Expand All @@ -26,7 +26,6 @@ import { ServiceContainer } from './ioc/container';
import { ServiceManager } from './ioc/serviceManager';
import { IServiceContainer, IServiceManager } from './ioc/types';
import * as pythonEnvironments from './pythonEnvironments';
import { TEST_OUTPUT_CHANNEL } from './testing/constants';
import { IDiscoveryAPI } from './pythonEnvironments/base/locator';
import { registerLogger } from './logging';
import { OutputChannelLogger } from './logging/outputChannelLogger';
Expand Down Expand Up @@ -54,20 +53,20 @@ export function initializeGlobals(
serviceManager.addSingletonInstance<Memento>(IMemento, context.workspaceState, WORKSPACE_MEMENTO);
serviceManager.addSingletonInstance<IExtensionContext>(IExtensionContext, context);

const standardOutputChannel = window.createOutputChannel(OutputChannelNames.python);
const standardOutputChannel = window.createOutputChannel(OutputChannelNames.python, { log: true });
disposables.push(standardOutputChannel);
disposables.push(registerLogger(new OutputChannelLogger(standardOutputChannel)));

const workspaceService = new WorkspaceService();
const unitTestOutChannel =
workspaceService.isVirtualWorkspace || !workspaceService.isTrusted
? // Do not create any test related output UI when using virtual workspaces.
instance(mock<IOutputChannel>())
instance(mock<ITestOutputChannel>())
: window.createOutputChannel(OutputChannelNames.pythonTest);
disposables.push(unitTestOutChannel);

serviceManager.addSingletonInstance<OutputChannel>(IOutputChannel, standardOutputChannel, STANDARD_OUTPUT_CHANNEL);
serviceManager.addSingletonInstance<OutputChannel>(IOutputChannel, unitTestOutChannel, TEST_OUTPUT_CHANNEL);
serviceManager.addSingletonInstance<ILogOutputChannel>(ILogOutputChannel, standardOutputChannel);
serviceManager.addSingletonInstance<ITestOutputChannel>(ITestOutputChannel, unitTestOutChannel);

return {
context,
Expand Down
5 changes: 2 additions & 3 deletions src/client/linters/errorHandlers/standard.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { l10n, Uri } from 'vscode';
import { IApplicationShell } from '../../common/application/types';
import { STANDARD_OUTPUT_CHANNEL } from '../../common/constants';
import { ExecutionInfo, IOutputChannel } from '../../common/types';
import { ExecutionInfo, ILogOutputChannel } from '../../common/types';
import { traceError, traceLog } from '../../logging';
import { ILinterManager, LinterId } from '../types';
import { BaseErrorHandler } from './baseErrorHandler';
Expand Down Expand Up @@ -29,7 +28,7 @@ export class StandardErrorHandler extends BaseErrorHandler {
private async displayLinterError(linterId: LinterId) {
const message = l10n.t("There was an error in running the linter '{0}'", linterId);
const appShell = this.serviceContainer.get<IApplicationShell>(IApplicationShell);
const outputChannel = this.serviceContainer.get<IOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
const outputChannel = this.serviceContainer.get<ILogOutputChannel>(ILogOutputChannel);
const action = await appShell.showErrorMessage(message, 'View Errors');
if (action === 'View Errors') {
outputChannel.show();
Expand Down
17 changes: 6 additions & 11 deletions src/client/logging/outputChannelLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,29 @@
// Licensed under the MIT License.

import * as util from 'util';
import { OutputChannel } from 'vscode';
import { LogOutputChannel } from 'vscode';
import { Arguments, ILogging } from './types';
import { getTimeForLogging } from './util';

function formatMessage(level?: string, ...data: Arguments): string {
return level ? `[${level.toUpperCase()} ${getTimeForLogging()}]: ${util.format(...data)}` : util.format(...data);
}

export class OutputChannelLogger implements ILogging {
constructor(private readonly channel: OutputChannel) {}
constructor(private readonly channel: LogOutputChannel) {}

public traceLog(...data: Arguments): void {
this.channel.appendLine(util.format(...data));
}

public traceError(...data: Arguments): void {
this.channel.appendLine(formatMessage('error', ...data));
this.channel.error(util.format(...data));
}

public traceWarn(...data: Arguments): void {
this.channel.appendLine(formatMessage('warn', ...data));
this.channel.warn(util.format(...data));
}

public traceInfo(...data: Arguments): void {
this.channel.appendLine(formatMessage('info', ...data));
this.channel.info(util.format(...data));
}

public traceVerbose(...data: Arguments): void {
this.channel.appendLine(formatMessage('debug', ...data));
this.channel.debug(util.format(...data));
}
}
4 changes: 0 additions & 4 deletions src/client/testing/constants.ts

This file was deleted.

1 change: 1 addition & 0 deletions src/client/testing/testController/common/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export class PythonTestServer implements ITestServer, Disposable {
token: options.token,
cwd: options.cwd,
throwOnStdErr: true,
outputChannel: options.outChannel,
};

// Create the Python environment in which to execute the command.
Expand Down
Loading

0 comments on commit a1f5041

Please sign in to comment.