Skip to content

Commit

Permalink
Unified and simplified vCore telemetry (#2476)
Browse files Browse the repository at this point in the history
### PR Merge Message Summary

- **Telemetry Updates**:
  - Renamed telemetry properties: `parentContext` -> `parentNodeContext`.
  - Added `calledFrom` to commands shared between webview and extension.
  - Added telemetry calls for tRPC webview-extension communication (path only, no data).
  - Introduced telemetry events, errors, and RPC call logging.
  - Added support for paging, view changes, query, and refresh events.
  - Extended telemetry to include `documentView` webview and MongoCluster server information.
  - Added telemetry for import and export actions.

- **Code Improvements**:
  - Removed obsolete code and unused files.
  - Removed outdated duration measurement (replaced by `callWithTelemetryAndErrorHandling`).
  - Improved logging: replaced `console.log` with `console.error`.

- **VCore Enhancements**:
  - Exposed `appName` to `azure.com` servers.

- **PR Review Feedback**:
  - Incorporated improvements based on review comments.

This update focuses on enhancing telemetry coverage and cleaning up the codebase for better maintainability and reliability.
  • Loading branch information
tnaum-ms authored Dec 5, 2024
1 parent 9741015 commit db20b2d
Show file tree
Hide file tree
Showing 33 changed files with 628 additions and 240 deletions.
1 change: 1 addition & 0 deletions src/AzureDBExperiences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export enum API {
Core = 'Core', // Now called NoSQL
PostgresSingle = 'PostgresSingle',
PostgresFlexible = 'PostgresFlexible',
Common = 'Common', // In case we're reporting a common event and still need to provide the value of the API
}

export enum DBAccountKind {
Expand Down
29 changes: 23 additions & 6 deletions src/commands/importDocuments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { type ItemDefinition } from '@azure/cosmos';
import { parseError, type IActionContext } from '@microsoft/vscode-azext-utils';
import { callWithTelemetryAndErrorHandling, parseError, type IActionContext } from '@microsoft/vscode-azext-utils';
import { EJSON } from 'bson';
import * as fse from 'fs-extra';
import * as vscode from 'vscode';
Expand Down Expand Up @@ -191,9 +191,17 @@ async function insertDocumentsIntoDocdb(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
async function insertDocumentsIntoMongo(node: MongoCollectionTreeItem, documents: any[]): Promise<string> {
let output = '';
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
const parsed = await node.collection.insertMany(documents);
if (parsed.acknowledged) {

const parsed = await callWithTelemetryAndErrorHandling('cosmosDB.mongo.importDocumets', async (actionContext) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
const parsed = await node.collection.insertMany(documents);

actionContext.telemetry.measurements.documentCount = parsed?.insertedCount;

return parsed;
});

if (parsed?.acknowledged) {
output = `Import into mongo successful. Inserted ${parsed.insertedCount} document(s). See output for more details.`;
for (const inserted of Object.values(parsed.insertedIds)) {
ext.outputChannel.appendLog(`Inserted document: ${inserted}`);
Expand All @@ -207,10 +215,19 @@ async function insertDocumentsIntoMongoCluster(
node: CollectionItem,
documents: unknown[],
): Promise<string> {
const result = await node.insertDocuments(context, documents as Document[]);
const result = await callWithTelemetryAndErrorHandling(
'cosmosDB.mongoClusters.importDocumets',
async (actionContext) => {
const result = await node.insertDocuments(context, documents as Document[]);

actionContext.telemetry.measurements.documentCount = result?.insertedCount;

return result;
},
);

let message: string;
if (result.acknowledged) {
if (result?.acknowledged) {
message = `Import successful. Inserted ${result.insertedCount} document(s).`;
} else {
message = `Import failed. The operation was not acknowledged by the database.`;
Expand Down
2 changes: 1 addition & 1 deletion src/docdb/tree/DocDBAccountTreeItemBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export abstract class DocDBAccountTreeItemBase extends DocDBTreeItemBase<Databas
const result = await callWithTelemetryAndErrorHandling(
'getChildren',
async (context: IActionContext): Promise<AzExtTreeItem[]> => {
context.telemetry.properties.parentContext = this.contextValue;
context.telemetry.properties.parentNodeContext = this.contextValue;

// move this to a shared file, currently it's defined in DocDBAccountTreeItem so I can't reference it here
if (this.contextValue.includes('cosmosDBDocumentServer')) {
Expand Down
2 changes: 1 addition & 1 deletion src/mongo/connectToMongoClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export async function connectToMongoClient(connectionString: string, appName: st
// appname appears to be the correct equivalent to user-agent for mongo
const options: MongoClientOptions = <MongoClientOptions>{
// appName should be wrapped in '@'s when trying to connect to a Mongo account, this doesn't effect the appendUserAgent string
appName: `@${appName}@`,
appName: `${appName}[RU]`,
// https://github.com/lmammino/mongo-uri-builder/issues/2
useNewUrlParser: true,
useUnifiedTopology: true,
Expand Down
2 changes: 1 addition & 1 deletion src/mongo/tree/MongoAccountTreeItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export class MongoAccountTreeItem extends AzExtParentTreeItem {
'getChildren',
async (context: IActionContext): Promise<AzExtTreeItem[]> => {
context.telemetry.properties.experience = API.MongoDB;
context.telemetry.properties.parentContext = this.contextValue;
context.telemetry.properties.parentNodeContext = this.contextValue;

let mongoClient: MongoClient | undefined;
try {
Expand Down
28 changes: 24 additions & 4 deletions src/mongoClusters/MongoClustersClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* singletone on a client with a getter from a connection pool..
*/

import { appendExtensionUserAgent, callWithTelemetryAndErrorHandling } from '@microsoft/vscode-azext-utils';
import { EJSON } from 'bson';
import {
MongoClient,
Expand All @@ -22,6 +23,8 @@ import {
type WithoutId,
} from 'mongodb';
import { CredentialCache } from './CredentialCache';
import { areMongoDBAzure, getHostsFromConnectionString } from './utils/connectionStringHelpers';
import { getMongoClusterMetadata, type MongoClusterMetadata } from './utils/getMongoClusterMetadata';
import { toFilterQueryObj } from './utils/toFilterQuery';

export interface DatabaseItemModel {
Expand Down Expand Up @@ -73,9 +76,26 @@ export class MongoClustersClient {
}

this._credentialId = credentialId;

// check if it's an azure connection, and do some special handling
const cString = CredentialCache.getCredentials(credentialId)?.connectionString as string;
const hosts = getHostsFromConnectionString(cString);
const userAgentString = areMongoDBAzure(hosts) ? appendExtensionUserAgent() : undefined;

const cStringPassword = CredentialCache.getConnectionStringWithPassword(credentialId);

this._mongoClient = await MongoClient.connect(cStringPassword as string);
this._mongoClient = await MongoClient.connect(cStringPassword as string, {
appName: userAgentString,
});

void callWithTelemetryAndErrorHandling('cosmosDB.mongoClusters.connect.getmetadata', async (context) => {
const metadata: MongoClusterMetadata = await getMongoClusterMetadata(this._mongoClient);

context.telemetry.properties = {
...context.telemetry.properties,
...metadata,
};
});
}

public static async getClient(credentialId: string): Promise<MongoClustersClient> {
Expand Down Expand Up @@ -197,7 +217,7 @@ export class MongoClustersClient {
try {
while (await cursor.hasNext()) {
if (abortSignal.aborted) {
console.log('streamDocuments: Aborted by an abort signal.');
console.debug('streamDocuments: Aborted by an abort signal.');
return;
}

Expand Down Expand Up @@ -324,7 +344,7 @@ export class MongoClustersClient {
try {
newCollection = await this._mongoClient.db(databaseName).createCollection(collectionName);
} catch (_e) {
console.log(_e); //todo: add to telemetry
console.error(_e); //todo: add to telemetry
return false;
}

Expand All @@ -338,7 +358,7 @@ export class MongoClustersClient {
.createCollection('_dummy_collection_creation_forces_db_creation');
await newCollection.drop();
} catch (_e) {
console.log(_e); //todo: add to telemetry
console.error(_e); //todo: add to telemetry
return false;
}

Expand Down
15 changes: 12 additions & 3 deletions src/mongoClusters/MongoClustersExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,6 @@ export class MongoClustersExtension implements vscode.Disposable {
registerCommand('command.internal.mongoClusters.containerView.open', openCollectionView);
registerCommand('command.internal.mongoClusters.documentView.open', openDocumentView);

registerCommand('command.internal.mongoClusters.importDocuments', mongoClustersImportDocuments);
registerCommand('command.internal.mongoClusters.exportDocuments', mongoClustersExportQueryResults);

registerCommandWithTreeNodeUnwrapping('command.mongoClusters.launchShell', launchShell);

registerCommandWithTreeNodeUnwrapping('command.mongoClusters.dropCollection', dropCollection);
Expand All @@ -101,6 +98,18 @@ export class MongoClustersExtension implements vscode.Disposable {
'command.mongoClusters.importDocuments',
mongoClustersImportDocuments,
);

/**
* Here, exporting documents is done in two ways: one is accessible from the tree view
* via a context menu, and the other is accessible programmatically. Both of them
* use the same underlying function to export documents.
*
* mongoClustersExportEntireCollection calls mongoClustersExportQueryResults with no queryText.
*
* It was possible to merge the two commands into one, but it would result in code that is
* harder to understand and maintain.
*/
registerCommand('command.internal.mongoClusters.exportDocuments', mongoClustersExportQueryResults);
registerCommandWithTreeNodeUnwrapping(
'command.mongoClusters.exportDocuments',
mongoClustersExportEntireCollection,
Expand Down
21 changes: 3 additions & 18 deletions src/mongoClusters/commands/addWorkspaceConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { WorkspaceResourceType } from '../../tree/workspace/sharedWorkspaceResou
import { SharedWorkspaceStorage } from '../../tree/workspace/sharedWorkspaceStorage';
import { showConfirmationAsInSettings } from '../../utils/dialogs/showConfirmation';
import { localize } from '../../utils/localize';
import { areMongoDBRU } from '../utils/connectionStringHelpers';
import { type AddWorkspaceConnectionContext } from '../wizards/addWorkspaceConnection/AddWorkspaceConnectionContext';
import { ConnectionStringStep } from '../wizards/addWorkspaceConnection/ConnectionStringStep';
import { PasswordStep } from '../wizards/addWorkspaceConnection/PasswordStep';
Expand Down Expand Up @@ -55,12 +56,8 @@ export async function addWorkspaceConnection(context: IActionContext): Promise<v
wizardContext.valuesToMask.push(connectionStringWithCredentials);

// discover whether it's a MongoDB RU connection string and abort here.
let isRU: boolean = false;
connectionString.hosts.forEach((host) => {
if (isMongoDBRU(host)) {
isRU = true;
}
});
const isRU = areMongoDBRU(connectionString.hosts);

if (isRU) {
try {
await vscode.window.showInformationMessage(
Expand Down Expand Up @@ -104,15 +101,3 @@ export async function addWorkspaceConnection(context: IActionContext): Promise<v
localize('showConfirmation.addedWorkspaceConnecdtion', 'New connection has been added to your workspace.'),
);
}

function isMongoDBRU(host: string): boolean {
const knownSuffixes = ['mongo.cosmos.azure.com'];
const hostWithoutPort = host.split(':')[0];

for (const suffix of knownSuffixes) {
if (hostWithoutPort.toLowerCase().endsWith(suffix)) {
return true;
}
}
return false;
}
38 changes: 23 additions & 15 deletions src/mongoClusters/commands/exportDocuments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { type IActionContext } from '@microsoft/vscode-azext-utils';
import { callWithTelemetryAndErrorHandling, type IActionContext } from '@microsoft/vscode-azext-utils';
import { EJSON } from 'bson';
import * as vscode from 'vscode';
import { ext } from '../../extensionVariables';
Expand All @@ -12,21 +12,23 @@ import { getRootPath } from '../../utils/workspacUtils';
import { MongoClustersClient } from '../MongoClustersClient';
import { type CollectionItem } from '../tree/CollectionItem';

export async function mongoClustersExportEntireCollection(_context: IActionContext, node?: CollectionItem) {
return mongoClustersExportQueryResults(_context, node);
export async function mongoClustersExportEntireCollection(context: IActionContext, node?: CollectionItem) {
return mongoClustersExportQueryResults(context, node);
}

export async function mongoClustersExportQueryResults(
_context: IActionContext,
context: IActionContext,
node?: CollectionItem,
queryText?: string,
props?: { queryText?: string; source?: string },
): Promise<void> {
// node ??= ... pick a node if not provided
if (!node) {
throw new Error('No collection selected.');
}

const targetUri = await askForTargetFile(_context);
context.telemetry.properties.calledFrom = props?.source || 'contextMenu';

const targetUri = await askForTargetFile(context);

if (!targetUri) {
return;
Expand All @@ -39,7 +41,7 @@ export async function mongoClustersExportQueryResults(
node.databaseInfo.name,
node.collectionInfo.name,
docStreamAbortController.signal,
queryText,
props?.queryText,
);

const filePath = targetUri.fsPath; // Convert `vscode.Uri` to a regular file path
Expand All @@ -48,14 +50,20 @@ export async function mongoClustersExportQueryResults(
let documentCount = 0;

// Wrap the export process inside a progress reporting function
await runExportWithProgressAndDescription(node.id, async (progress, cancellationToken) => {
documentCount = await exportDocumentsToFile(
docStream,
filePath,
progress,
cancellationToken,
docStreamAbortController,
);
await callWithTelemetryAndErrorHandling('cosmosDB.mongoClusters.exportDocuments', async (actionContext) => {
await runExportWithProgressAndDescription(node.id, async (progress, cancellationToken) => {
documentCount = await exportDocumentsToFile(
docStream,
filePath,
progress,
cancellationToken,
docStreamAbortController,
);
});

actionContext.telemetry.properties.source = props?.source;
actionContext.telemetry.measurements.queryLength = props?.queryText?.length;
actionContext.telemetry.measurements.documentCount = documentCount;
});

ext.outputChannel.appendLog(`MongoDB Clusters: Exported document count: ${documentCount}`);
Expand Down
5 changes: 5 additions & 0 deletions src/mongoClusters/commands/importDocuments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import { type CollectionItem } from '../tree/CollectionItem';
export async function mongoClustersImportDocuments(
context: IActionContext,
collectionNode?: CollectionItem,
_collectionNodes?: CollectionItem[], // required by the TreeNodeCommandCallback, but not used
...args: unknown[]
): Promise<void> {
const source = (args[0] as { source?: string })?.source || 'contextMenu';
context.telemetry.properties.calledFrom = source;

return importDocuments(context, undefined, collectionNode);
}
2 changes: 1 addition & 1 deletion src/mongoClusters/tree/MongoClusterResourceItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class MongoClusterResourceItem extends MongoClusterItemBase {
*/
protected async authenticateAndConnect(): Promise<MongoClustersClient | null> {
const result = await callWithTelemetryAndErrorHandling(
'cosmosDB.mongoClusters.authenticate',
'cosmosDB.mongoClusters.connect',
async (context: IActionContext) => {
ext.outputChannel.appendLine(
`MongoDB Clusters: Attempting to authenticate with "${this.mongoCluster.name}"...`,
Expand Down
4 changes: 2 additions & 2 deletions src/mongoClusters/tree/MongoClustersBranchDataProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class MongoClustersBranchDataProvider
*/
return await callWithTelemetryAndErrorHandling('getChildren', async (context: IActionContext) => {
context.telemetry.properties.experience = API.MongoClusters;
context.telemetry.properties.parentContext = (await element.getTreeItem()).contextValue ?? 'unknown';
context.telemetry.properties.parentNodeContext = (await element.getTreeItem()).contextValue || 'unknown';

return (await element.getChildren?.())?.map((child) => {
if (child.id) {
Expand Down Expand Up @@ -174,7 +174,7 @@ export class MongoClustersBranchDataProvider
});
});
} catch (e) {
console.error({ ...context, ...subscription });
console.debug({ ...context, ...subscription });
throw e;
}
},
Expand Down
6 changes: 3 additions & 3 deletions src/mongoClusters/tree/workspace/MongoClusterWorkspaceItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class MongoClusterWorkspaceItem extends MongoClusterItemBase {
*/
protected async authenticateAndConnect(): Promise<MongoClustersClient | null> {
const result = await callWithTelemetryAndErrorHandling(
'cosmosDB.mongoClusters.authenticate',
'cosmosDB.mongoClusters.connect',
async (context: IActionContext) => {
context.telemetry.properties.view = 'workspace';

Expand Down Expand Up @@ -93,7 +93,7 @@ export class MongoClusterWorkspaceItem extends MongoClusterItemBase {
throw error;
});
} catch (error) {
console.log(error);
console.error(error);
// If connection fails, remove cached credentials
await MongoClustersClient.deleteClient(this.id);
CredentialCache.deleteCredentials(this.id);
Expand Down Expand Up @@ -126,7 +126,7 @@ export class MongoClusterWorkspaceItem extends MongoClusterItemBase {

// Prompt the user for credentials
await callWithTelemetryAndErrorHandling(
'cosmosDB.mongoClusters.authenticate.promptForCredentials',
'cosmosDB.mongoClusters.connect.promptForCredentials',
async (context: IActionContext) => {
context.telemetry.properties.view = 'workspace';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class MongoClustersWorkspaceBranchDataProvider
return await callWithTelemetryAndErrorHandling('getChildren', async (context: IActionContext) => {
context.telemetry.properties.experience = API.MongoClusters;
context.telemetry.properties.view = 'workspace';
context.telemetry.properties.parentContext = (await element.getTreeItem()).contextValue ?? 'unknown';
context.telemetry.properties.parentNodeContext = (await element.getTreeItem()).contextValue ?? 'unknown';

return (await element.getChildren?.())?.map((child) => {
if (child.id) {
Expand Down
Loading

0 comments on commit db20b2d

Please sign in to comment.