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

Batch sending telemetry for some of the event notifications. #300

Merged
merged 2 commits into from Mar 7, 2018

Conversation

Projects
None yet
2 participants
@changsi-an
Copy link
Member

changsi-an commented Mar 6, 2018

Batch-sending telemetry is necessary because some notifications from debuggee might be too frequent for the telemetry infrastructure to handle. Take VS as an example, it throttles telemetry events if there are two many within a short period of time. We will lose some important telemetry when that happens.
The BatchTelemetryReporter will cache the received telemetry events and sends them out every 10 seconds.

This PR also refactors some code and migrates some general telemetry handling code from DebugSession.ts to utils.ts or telemetry.ts.

This PR also enables telemetry for debugger.paused and debugger.onScriptParsed notification events.

This PR retargets the change to master branch.

changsi-an added some commits Feb 21, 2018

Introduces a mechanism to batch-sending telemetry events.
# Conflicts:
#	src/chrome/chromeDebugAdapter.ts
#	src/utils.ts

@changsi-an changsi-an requested a review from roblourens Mar 6, 2018

@roblourens roblourens merged commit b806895 into Microsoft:master Mar 7, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
@@ -567,7 +600,7 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter {

this._expectingStopReason = undefined;

smartStepP.then(should => {
await smartStepP.then(should => {

This comment has been minimized.

@changsi-an

changsi-an Mar 7, 2018

Member

I add it to include the time spent in this operation in the overall measurement of onPaused event. Apart from the measurement function, no one seems to actively wait on the promise returned from this method.

@roblourens roblourens added this to the March 2018 milestone Apr 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment