From b5e66a715f4322a1e422cdc63a45b1b6d6a77d8b Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 9 Mar 2021 08:39:14 -0800 Subject: [PATCH] Update connection event properties (#14608) --- .../telemetry/common/adsTelemetryService.ts | 27 ++++++++++++++----- .../platform/telemetry/common/telemetry.ts | 21 +++++++-------- .../browser/connectionManagementService.ts | 26 +++++++----------- 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/sql/platform/telemetry/common/adsTelemetryService.ts b/src/sql/platform/telemetry/common/adsTelemetryService.ts index 835917230e..ba04537418 100644 --- a/src/sql/platform/telemetry/common/adsTelemetryService.ts +++ b/src/sql/platform/telemetry/common/adsTelemetryService.ts @@ -3,7 +3,8 @@ * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { IAdsTelemetryService, ITelemetryInfo, ITelemetryEvent, ITelemetryConnectionInfo, ITelemetryEventMeasures, ITelemetryEventProperties } from 'sql/platform/telemetry/common/telemetry'; +import * as azdata from 'azdata'; +import { IAdsTelemetryService, ITelemetryInfo, ITelemetryEvent, ITelemetryEventMeasures, ITelemetryEventProperties } from 'sql/platform/telemetry/common/telemetry'; import { ILogService } from 'vs/platform/log/common/log'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { assign } from 'vs/base/common/objects'; @@ -43,13 +44,23 @@ class TelemetryEventImpl implements ITelemetryEvent { return this; } - public withConnectionInfo(connectionInfo: ITelemetryConnectionInfo): ITelemetryEvent { + public withConnectionInfo(connectionInfo?: azdata.IConnectionProfile): ITelemetryEvent { assign(this._properties, { - authenticationType: connectionInfo.authenticationType, - providerName: connectionInfo.providerName, - serverType: connectionInfo.serverType, - engineType: connectionInfo.engineType + authenticationType: connectionInfo?.authenticationType, + providerName: connectionInfo?.providerName + }); + return this; + } + + public withServerInfo(serverInfo?: azdata.ServerInfo): ITelemetryEvent { + assign(this._properties, + { + connectionType: serverInfo?.isCloud !== undefined ? (serverInfo.isCloud ? 'Azure' : 'Standalone') : '', + serverVersion: serverInfo?.serverVersion ?? '', + serverEdition: serverInfo?.serverEdition ?? '', + serverEngineEdition: serverInfo?.engineEditionId ?? '', + isBigDataCluster: serverInfo?.options?.isBigDataCluster ?? false, }); return this; } @@ -64,7 +75,9 @@ class NullTelemetryEventImpl implements ITelemetryEvent { public withAdditionalMeasurements(additionalMeasurements: ITelemetryEventMeasures): ITelemetryEvent { return this; } - public withConnectionInfo(connectionInfo: ITelemetryConnectionInfo): ITelemetryEvent { return this; } + public withConnectionInfo(connectionInfo: azdata.IConnectionProfile): ITelemetryEvent { return this; } + + public withServerInfo(serverInfo: azdata.ServerInfo): ITelemetryEvent { return this; } } export class AdsTelemetryService implements IAdsTelemetryService { diff --git a/src/sql/platform/telemetry/common/telemetry.ts b/src/sql/platform/telemetry/common/telemetry.ts index 5660e1daa8..9e8527be07 100644 --- a/src/sql/platform/telemetry/common/telemetry.ts +++ b/src/sql/platform/telemetry/common/telemetry.ts @@ -3,6 +3,7 @@ * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import * as azdata from 'azdata'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; export const IAdsTelemetryService = createDecorator('adsTelemetryService'); @@ -21,16 +22,6 @@ export interface ITelemetryEventMeasures { [key: string]: number; } -/** - * Connection info properties to add into an event. - */ -export interface ITelemetryConnectionInfo { - authenticationType?: string; - providerName?: string; - serverType?: string; - engineType?: string; -} - export interface ITelemetryEvent { /** * Sends the event @@ -51,9 +42,15 @@ export interface ITelemetryEvent { /** * Adds additional connection-related information to this event. - * @param connectionInfo The connection info to add. Only the fields in TelemetryConnectionInfo are included, all others are ignored. + * @param connectionInfo The connection info to add. */ - withConnectionInfo(connectionInfo: ITelemetryConnectionInfo): ITelemetryEvent; + withConnectionInfo(connectionInfo?: azdata.IConnectionProfile): ITelemetryEvent; + + /** + * Adds additional server-related information to this event. + * @param serverInfo The server info to add. + */ + withServerInfo(serverInfo?: azdata.ServerInfo): ITelemetryEvent; } export interface ITelemetryInfo { diff --git a/src/sql/workbench/services/connection/browser/connectionManagementService.ts b/src/sql/workbench/services/connection/browser/connectionManagementService.ts index 4a0c846dbe..4aa8e23f54 100644 --- a/src/sql/workbench/services/connection/browser/connectionManagementService.ts +++ b/src/sql/workbench/services/connection/browser/connectionManagementService.ts @@ -955,14 +955,9 @@ export class ConnectionManagementService extends Disposable implements IConnecti connection.connectHandler(true); this._telemetryService.createActionEvent(TelemetryKeys.TelemetryView.Shell, TelemetryKeys.DatabaseConnected) - .withAdditionalProperties({ - connectionType: connection.serverInfo ? (connection.serverInfo.isCloud ? 'Azure' : 'Standalone') : '', - provider: connection.connectionProfile.providerName, - serverVersion: connection.serverInfo ? connection.serverInfo.serverVersion : '', - serverEdition: connection.serverInfo ? connection.serverInfo.serverEdition : '', - serverEngineEdition: connection.serverInfo ? connection.serverInfo.engineEditionId : '', - isBigDataCluster: connection.serverInfo?.options?.isBigDataCluster ?? false, - }).withAdditionalMeasurements({ + .withConnectionInfo(connection.connectionProfile) + .withServerInfo(connection.serverInfo) + .withAdditionalMeasurements({ extensionConnectionTimeMs: connection.extensionTimer.elapsed() - connection.serviceTimer.elapsed(), serviceConnectionTimeMs: connection.serviceTimer.elapsed() }) @@ -974,9 +969,7 @@ export class ConnectionManagementService extends Disposable implements IConnecti } else { connection.connectHandler(false, info.errorMessage, info.errorNumber, info.messages); this._telemetryService.createErrorEvent(TelemetryKeys.TelemetryView.Shell, TelemetryKeys.DatabaseConnectionError, info.errorNumber.toString()) - .withAdditionalProperties({ - provider: connection.connectionProfile.providerName, - }) + .withConnectionInfo(connection.connectionProfile) .withAdditionalMeasurements({ extensionConnectionTimeMs: connection.extensionTimer.elapsed() - connection.serviceTimer.elapsed(), serviceConnectionTimeMs: connection.serviceTimer.elapsed() @@ -1131,9 +1124,6 @@ export class ConnectionManagementService extends Disposable implements IConnecti if (this._connectionStatusManager.isDefaultTypeUri(fileUri)) { this._connectionGlobalStatus.setStatusToDisconnected(fileUri); } - - // TODO: send telemetry events - // Telemetry.sendTelemetryEvent('DatabaseDisconnected'); } return result; @@ -1145,19 +1135,21 @@ export class ConnectionManagementService extends Disposable implements IConnecti public disconnect(input: string | interfaces.IConnectionProfile): Promise { let uri: string; let profile: interfaces.IConnectionProfile; + let info: ConnectionManagementInfo | undefined; if (typeof input === 'object') { uri = Utils.generateUri(input); profile = input; + info = this.getConnectionInfo(uri); } else if (typeof input === 'string') { profile = this.getConnectionProfile(input); + info = this.getConnectionInfo(input); uri = input; } return this.doDisconnect(uri, profile).then(result => { if (result) { this._telemetryService.createActionEvent(TelemetryKeys.TelemetryView.Shell, TelemetryKeys.DatabaseDisconnected) - .withAdditionalProperties({ - provider: profile.providerName - }) + .withConnectionInfo(profile) + .withServerInfo(info?.serverInfo) .send(); this._connectionStatusManager.removeConnection(uri); } else {