Skip to content

Commit 2c20449

Browse files
Achal1607Achal1607
committed
Improved retry mechanism, closeEvent handling and fixed some bugs
1 parent efab04d commit 2c20449

File tree

8 files changed

+94
-90
lines changed

8 files changed

+94
-90
lines changed

‎vscode/src/telemetry/constants.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
export const TELEMETRY_CONSENT_VERSION_SCHEMA_KEY = "telemetryConsentSchemaVersion";
2-
export const TELEMETRY_CONSENT_POPUP_TIME_KEY = "telemetryConsentPopupTime";
2+
export const TELEMETRY_CONSENT_RESPONSE_TIME_KEY = "telemetryConsentResponseTime";
33
export const TELEMETRY_SETTING_VALUE_KEY = "telemetrySettingValue";

‎vscode/src/telemetry/events/baseEvent.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export abstract class BaseEvent<T> {
4242
get getPayload(): T & BaseEventPayload {
4343
return this._payload;
4444
}
45-
45+
4646
get getData(): T {
4747
return this._data;
4848
}
@@ -58,8 +58,8 @@ export abstract class BaseEvent<T> {
5858
protected addEventToCache = (): void => {
5959
const dataString = JSON.stringify(this.getData);
6060
const calculatedHashVal = getHashCode(dataString);
61-
const isAdded = cacheService.put(this.NAME, calculatedHashVal);
62-
63-
LOGGER.debug(`${this.NAME} added in cache ${isAdded ? "Successfully" : "Unsucessfully"}`);
61+
cacheService.put(this.NAME, calculatedHashVal).then((isAdded: boolean) => {
62+
LOGGER.debug(`${this.NAME} added in cache ${isAdded ? "Successfully" : "Unsucessfully"}`);
63+
});
6464
}
6565
}

‎vscode/src/telemetry/impl/cacheServiceImpl.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ class CacheServiceImpl implements CacheService {
2828
}
2929
}
3030

31-
public put = (key: string, value: string): boolean => {
31+
public put = async (key: string, value: string): Promise<boolean> => {
3232
try {
3333
const vscGlobalState = globalState.getExtensionContextInfo().getVscGlobalState();
34-
vscGlobalState.update(key, value);
34+
await vscGlobalState.update(key, value);
3535
LOGGER.debug(`Updating key: ${key} to ${value}`);
3636
return true;
3737
} catch (err) {

‎vscode/src/telemetry/impl/telemetryEventQueue.ts‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
16+
import { LOGGER } from "../../logger";
1617
import { BaseEvent } from "../events/baseEvent";
1718

1819
export class TelemetryEventQueue {
@@ -36,18 +37,25 @@ export class TelemetryEventQueue {
3637
return queue;
3738
}
3839

39-
public decreaseSizeOnMaxOverflow = () => {
40-
const seen = new Set<string>();
41-
const newQueueStart = Math.floor(this.size() / 2);
42-
43-
const secondHalf = this.events.slice(newQueueStart);
44-
45-
const uniqueEvents = secondHalf.filter(event => {
46-
if (seen.has(event.NAME)) return false;
47-
seen.add(event.NAME);
48-
return true;
49-
});
50-
51-
this.events = [...uniqueEvents, ...secondHalf];
40+
public decreaseSizeOnMaxOverflow = (maxNumberOfEventsToBeKept: number) => {
41+
const excess = this.size() - maxNumberOfEventsToBeKept;
42+
43+
if (excess > 0) {
44+
LOGGER.debug('Decreasing size of the queue as max capacity reached');
45+
46+
const seen = new Set<string>();
47+
const deduplicated = [];
48+
49+
for (let i = 0; i < excess; i++) {
50+
const event = this.events[i];
51+
if (!seen.has(event.NAME)) {
52+
deduplicated.push(event);
53+
seen.add(event.NAME);
54+
}
55+
}
56+
57+
this.events = [...deduplicated, ...this.events.slice(excess)];
58+
}
5259
}
60+
5361
}

‎vscode/src/telemetry/impl/telemetryPrefs.ts‎

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { appendPrefixToCommand } from "../../utils";
2020
import { ExtensionContextInfo } from "../../extensionContextInfo";
2121
import { TelemetryPreference } from "../types";
2222
import { cacheService } from "./cacheServiceImpl";
23-
import { TELEMETRY_CONSENT_POPUP_TIME_KEY, TELEMETRY_CONSENT_VERSION_SCHEMA_KEY, TELEMETRY_SETTING_VALUE_KEY } from "../constants";
23+
import { TELEMETRY_CONSENT_RESPONSE_TIME_KEY, TELEMETRY_CONSENT_VERSION_SCHEMA_KEY, TELEMETRY_SETTING_VALUE_KEY } from "../constants";
2424
import { TelemetryConfiguration } from "../config";
2525
import { LOGGER } from "../../logger";
2626

@@ -37,17 +37,11 @@ export class TelemetrySettings {
3737

3838
this.extensionPrefs = new ExtensionTelemetryPreference();
3939
this.vscodePrefs = new VscodeTelemetryPreference();
40-
41-
extensionContext.pushSubscription(
42-
this.extensionPrefs.onChangeTelemetrySetting(this.onChangeTelemetrySettingCallback)
43-
);
44-
extensionContext.pushSubscription(
45-
this.vscodePrefs.onChangeTelemetrySetting(this.onChangeTelemetrySettingCallback)
46-
);
47-
40+
extensionContext.pushSubscription(this.extensionPrefs.onChangeTelemetrySetting(this.onChangeTelemetrySettingCallback));
41+
extensionContext.pushSubscription(this.vscodePrefs.onChangeTelemetrySetting(this.onChangeTelemetrySettingCallback));
42+
4843
this.isTelemetryEnabled = this.checkTelemetryStatus();
49-
this.updateGlobalState();
50-
this.checkConsentVersion();
44+
this.syncTelemetrySettingGlobalState();
5145
}
5246

5347
private checkTelemetryStatus = (): boolean => this.extensionPrefs.getIsTelemetryEnabled() && this.vscodePrefs.getIsTelemetryEnabled();
@@ -56,14 +50,16 @@ export class TelemetrySettings {
5650
const newTelemetryStatus = this.checkTelemetryStatus();
5751
if (newTelemetryStatus !== this.isTelemetryEnabled) {
5852
this.isTelemetryEnabled = newTelemetryStatus;
59-
cacheService.put(TELEMETRY_SETTING_VALUE_KEY, newTelemetryStatus.toString());
53+
this.updateGlobalStates();
6054

6155
if (newTelemetryStatus) {
6256
this.onTelemetryEnableCallback();
6357
} else {
6458
this.onTelemetryDisableCallback();
6559
}
66-
} else if (this.vscodePrefs.getIsTelemetryEnabled() && !this.extensionPrefs.isTelemetrySettingSet()) {
60+
} else if (this.vscodePrefs.getIsTelemetryEnabled()
61+
&& !this.extensionPrefs.isTelemetrySettingSet()
62+
&& !cacheService.get(TELEMETRY_CONSENT_RESPONSE_TIME_KEY)) {
6763
this.triggerPopup();
6864
}
6965
}
@@ -74,38 +70,38 @@ export class TelemetrySettings {
7470
const isExtensionSettingSet = this.extensionPrefs.isTelemetrySettingSet();
7571
const isVscodeSettingEnabled = this.vscodePrefs.getIsTelemetryEnabled();
7672

77-
const showPopup = !isExtensionSettingSet && isVscodeSettingEnabled;
78-
79-
if (showPopup) {
80-
cacheService.put(TELEMETRY_CONSENT_POPUP_TIME_KEY, Date.now().toString());
81-
}
82-
83-
return showPopup;
73+
return !isExtensionSettingSet && isVscodeSettingEnabled;
8474
}
8575

8676
public updateTelemetrySetting = (value: boolean | undefined): void => {
8777
this.extensionPrefs.updateTelemetryConfig(value);
8878
}
8979

90-
private updateGlobalState(): void {
91-
const cachedValue = cacheService.get(TELEMETRY_SETTING_VALUE_KEY);
80+
private syncTelemetrySettingGlobalState (): void {
81+
const cachedSettingValue = cacheService.get(TELEMETRY_SETTING_VALUE_KEY);
82+
const cachedConsentSchemaVersion = cacheService.get(TELEMETRY_CONSENT_VERSION_SCHEMA_KEY);
9283

93-
if (this.isTelemetryEnabled.toString() !== cachedValue) {
94-
cacheService.put(TELEMETRY_SETTING_VALUE_KEY, this.isTelemetryEnabled.toString());
84+
if (this.isTelemetryEnabled.toString() !== cachedSettingValue) {
85+
this.updateGlobalStates();
9586
}
87+
this.checkConsentVersionSchemaGlobalState(cachedConsentSchemaVersion);
9688
}
9789

98-
private checkConsentVersion(): void {
99-
const cachedVersion = cacheService.get(TELEMETRY_CONSENT_VERSION_SCHEMA_KEY);
100-
const currentVersion = TelemetryConfiguration.getInstance().getTelemetryConfigMetadata()?.consentSchemaVersion;
90+
private updateGlobalStates(): void {
91+
cacheService.put(TELEMETRY_CONSENT_RESPONSE_TIME_KEY, Date.now().toString());
92+
cacheService.put(TELEMETRY_CONSENT_VERSION_SCHEMA_KEY, TelemetryConfiguration.getInstance().getTelemetryConfigMetadata()?.consentSchemaVersion);
93+
cacheService.put(TELEMETRY_SETTING_VALUE_KEY, this.isTelemetryEnabled.toString());
94+
}
10195

102-
if (cachedVersion !== currentVersion) {
103-
cacheService.put(TELEMETRY_CONSENT_VERSION_SCHEMA_KEY, currentVersion);
104-
LOGGER.debug("Removing telemetry config from user settings");
105-
if (this.extensionPrefs.isTelemetrySettingSet()) {
96+
private checkConsentVersionSchemaGlobalState(consentSchemaVersion: string | undefined): void {
97+
if (this.extensionPrefs.isTelemetrySettingSet()) {
98+
const currentExtConsentSchemaVersion = TelemetryConfiguration.getInstance().getTelemetryConfigMetadata()?.consentSchemaVersion;
99+
100+
if (consentSchemaVersion !== currentExtConsentSchemaVersion) {
101+
LOGGER.debug("Removing telemetry config from user settings due to consent schema version change");
102+
this.isTelemetryEnabled = false;
106103
this.updateTelemetrySetting(undefined);
107104
}
108-
this.isTelemetryEnabled = false;
109105
}
110106
}
111107
}
@@ -157,9 +153,6 @@ class VscodeTelemetryPreference implements TelemetryPreference {
157153
});
158154
}
159155

160-
// Question:
161-
// When consent version is changed, we have to show popup to all the users or only those who had accepted earlier?
162-
163156
// Test cases:
164157
// 1. User accepts consent and VSCode telemetry is set to 'all'. Output: enabled telemetry
165158
// 2. User accepts consent and VSCode telemetry is not set to 'all'. Output: disabled telemetry

‎vscode/src/telemetry/impl/telemetryReporterImpl.ts‎

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export class TelemetryReporterImpl implements TelemetryReporter {
3939
}
4040

4141
public startEvent = (): void => {
42-
this.resetOnCloseEventState();
42+
this.setOnCloseEventState();
4343
this.retryManager.startTimer();
4444

4545
const extensionStartEvent = ExtensionStartEvent.builder();
@@ -50,11 +50,6 @@ export class TelemetryReporterImpl implements TelemetryReporter {
5050
}
5151

5252
public closeEvent = (): void => {
53-
this.onCloseEventState = {
54-
status: true,
55-
numOfRetries: 0
56-
};
57-
5853
const extensionCloseEvent = ExtensionCloseEvent.builder(this.activationTime);
5954
this.addEventToQueue(extensionCloseEvent);
6055

@@ -63,42 +58,49 @@ export class TelemetryReporterImpl implements TelemetryReporter {
6358
}
6459

6560
public addEventToQueue = (event: BaseEvent<any>): void => {
66-
this.resetOnCloseEventState();
61+
this.setOnCloseEventState(event);
6762

6863
this.queue.enqueue(event);
6964
if (this.retryManager.isQueueOverflow(this.queue.size())) {
7065
LOGGER.debug(`Send triggered to queue size overflow`);
71-
if (this.retryManager.IsQueueMaxCapacityReached()) {
72-
LOGGER.debug('Decreasing size of the queue as max capacity reached');
73-
this.queue.decreaseSizeOnMaxOverflow();
74-
}
66+
const numOfeventsToBeDropped = this.retryManager.getNumberOfEventsToBeDropped();
7567
this.sendEvents();
68+
if (numOfeventsToBeDropped) {
69+
this.queue.decreaseSizeOnMaxOverflow(numOfeventsToBeDropped);
70+
}
7671
}
7772
}
7873

79-
private resetOnCloseEventState = () => {
80-
this.onCloseEventState = {
81-
status: false,
82-
numOfRetries: 0
83-
};
74+
private setOnCloseEventState = (event?: BaseEvent<any>) => {
75+
if (event?.NAME === ExtensionCloseEvent.NAME) {
76+
this.onCloseEventState = {
77+
status: true,
78+
numOfRetries: 0
79+
};
80+
} else {
81+
this.onCloseEventState = {
82+
status: false,
83+
numOfRetries: 0
84+
};
85+
}
8486
}
8587

8688
private increaseRetryCountOrDisableRetry = () => {
87-
if (this.onCloseEventState.status) {
88-
if (this.onCloseEventState.numOfRetries < this.MAX_RETRY_ON_CLOSE && this.queue.size()) {
89-
LOGGER.debug("Telemetry disabled state: Increasing retry count");
90-
this.onCloseEventState.numOfRetries++;
91-
} else {
92-
LOGGER.debug(`Telemetry disabled state: ${this.queue.size() ? 'Max retries reached': 'queue is empty'}, resetting timer`);
93-
this.retryManager.clearTimer();
94-
this.queue.flush();
95-
this.onCloseEventState = {
96-
status: false,
97-
numOfRetries: 0
98-
};
99-
}
89+
if (!this.onCloseEventState.status) return;
90+
91+
const queueEmpty = this.queue.size() === 0;
92+
const retriesExceeded = this.onCloseEventState.numOfRetries >= this.MAX_RETRY_ON_CLOSE;
93+
94+
if (queueEmpty || retriesExceeded) {
95+
LOGGER.debug(`Telemetry disabled state: ${queueEmpty ? 'Queue is empty' : 'Max retries reached'}, clearing timer`);
96+
this.retryManager.clearTimer();
97+
this.queue.flush();
98+
this.setOnCloseEventState();
99+
} else {
100+
LOGGER.debug("Telemetry disabled state: Increasing retry count");
101+
this.onCloseEventState.numOfRetries++;
100102
}
101-
}
103+
};
102104

103105
private sendEvents = async (): Promise<void> => {
104106
try {

‎vscode/src/telemetry/impl/telemetryRetry.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export class TelemetryRetry {
6262
this.numOfAttemptsWhenTimerHits++;
6363
return;
6464
}
65-
LOGGER.debug("Keeping timer same as max retries exceeded");
65+
LOGGER.debug("Keeping timer same as max capactiy reached");
6666
}
6767

6868
public clearTimer = (): void => {
@@ -88,11 +88,12 @@ export class TelemetryRetry {
8888
this.queueCapacity = this.TELEMETRY_RETRY_CONFIG.baseCapacity *
8989
Math.pow(this.TELEMETRY_RETRY_CONFIG.backoffFactor, this.numOfAttemptsWhenQueueIsFull);
9090
}
91-
LOGGER.debug("Keeping queue capacity same as max retries exceeded");
91+
LOGGER.debug("Keeping queue capacity same as max capacity reached");
9292
}
9393

94-
public IsQueueMaxCapacityReached = (): boolean =>
95-
this.numOfAttemptsWhenQueueIsFull > this.TELEMETRY_RETRY_CONFIG.maxRetries;
94+
public getNumberOfEventsToBeDropped = (): number =>
95+
this.numOfAttemptsWhenQueueIsFull >= this.TELEMETRY_RETRY_CONFIG.maxRetries ?
96+
this.queueCapacity/2 : 0;
9697

9798
private resetQueueCapacity = (): void => {
9899
LOGGER.debug("Resetting queue capacity to default");

‎vscode/src/telemetry/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export interface TelemetryReporter {
2727
export interface CacheService {
2828
get(key: string): string | undefined;
2929

30-
put(key: string, value: string): boolean;
30+
put(key: string, value: string): Promise<boolean>;
3131
}
3232

3333
export interface TelemetryEventQueue {

0 commit comments

Comments
 (0)