Skip to content

Commit 672f95f

Browse files
committed
Added ability to transform telemetry events and fixed possible race condition while sending JDK downloader event
addressed review comments
1 parent 78f4858 commit 672f95f

File tree

8 files changed

+302
-16
lines changed

8 files changed

+302
-16
lines changed

vscode/src/telemetry/events/baseEvent.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import { LOGGER } from "../../logger";
1717
import { AnonymousIdManager } from "../impl/AnonymousIdManager";
1818
import { cacheService } from "../impl/cacheServiceImpl";
19-
import { getHashCode } from "../utils";
19+
import { getHashCode, getValuesToBeTransformed, transformValue } from "../utils";
2020

2121
export interface BaseEventPayload {
2222
vsCodeId: string;
@@ -26,6 +26,7 @@ export interface BaseEventPayload {
2626
export abstract class BaseEvent<T> {
2727
protected _payload: T & BaseEventPayload;
2828
protected _data: T
29+
private static readonly blockedValues = getValuesToBeTransformed();
2930

3031
constructor(public readonly NAME: string,
3132
public readonly ENDPOINT: string,
@@ -47,6 +48,13 @@ export abstract class BaseEvent<T> {
4748
return this._data;
4849
}
4950

51+
protected static transformEvent = (propertiesToTransform: string[], payload: Record<string, any>): any => {
52+
const replacedValue = "_REM_";
53+
54+
return transformValue(null, this.blockedValues, propertiesToTransform, replacedValue, payload);
55+
};
56+
57+
5058
public onSuccessPostEventCallback = async (): Promise<void> => {
5159
LOGGER.debug(`${this.NAME} sent successfully`);
5260
}

vscode/src/telemetry/events/jdkFeature.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@ export interface JdkFeatureEventData {
2525
export class JdkFeatureEvent extends BaseEvent<JdkFeatureEventData> {
2626
public static readonly NAME = "jdkFeature";
2727
public static readonly ENDPOINT = "/jdkFeature";
28+
private static readonly propertiesToTransform = ['javaVersion'];
2829

2930
constructor(payload: JdkFeatureEventData) {
30-
super(JdkFeatureEvent.NAME, JdkFeatureEvent.ENDPOINT, payload);
31+
const updatedPayload: JdkFeatureEventData = BaseEvent.transformEvent(JdkFeatureEvent.propertiesToTransform, payload);
32+
super(JdkFeatureEvent.NAME, JdkFeatureEvent.ENDPOINT, updatedPayload);
3133
}
3234

3335
public static concatEvents(events:JdkFeatureEvent[]): JdkFeatureEvent[] {

vscode/src/telemetry/events/start.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,11 @@ export interface StartEventData {
5353
export class ExtensionStartEvent extends BaseEvent<StartEventData> {
5454
public static readonly NAME = "startup";
5555
public static readonly ENDPOINT = "/start";
56+
private static readonly propertiesToTransform = ['osVersion'];
5657

5758
constructor(payload: StartEventData) {
58-
super(ExtensionStartEvent.NAME, ExtensionStartEvent.ENDPOINT, payload);
59+
const updatedPayload: StartEventData = BaseEvent.transformEvent(ExtensionStartEvent.propertiesToTransform, payload);
60+
super(ExtensionStartEvent.NAME, ExtensionStartEvent.ENDPOINT, updatedPayload);
5961
}
6062

6163
onSuccessPostEventCallback = async (): Promise<void> => {

vscode/src/telemetry/events/workspaceChange.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@ let workspaceChangeEventTimeout: NodeJS.Timeout | null = null;
3737
export class WorkspaceChangeEvent extends BaseEvent<WorkspaceChangeData> {
3838
public static readonly NAME = "workspaceChange";
3939
public static readonly ENDPOINT = "/workspaceChange";
40+
private static readonly propertiesToTransform = ['javaVersion'];
4041

4142
constructor(payload: WorkspaceChangeData) {
42-
super(WorkspaceChangeEvent.NAME, WorkspaceChangeEvent.ENDPOINT, payload);
43+
const updatedPayload: WorkspaceChangeData = BaseEvent.transformEvent(WorkspaceChangeEvent.propertiesToTransform, payload);
44+
super(WorkspaceChangeEvent.NAME, WorkspaceChangeEvent.ENDPOINT, updatedPayload);
4345
}
4446

4547
public onSuccessPostEventCallback = async (): Promise<void> => {

vscode/src/telemetry/utils.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
*/
1616
import * as crypto from 'crypto';
1717
import { Uri, workspace } from 'vscode';
18+
import * as os from 'os';
19+
import { isObject, isString } from '../utils';
1820

1921
export const getCurrentUTCDateInSeconds = () => {
2022
const date = Date.now();
@@ -70,3 +72,72 @@ const getUri = (pathOrUri: Uri | string): Uri => {
7072
}
7173
return Uri.file(pathOrUri);
7274
}
75+
76+
export const getValuesToBeTransformed = (): string[] => {
77+
const MIN_BLOCKED_VAL_LENGTH = 3;
78+
const IGNORED_VALUES = ['java', 'vscode', 'user', 'oracle'];
79+
80+
const blockedValues: (string | undefined)[] = [
81+
process.env?.SUDO_USER,
82+
process.env?.C9_USER,
83+
process.env?.LOGNAME,
84+
process.env?.USER,
85+
process.env?.LNAME,
86+
process.env?.USERNAME,
87+
process.env?.HOSTNAME,
88+
process.env?.COMPUTERNAME,
89+
process.env?.NAME,
90+
os.userInfo().username].map(el => el?.trim());
91+
92+
return Array.from(new Set(blockedValues.filter((el): el is string => el !== undefined &&
93+
el.length >= MIN_BLOCKED_VAL_LENGTH &&
94+
!IGNORED_VALUES.includes(el)
95+
)));
96+
}
97+
98+
99+
export const isPrimitiveTransformationNotRequired = (value: any) => (
100+
value === null ||
101+
typeof value === 'number' ||
102+
typeof value === 'boolean' ||
103+
typeof value === 'undefined' ||
104+
typeof value === 'symbol' ||
105+
typeof value === 'bigint'
106+
);
107+
108+
109+
export const transformValue = (key: string | null, blockedValues: string[], propertiesToTransform: string[], replacedValue: string, value: any): any => {
110+
if (isPrimitiveTransformationNotRequired(value)) {
111+
return value;
112+
}
113+
114+
if (isString(value)) {
115+
if (!value.trim().length) return value;
116+
117+
let updatedValue = value;
118+
if (key == null || !propertiesToTransform.includes(key)) return value;
119+
blockedValues.forEach(valueToBeReplaced =>
120+
updatedValue = replaceAllOccurrences(updatedValue, valueToBeReplaced, replacedValue)
121+
);
122+
123+
return updatedValue;
124+
}
125+
126+
if (Array.isArray(value)) {
127+
return value.map(el => transformValue(key, blockedValues, propertiesToTransform, replacedValue, el));
128+
}
129+
130+
if (isObject(value)) {
131+
const result: any = {};
132+
for (const [k, val] of Object.entries(value)) {
133+
result[k] = transformValue(k, blockedValues, propertiesToTransform, replacedValue, val);
134+
}
135+
return result;
136+
}
137+
return value;
138+
};
139+
140+
export const replaceAllOccurrences = (str: string, valueString: string, replaceString: string) => {
141+
if(!valueString.trim().length) return str;
142+
return str.split(valueString).join(replaceString);
143+
}
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
import { expect } from "chai";
2+
import { describe, it, beforeEach, afterEach } from "mocha";
3+
import { transformValue, replaceAllOccurrences, getValuesToBeTransformed } from "../../../telemetry/utils";
4+
import * as sinon from 'sinon';
5+
import * as os from 'os';
6+
7+
const replacedValue = "_REM_";
8+
const propertiesToTransform = ["message", "b", "c", "d"];
9+
10+
describe("transformValue()", () => {
11+
it("replaces top-level string if key is in propertiesToTransform", () => {
12+
const result = transformValue("message", ["john"], propertiesToTransform, replacedValue, "hello john");
13+
expect(result).to.equal(`hello ${replacedValue}`);
14+
});
15+
16+
it("does not replace if key is not in propertiesToTransform", () => {
17+
const result = transformValue("greeting", ["john"], propertiesToTransform, replacedValue, "hello john");
18+
expect(result).to.equal("hello john");
19+
});
20+
21+
it("replaces nested name values for matching keys only", () => {
22+
const input = {
23+
b: "john is here",
24+
c: ["john", { d: "also john" }]
25+
};
26+
27+
const result = transformValue("a", ["john"], propertiesToTransform, replacedValue, input);
28+
29+
expect(result.b).to.equal(`${replacedValue} is here`);
30+
expect(result.c[0]).to.equal(`${replacedValue}`);
31+
expect(result.c[1].d).to.equal(`also ${replacedValue}`);
32+
});
33+
34+
it("handles keys with deep nesting", () => {
35+
const blocked = ["john", "doe"];
36+
const data = {
37+
level1: {
38+
message: "john was here",
39+
test: "john was here",
40+
b: ["hello john", "doe speaks", "java"],
41+
nestedArray: [
42+
{
43+
b: "john and doe",
44+
other: "keep this"
45+
},
46+
{
47+
b: "john is safe",
48+
d: ["john doe", { d: "doe and john" }]
49+
}
50+
]
51+
},
52+
c: [
53+
{
54+
d: "test john",
55+
random: "doe"
56+
},
57+
{
58+
a: {
59+
b: "john"
60+
},
61+
b: "john and jane",
62+
d: null
63+
}
64+
],
65+
extra: "nothing here"
66+
};
67+
68+
const result = transformValue(null, blocked, propertiesToTransform, replacedValue, data);
69+
70+
expect(result.level1.message).to.equal(`${replacedValue} was here`);
71+
expect(result.level1.test).to.equal("john was here");
72+
expect(result.level1.b).to.deep.equal([`hello ${replacedValue}`, `${replacedValue} speaks`, "java"]);
73+
expect(result.level1.nestedArray[0].b).to.equal(`${replacedValue} and ${replacedValue}`);
74+
expect(result.level1.nestedArray[0].other).to.equal("keep this");
75+
76+
expect(result.level1.nestedArray[1].b).to.equal(`${replacedValue} is safe`);
77+
expect(result.level1.nestedArray[1].d[0]).to.equal(`${replacedValue} ${replacedValue}`);
78+
expect(result.level1.nestedArray[1].d[1].d).to.equal(`${replacedValue} and ${replacedValue}`);
79+
80+
expect(result.c[0].d).to.equal(`test ${replacedValue}`);
81+
expect(result.c[0].random).to.equal("doe");
82+
83+
expect(result.c[1].a.b).to.equal(`${replacedValue}`);
84+
expect(result.c[1].b).to.equal(`${replacedValue} and jane`);
85+
expect(result.c[1].d).to.equal(null);
86+
87+
expect(result.extra).to.equal("nothing here");
88+
});
89+
90+
it("does not transform primitive values", () => {
91+
const data = {
92+
count: 5,
93+
flag: true,
94+
nothing: null
95+
};
96+
const result = transformValue(null, ["john"], propertiesToTransform, replacedValue, data);
97+
98+
expect(result.count).to.equal(5);
99+
expect(result.flag).to.equal(true);
100+
expect(result.nothing).to.equal(null);
101+
});
102+
103+
it("returns original string if no blocked values", () => {
104+
const result = transformValue("message", [], propertiesToTransform, replacedValue, "hello john");
105+
expect(result).to.equal("hello john");
106+
});
107+
});
108+
109+
describe("replaceAllOccurrences()", () => {
110+
it("replaces all non-overlapping occurrences", () => {
111+
const result = replaceAllOccurrences("john is john", "john", "_REM_");
112+
expect(result).to.equal("_REM_ is _REM_");
113+
});
114+
115+
it("does not change string if valueString not found", () => {
116+
const result = replaceAllOccurrences("hello world", "john", "_REM_");
117+
expect(result).to.equal("hello world");
118+
});
119+
120+
it("replaces multiple adjacent matches", () => {
121+
const result = replaceAllOccurrences("johnjohnjohn", "john", "_REM_");
122+
expect(result).to.equal("_REM__REM__REM_");
123+
});
124+
125+
it("returns original string if valueString is empty", () => {
126+
const result = replaceAllOccurrences("john is here", "", "_REM_");
127+
expect(result).to.equal("john is here");
128+
});
129+
130+
it("can replace special characters (treated literally)", () => {
131+
const result = replaceAllOccurrences("price is $5.00 and $5.00", "$5.00", "_REM_");
132+
expect(result).to.equal("price is _REM_ and _REM_");
133+
});
134+
135+
it("is case-sensitive by default", () => {
136+
const result = replaceAllOccurrences("John john JOHN", "john", "_REM_");
137+
expect(result).to.equal("John _REM_ JOHN");
138+
});
139+
140+
it("handles overlapping cases (only first per loop)", () => {
141+
const result = replaceAllOccurrences("aaa", "aa", "_X_");
142+
expect(result).to.equal("_X_a");
143+
});
144+
});
145+
146+
describe("transformValue() with getValuesToBeTransformed()", () => {
147+
let stub: sinon.SinonStub;
148+
149+
beforeEach(() => {
150+
stub = sinon.stub(os, "userInfo").returns({ username: "john" } as os.UserInfo<string>);
151+
});
152+
153+
afterEach(() => {
154+
stub.restore();
155+
});
156+
157+
it("transforms string using values from getValuesToBeTransformed", () => {
158+
const blocked = getValuesToBeTransformed();
159+
160+
const result = transformValue("message", blocked, propertiesToTransform, replacedValue, "hello john");
161+
162+
expect(result).to.equal(`hello ${replacedValue}`);
163+
});
164+
165+
it("does not transform if key is not in propertiesToTransform", () => {
166+
const blocked = getValuesToBeTransformed();
167+
168+
const result = transformValue("greeting", blocked, propertiesToTransform, replacedValue, "hello john");
169+
170+
expect(result).to.equal("hello john");
171+
});
172+
173+
it("filters ignored and short env values correctly", () => {
174+
const testEnv = {
175+
SUDO_USER: "ja",
176+
C9_USER: "java",
177+
LOGNAME: "john",
178+
USER: "oracle",
179+
LNAME: "ab",
180+
USERNAME: "johndoe",
181+
HOSTNAME: undefined,
182+
COMPUTERNAME: "user",
183+
NAME: "vscode"
184+
};
185+
186+
const stubbed = sinon.stub(process, "env").value(testEnv as any);
187+
stub.restore();
188+
189+
const values = getValuesToBeTransformed();
190+
191+
expect(values).to.include("john");
192+
expect(values).to.include("johndoe");
193+
expect(values).to.not.include("java");
194+
expect(values).to.not.include("user");
195+
expect(values).to.not.include("ab");
196+
expect(values).to.not.include("vscode");
197+
198+
stubbed.restore();
199+
});
200+
});

vscode/src/utils.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,8 @@ export function isError(obj: unknown): obj is Error {
287287
return obj instanceof Error;
288288
}
289289

290+
export const isObject = (value: any) => value !== null && typeof value === 'object' && !Array.isArray(value);
291+
290292
export async function initializeRunConfiguration(): Promise<boolean> {
291293
if (vscode.workspace.name || vscode.workspace.workspaceFile) {
292294
const java = await vscode.workspace.findFiles('**/*.java', '**/node_modules/**', 1);

0 commit comments

Comments
 (0)