Skip to content

Commit

Permalink
Make MPLS and vscode-extension-telemetry work together šŸ¤ (microsoft#1ā€¦
Browse files Browse the repository at this point in the history
ā€¦1823)

* Revert "Revert vscode-extension-telemetry changes for the release (microsoft#11602)"
This reverts commit 71d1793.
* Remove entry from changelog + add new news entry
* Update LS code to use periods instead of slashes
* Revert "Update LS code to use periods instead of slashes"
This reverts commit 1356651.
* Replace slashes before sending telemetry instead
* Too fast too furious
  • Loading branch information
kimadeline authored and karthiknadig committed Jun 5, 2020
1 parent 26d47fc commit cb9d665
Show file tree
Hide file tree
Showing 11 changed files with 154 additions and 189 deletions.
14 changes: 11 additions & 3 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,9 @@ function getAllowedWarningsForWebPack(buildConfig) {
'WARNING in ./node_modules/@jupyterlab/services/node_modules/ws/lib/validation.js',
'WARNING in ./node_modules/any-promise/register.js',
'WARNING in ./node_modules/log4js/lib/appenders/index.js',
'WARNING in ./node_modules/log4js/lib/clustering.js'
'WARNING in ./node_modules/log4js/lib/clustering.js',
'WARNING in ./node_modules/diagnostic-channel-publishers/dist/src/azure-coretracing.pub.js',
'WARNING in ./node_modules/applicationinsights/out/AutoCollection/NativePerformance.js'
];
case 'extension':
return [
Expand All @@ -300,10 +302,16 @@ function getAllowedWarningsForWebPack(buildConfig) {
'remove-files-plugin@1.4.0:',
'WARNING in ./node_modules/@jupyterlab/services/node_modules/ws/lib/buffer-util.js',
'WARNING in ./node_modules/@jupyterlab/services/node_modules/ws/lib/validation.js',
'WARNING in ./node_modules/@jupyterlab/services/node_modules/ws/lib/Validation.js'
'WARNING in ./node_modules/@jupyterlab/services/node_modules/ws/lib/Validation.js',
'WARNING in ./node_modules/diagnostic-channel-publishers/dist/src/azure-coretracing.pub.js',
'WARNING in ./node_modules/applicationinsights/out/AutoCollection/NativePerformance.js'
];
case 'debugAdapter':
return ['WARNING in ./node_modules/vscode-uri/lib/index.js'];
return [
'WARNING in ./node_modules/vscode-uri/lib/index.js',
'WARNING in ./node_modules/diagnostic-channel-publishers/dist/src/azure-coretracing.pub.js',
'WARNING in ./node_modules/applicationinsights/out/AutoCollection/NativePerformance.js'
];
default:
throw new Error('Unknown WebPack Configuration');
}
Expand Down
1 change: 1 addition & 0 deletions news/3 Code Health/11597.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update telemetry on errors and exceptions to use [vscode-extension-telemetry](https://www.npmjs.com/package/vscode-extension-telemetry).
84 changes: 67 additions & 17 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 @@ -2986,7 +2986,7 @@
"untildify": "^3.0.2",
"vscode-debugadapter": "^1.28.0",
"vscode-debugprotocol": "^1.28.0",
"vscode-extension-telemetry": "0.1.0",
"vscode-extension-telemetry": "0.1.4",
"vscode-jsonrpc": "^5.0.1",
"vscode-languageclient": "^6.2.0-next.2",
"vscode-languageserver": "^6.2.0-next.2",
Expand Down
5 changes: 4 additions & 1 deletion src/client/activation/languageClientMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,9 +444,12 @@ function captureTelemetryForLSPMethod(method: string, debounceMilliseconds: numb
this.lastCaptured.set(method, now);
this.eventCount += 1;

// Replace all slashes in the method name so it doesn't get scrubbed by vscode-extension-telemetry.
const formattedMethod = method.replace(/\//g, '.');

const properties = {
lsVersion: this.serverVersion || 'unknown',
method: method
method: formattedMethod
};

const stopWatch = new StopWatch();
Expand Down
7 changes: 6 additions & 1 deletion src/client/activation/languageServer/languageServerProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,12 @@ export class DotNetLanguageServerProxy implements ILanguageServerProxy {
if (settings.downloadLanguageServer) {
this.languageClient.onTelemetry((telemetryEvent) => {
const eventName = telemetryEvent.EventName || EventName.PYTHON_LANGUAGE_SERVER_TELEMETRY;
sendTelemetryEvent(eventName, telemetryEvent.Measurements, telemetryEvent.Properties);
const formattedProperties = {
...telemetryEvent.Properties,
// Replace all slashes in the method name so it doesn't get scrubbed by vscode-extension-telemetry.
method: telemetryEvent.Properties.method?.replace(/\//g, '.')
};
sendTelemetryEvent(eventName, telemetryEvent.Measurements, formattedProperties);
});
}
await this.registerTestServices();
Expand Down
7 changes: 6 additions & 1 deletion src/client/activation/node/languageServerProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,12 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy {
if (settings.downloadLanguageServer) {
this.languageClient.onTelemetry((telemetryEvent) => {
const eventName = telemetryEvent.EventName || EventName.LANGUAGE_SERVER_TELEMETRY;
sendTelemetryEvent(eventName, telemetryEvent.Measurements, telemetryEvent.Properties);
const formattedProperties = {
...telemetryEvent.Properties,
// Replace all slashes in the method name so it doesn't get scrubbed by vscode-extension-telemetry.
method: telemetryEvent.Properties.method?.replace(/\//g, '.')
};
sendTelemetryEvent(eventName, telemetryEvent.Measurements, formattedProperties);
});
}
await this.registerTestServices();
Expand Down
20 changes: 10 additions & 10 deletions src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
// tslint:disable:no-reference no-any import-name no-any function-name
/// <reference path="./vscode-extension-telemetry.d.ts" />

import type { JSONObject } from '@phosphor/coreutils';
import * as stackTrace from 'stack-trace';
import TelemetryReporter from 'vscode-extension-telemetry';
// tslint:disable-next-line: import-name
import TelemetryReporter from 'vscode-extension-telemetry/lib/telemetryReporter';

import { DiagnosticCodes } from '../application/diagnostics/constants';
import { IWorkspaceService } from '../common/application/types';
Expand All @@ -27,10 +27,12 @@ import { TestProvider } from '../testing/common/types';
import { EventName, PlatformErrors } from './constants';
import { LinterTrigger, TestTool } from './types';

// tslint:disable: no-any

/**
* Checks whether telemetry is supported.
* Its possible this function gets called within Debug Adapter, vscode isn't available in there.
* Withiin DA, there's a completely different way to send telemetry.
* Within DA, there's a completely different way to send telemetry.
* @returns {boolean}
*/
function isTelemetrySupported(): boolean {
Expand Down Expand Up @@ -80,14 +82,12 @@ function getTelemetryReporter() {
const extensionId = PVSC_EXTENSION_ID;
// tslint:disable-next-line:no-require-imports
const extensions = (require('vscode') as typeof import('vscode')).extensions;
// tslint:disable-next-line:no-non-null-assertion
const extension = extensions.getExtension(extensionId)!;
// tslint:disable-next-line:no-unsafe-any
const extensionVersion = extension.packageJSON.version;

// tslint:disable-next-line:no-require-imports
const reporter = require('vscode-extension-telemetry').default as typeof TelemetryReporter;
return (telemetryReporter = new reporter(extensionId, extensionVersion, AppinsightsKey));
return (telemetryReporter = new reporter(extensionId, extensionVersion, AppinsightsKey, true));
}

export function clearTelemetryReporter() {
Expand All @@ -108,8 +108,8 @@ export function sendTelemetryEvent<P extends IEventNamePropertyMapping, E extend
let customProperties: Record<string, string> = {};
let eventNameSent = eventName as string;

if (ex && (eventName as any) !== 'ERROR') {
// When sending `ERROR` telemetry event no need to send custom properties.
if (ex) {
// When sending telemetry events for exceptions no need to send custom properties.
// Else we have to review all properties every time as part of GDPR.
// Assume we have 10 events all with their own properties.
// As we have errors for each event, those properties are treated as new data items.
Expand Down Expand Up @@ -147,7 +147,7 @@ export function sendTelemetryEvent<P extends IEventNamePropertyMapping, E extend

if (process.env && process.env.VSC_PYTHON_LOG_TELEMETRY) {
traceInfo(
`Telemetry Event : ${eventName} Measures: ${JSON.stringify(measures)} Props: ${JSON.stringify(
`Telemetry Event : ${eventNameSent} Measures: ${JSON.stringify(measures)} Props: ${JSON.stringify(
customProperties
)} `
);
Expand Down
33 changes: 0 additions & 33 deletions src/client/telemetry/vscode-extension-telemetry.d.ts

This file was deleted.

Loading

0 comments on commit cb9d665

Please sign in to comment.