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

chore: add language server logs VSCODE-447, VSCODE-448, VSCODE-449 #563

Merged
merged 18 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@
"micromatch": "^4.0.5",
"mongodb": "^5.6.0",
"mongodb-build-info": "^1.5.0",
"mongodb-cloud-info": "^2.0.1",
"mongodb-cloud-info": "^2.1.0",
"mongodb-connection-string-url": "^2.6.0",
"mongodb-data-service": "^22.8.0",
"mongodb-query-parser": "^2.5.0",
Expand Down
12 changes: 10 additions & 2 deletions src/connectionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export default class ConnectionController {
} = Object.create(null);
_activeDataService: DataService | null = null;
_storageController: StorageController;
_telemetryService: TelemetryService;

private readonly _serviceName = 'mdb.vscode.savedConnections';
private _currentConnectionId: null | string = null;
Expand All @@ -125,7 +126,6 @@ export default class ConnectionController {
private _disconnecting = false;

private _statusView: StatusView;
private _telemetryService: TelemetryService;

// Used by other parts of the extension that respond to changes in the connections.
private eventEmitter: EventEmitter = new EventEmitter();
Expand Down Expand Up @@ -509,10 +509,18 @@ export default class ConnectionController {
this.eventEmitter.emit(DataServiceEventTypes.CONNECTIONS_DID_CHANGE);

if (this._activeDataService) {
log.info('Disconnecting from the previous connection...', {
connectionId: this._currentConnectionId,
});
await this.disconnect();
}

this._statusView.showMessage('Connecting to MongoDB...');
log.info('Connecting to MongoDB...', {
connectionInfo: JSON.stringify(
extractSecrets(this._connections[connectionId]).connectionInfo
),
});

const connectionOptions = this._connections[connectionId].connectionOptions;

Expand Down Expand Up @@ -551,7 +559,7 @@ export default class ConnectionController {
throw connectError;
}

log.info('Successfully connected');
log.info('Successfully connected', { connectionId });
Copy link
Member

Choose a reason for hiding this comment

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

Adding the connection name as well might help debugging as it can be hard to infer which connection the connectionId belongs to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't pass the connection name anywhere outside the connection controller, not sure we can use it to match connectivity issues here and for example in LS. But I added a name to the explorer logs, so we can find the info what was expanded:
https://github.com/mongodb-js/vscode/pull/563/files#diff-06a415e2a54193ec0955688552bf1cb4813ac5868fc9392083f3c37f8f65a8c4R67-R69

void vscode.window.showInformationMessage('MongoDB connection successful.');

this._activeDataService = dataService;
Expand Down
87 changes: 36 additions & 51 deletions src/editors/playgroundController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export default class PlaygroundController {
this._connectionController.addEventListener(
DataServiceEventTypes.ACTIVE_CONNECTION_CHANGED,
() => {
void this._connectToServiceProvider();
void this._activeConnectionChanged();
}
);

Expand All @@ -163,22 +163,26 @@ export default class PlaygroundController {
this._playgroundResultViewColumn = editor.viewColumn;
this._playgroundResultTextDocument = editor?.document;
}
const isPlaygroundEditor = isPlayground(editor?.document.uri);

void vscode.commands.executeCommand(
'setContext',
'mdb.isPlayground',
isPlayground(editor?.document.uri)
isPlaygroundEditor
);

if (editor?.document.languageId !== 'Log') {
if (isPlaygroundEditor) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be that vscode chnages log names and we set logs as active playground editors.

this._activeTextEditor = editor;
this._activeConnectionCodeLensProvider.setActiveTextEditor(
this._activeTextEditor
);
this._playgroundSelectedCodeActionProvider.setActiveTextEditor(
this._activeTextEditor
);
log.info('Active editor path', editor?.document.uri?.path);
log.info('Active editor', {
documentPath: editor?.document.uri?.path,
documentLanguageId: editor?.document.languageId,
});
}
};

Expand Down Expand Up @@ -245,35 +249,24 @@ export default class PlaygroundController {
);
}

async _connectToServiceProvider(): Promise<void> {
// Disconnect if already connected.
await this._languageServerController.disconnectFromServiceProvider();

async _activeConnectionChanged(): Promise<void> {
const dataService = this._connectionController.getActiveDataService();
const connectionId = this._connectionController.getActiveConnectionId();
let mongoClientOption;

if (!dataService || !connectionId) {
this._activeConnectionCodeLensProvider.refresh();

return;
}

const mongoClientOption =
this._connectionController.getMongoClientConnectionOptions();

if (!mongoClientOption) {
this._activeConnectionCodeLensProvider.refresh();
this._activeConnectionCodeLensProvider.refresh();

return;
if (dataService && connectionId) {
mongoClientOption =
this._connectionController.getMongoClientConnectionOptions();
}

await this._languageServerController.connectToServiceProvider({
// The connectionId is null when disconnecting.
await this._languageServerController.activeConnectionChanged({
connectionId,
connectionString: mongoClientOption.url,
connectionOptions: mongoClientOption.options,
connectionString: mongoClientOption?.url,
connectionOptions: mongoClientOption?.options,
});

this._activeConnectionCodeLensProvider.refresh();
}

async _createPlaygroundFileWithContent(
Expand Down Expand Up @@ -420,36 +413,28 @@ export default class PlaygroundController {

this._statusView.showMessage('Getting results...');

let result: ShellEvaluateResult;
try {
// Send a request to the language server to execute scripts from a playground.
const result: ShellEvaluateResult =
await this._languageServerController.evaluate({
codeToEvaluate,
connectionId,
});

this._statusView.hideMessage();
this._telemetryService.trackPlaygroundCodeExecuted(
result,
this._isPartialRun,
result ? false : true
);

return result;
} catch (err: any) {
// We re-initialize the language server when we encounter an error.
// This happens when the language server worker runs out of memory, can't be revitalized, and restarts.
if (err?.code === -32097) {
Anemy marked this conversation as resolved.
Show resolved Hide resolved
void vscode.window.showErrorMessage(
'An error occurred when running the playground. This can occur when the playground runner runs out of memory.'
);
result = await this._languageServerController.evaluate({
codeToEvaluate,
connectionId,
});
} catch (error) {
const msg =
'An internal error has occurred. The playground services have been restored. This can occur when the playground runner runs out of memory.';
log.error(msg, error);
void vscode.window.showErrorMessage(msg);
}

await this._languageServerController.startLanguageServer();
void this._connectToServiceProvider();
}
this._statusView.hideMessage();
this._telemetryService.trackPlaygroundCodeExecuted(
result,
this._isPartialRun,
result ? false : true
);

throw err;
}
return result;
}

_getAllText(): string {
Expand Down
6 changes: 0 additions & 6 deletions src/explorer/explorerController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ import ConnectionController, {
} from '../connectionController';
import ExplorerTreeController from './explorerTreeController';

import { createLogger } from '../logging';

const log = createLogger('explorer controller');

export default class ExplorerController {
private _connectionController: ConnectionController;
private _treeController: ExplorerTreeController;
Expand Down Expand Up @@ -38,14 +34,12 @@ export default class ExplorerController {
};

activateConnectionsTreeView(): void {
log.info('Activating explorer controller...');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Such logs turned out to be distracting and not helpful for debugging, so removing them.

// Listen for a change in connections to occur before we create the tree
// so that we show the `viewsWelcome` before any connections are added.
this._connectionController.addEventListener(
DataServiceEventTypes.CONNECTIONS_DID_CHANGE,
this.createTreeView
);
log.info('Explorer controller activated');
}

deactivate(): void {
Expand Down
6 changes: 5 additions & 1 deletion src/explorer/explorerTreeController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@ export default class ExplorerTreeController
});

treeView.onDidExpandElement(async (event: any): Promise<void> => {
log.info('Tree item was expanded', event.element.label);
log.info('Connection tree item was expanded', {
connectionId: event.element.connectionId,
connectionName: event.element.label,
isExpanded: event.element.isExpanded,
});

if (!event.element.onDidExpand) {
return;
Expand Down
5 changes: 0 additions & 5 deletions src/explorer/helpExplorer.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import * as vscode from 'vscode';
import HelpTree from './helpTree';
import { createLogger } from '../logging';
import { TelemetryService } from '../telemetry';

const log = createLogger('help and info explorer');

export default class HelpExplorer {
_treeController: HelpTree;
_treeView?: vscode.TreeView<vscode.TreeItem>;
Expand All @@ -15,15 +12,13 @@ export default class HelpExplorer {

activateHelpTreeView(telemetryService: TelemetryService): void {
if (!this._treeView) {
log.info('Activating help explorer...');
this._treeView = vscode.window.createTreeView('mongoDBHelpExplorer', {
treeDataProvider: this._treeController,
});
this._treeController.activateTreeViewEventHandlers(
this._treeView,
telemetryService
);
log.info('Help explorer activated');
}
}

Expand Down
5 changes: 0 additions & 5 deletions src/explorer/playgroundsExplorer.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import * as vscode from 'vscode';
import PlaygroundsTree from './playgroundsTree';
import { createLogger } from '../logging';

const log = createLogger('playgrounds explorer');

export default class PlaygroundsExplorer {
private _treeController: PlaygroundsTree;
Expand All @@ -25,9 +22,7 @@ export default class PlaygroundsExplorer {
};

public activatePlaygroundsTreeView(): void {
log.info('Activating playgrounds explorer...');
this.createPlaygroundsTreeView();
log.info('Playgrounds explorer activated');
}

public deactivate(): void {
Expand Down
4 changes: 3 additions & 1 deletion src/explorer/playgroundsTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ export default class PlaygroundsTree
});

treeView.onDidExpandElement(async (event: any): Promise<void> => {
log.info('Tree item was expanded', event.element.label);
log.info('Playground tree item was expanded', {
playgroundName: event.element.label,
});

if (!event.element.onDidExpand) {
return;
Expand Down
30 changes: 27 additions & 3 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import * as vscode from 'vscode';
import { ext } from './extensionConstants';
import { createKeytar } from './utils/keytar';
import { createLogger } from './logging';
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { version } = require('../package.json');

const log = createLogger('extension');

Expand All @@ -27,28 +29,50 @@ let mdbExtension: MDBExtensionController;
export async function activate(
context: vscode.ExtensionContext
): Promise<void> {
log.info('Activating extension...');
ext.context = context;
let hasKeytar = false;

try {
ext.keytarModule = createKeytar();
hasKeytar = true;
} catch (err) {
// Couldn't load keytar, proceed without storing & loading connections.
}

const defaultConnectionSavingLocation = vscode.workspace
.getConfiguration('mdb.connectionSaving')
.get('defaultConnectionSavingLocation');

log.info('Activating extension...', {
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking a test for this log might be a very useful one for debugging the environment. Not needed for this pr if we don't want to put the time in now, just a thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would you test here? Do we have tests for logs already somewhere in compass?

id: context.extension.id,
version: version,
mode: vscode.ExtensionMode[context.extensionMode],
kind: vscode.ExtensionKind[context.extension.extensionKind],
extensionPath: context.extensionPath,
logPath: context.logUri.path,
workspaceStoragePath: context.storageUri?.path,
globalStoragePath: context.globalStorageUri.path,
defaultConnectionSavingLocation,
hasKeytar,
buildInfo: {
nodeVersion: process.version,
runtimePlatform: process.platform,
runtimeArch: process.arch,
},
});
alenakhineika marked this conversation as resolved.
Show resolved Hide resolved

mdbExtension = new MDBExtensionController(context, {
shouldTrackTelemetry: true,
});
await mdbExtension.activate();

// Add our extension to a list of disposables for when we are deactivated.
context.subscriptions.push(mdbExtension);

log.info('Extension activated');
}

// Called when our extension is deactivated.
export async function deactivate(): Promise<void> {
log.info('Deactivating extension...');
if (mdbExtension) {
await mdbExtension.deactivate();
}
Expand Down
Loading