Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions src/common/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,24 +212,24 @@ export enum EventNames {
/**
* Telemetry event fired once per session, per URI, the first time a `.py`
* file with a valid PEP 723 `# /// script` block is observed by the lazy
* detector. Used to size the population of users who actually see PEP 723
* files — the denominator for the "view vs edit" question.
* detector. Used to size the population of users who actually see inline
* script files — the denominator for the "view vs edit" question.
* Properties:
* - trigger: 'open' | 'save' (which workspace event surfaced the file)
* - hasRequiresPython: boolean (whether the block declares `requires-python`)
* Measures:
* - dependencyCount: number (number of entries in the `dependencies` list)
*/
PEP723_DETECTED = 'PEP723.DETECTED',
INLINE_SCRIPT_DETECTED = 'inlineScript.detected',
/**
* Telemetry event fired once per session, per URI, the first time a `.py`
* file that previously raised a `PEP723.DETECTED` event receives a real
* text edit. Together with `PEP723.DETECTED` this measures the fraction
* of users who do more than view PEP 723 scripts.
* file that previously raised an `inlineScript.detected` event receives a
* real text edit. Together with `inlineScript.detected` this measures the
* fraction of users who do more than view inline script files.
* Measures:
* - duration: number (ms between the detection and the first edit)
*/
PEP723_EDITED = 'PEP723.EDITED',
INLINE_SCRIPT_EDITED = 'inlineScript.edited',
}

// Map all events to their properties
Expand Down Expand Up @@ -707,23 +707,23 @@ export interface IEventNamePropertyMapping {
};

/* __GDPR__
"pep723.detected": {
"inlineScript.detected": {
"trigger": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "StellaHuang95" },
"hasRequiresPython": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "StellaHuang95" },
"dependencyCount": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "StellaHuang95" }
}
*/
[EventNames.PEP723_DETECTED]: {
[EventNames.INLINE_SCRIPT_DETECTED]: {
trigger: 'open' | 'save';
hasRequiresPython: boolean;
// Goes through the measures payload (numeric); listed here for GDPR only.
dependencyCount?: number;
};

/* __GDPR__
"pep723.edited": {
"inlineScript.edited": {
"<duration>": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "StellaHuang95" }
}
*/
[EventNames.PEP723_EDITED]: never | undefined;
[EventNames.INLINE_SCRIPT_EDITED]: never | undefined;
}
4 changes: 2 additions & 2 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
);

// Silent observer for `.py` files that declare PEP 723 inline
// script metadata. Emits anonymized telemetry (PEP723.DETECTED /
// PEP723.EDITED) but does not register projects or surface any UI.
// script metadata. Emits anonymized telemetry (inlineScript.detected /
// inlineScript.edited) but does not register projects or surface any UI.
const inlineScriptLazyDetector = new InlineScriptLazyDetector();
inlineScriptLazyDetector.activate();
context.subscriptions.push(inlineScriptLazyDetector);
Expand Down
22 changes: 11 additions & 11 deletions src/features/inlineScriptLazyDetector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import {
* every eligible `.py` file the user opens or saves and emits two
* anonymized telemetry events:
*
* - `PEP723.DETECTED` once per (URI, session) the first time a
* - `inlineScript.detected` once per (URI, session) the first time a
* valid `# /// script` block is observed. This is the denominator
* for the "how many users actually see PEP 723 files" question.
* - `PEP723.EDITED` once per (URI, session) the first time a
* for the "how many users actually see inline script files" question.
* - `inlineScript.edited` once per (URI, session) the first time a
* previously-detected file receives a real text edit. Together
* with `DETECTED` this distinguishes viewers from editors.
* with `inlineScript.detected` this distinguishes viewers from editors.
*
* No URIs, file paths, or file content are sent. The detector does
* not register projects, surface UI, or otherwise change extension
Expand All @@ -41,11 +41,11 @@ export class InlineScriptLazyDetector implements Disposable {
// doesn't double-process the same file.
private readonly inFlight = new Map<string, Promise<void>>();
// URIs (as `uri.toString()`) for which we have already emitted
// `PEP723.DETECTED` in this session. Used to dedup the detection
// event across repeat opens/saves and to gate `PEP723.EDITED` so
// `inlineScript.detected` in this session. Used to dedup the detection
// event across repeat opens/saves and to gate `inlineScript.edited` so
// the latter only fires for files we already counted as detected.
private readonly detectedUris = new Set<string>();
// URIs for which we have already emitted `PEP723.EDITED` in this
// URIs for which we have already emitted `inlineScript.edited` in this
// session. Each detected file emits at most one edited event.
private readonly editedUris = new Set<string>();
// Wall-clock ms (from `Date.now`) at which each URI's detection
Expand Down Expand Up @@ -178,7 +178,7 @@ export class InlineScriptLazyDetector implements Disposable {
this.detectionAtMs.set(key, Date.now());
traceVerbose(`inlineScriptLazyDetector: detected inline script metadata in ${uri.fsPath} (${trigger})`);
sendTelemetryEvent(
EventNames.PEP723_DETECTED,
EventNames.INLINE_SCRIPT_DETECTED,
{ dependencyCount: metadata.dependencies?.length ?? 0 },
{
trigger,
Expand All @@ -194,11 +194,11 @@ export class InlineScriptLazyDetector implements Disposable {
}

/**
* Emit `PEP723.EDITED` the first time a previously-detected URI
* Emit `inlineScript.edited` the first time a previously-detected URI
* receives a real content change. The handler is hot (fires on
* every keystroke in every text document workspace-wide) so it
* bails out as cheaply as possible for the common case where the
* file is not a tracked PEP 723 script.
* file is not a tracked inline script.
*/
private handleChange(e: TextDocumentChangeEvent): void {
if (this.disposed) {
Expand All @@ -222,7 +222,7 @@ export class InlineScriptLazyDetector implements Disposable {
traceVerbose(
`inlineScriptLazyDetector: first edit observed on ${e.document.uri.fsPath} (${duration}ms after detection)`,
);
sendTelemetryEvent(EventNames.PEP723_EDITED, duration);
sendTelemetryEvent(EventNames.INLINE_SCRIPT_EDITED, duration);
}
}

Expand Down
50 changes: 25 additions & 25 deletions src/test/features/inlineScriptLazyDetector.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ suite('InlineScriptLazyDetector', () => {
changeListener!(makeChange(uri, changes));
}

// Filter `sendTelemetryStub.getCalls()` to a single PEP 723 event name.
// Filter `sendTelemetryStub.getCalls()` to a single inline script event name.
function callsFor(name: EventNames): sinon.SinonSpyCall[] {
return sendTelemetryStub.getCalls().filter((c) => c.args[0] === name);
}
Expand Down Expand Up @@ -224,7 +224,7 @@ suite('InlineScriptLazyDetector', () => {
// further work — including the detection telemetry event.
resolveRead!(VALID_METADATA);
await assert.doesNotReject(inFlight ?? Promise.resolve());
assert.strictEqual(callsFor(EventNames.PEP723_DETECTED).length, 0, 'no detection event after dispose');
assert.strictEqual(callsFor(EventNames.INLINE_SCRIPT_DETECTED).length, 0, 'no detection event after dispose');
});

// ---------- catch-up replay over `getOpenTextDocuments` ----------
Expand Down Expand Up @@ -271,47 +271,47 @@ suite('InlineScriptLazyDetector', () => {
assert.ok(readMetadataStub.notCalled, 'dispose() must clear the pending setImmediate handle');
});

// ---------- PEP723.DETECTED telemetry ----------
// ---------- inlineScript.detected telemetry ----------

test('PEP723.DETECTED fires once with trigger=open + dependencyCount + hasRequiresPython', async () => {
test('inlineScript.detected fires once with trigger=open + dependencyCount + hasRequiresPython', async () => {
const uri = Uri.file(path.resolve('/ws/detect.py'));
readMetadataStub.resolves(VALID_METADATA);
const detector = new InlineScriptLazyDetector();
detector.activate();
await fireOpen(uri);

const detectedCalls = callsFor(EventNames.PEP723_DETECTED);
const detectedCalls = callsFor(EventNames.INLINE_SCRIPT_DETECTED);
assert.strictEqual(detectedCalls.length, 1, 'detection event should fire exactly once');
const [, measures, properties] = detectedCalls[0].args;
assert.deepStrictEqual(measures, { dependencyCount: 2 });
assert.deepStrictEqual(properties, { trigger: 'open', hasRequiresPython: true });
detector.dispose();
});

test('PEP723.DETECTED fires with trigger=save when surfaced by a save event', async () => {
test('inlineScript.detected fires with trigger=save when surfaced by a save event', async () => {
const uri = Uri.file(path.resolve('/ws/detectOnSave.py'));
readMetadataStub.resolves(VALID_METADATA);
const detector = new InlineScriptLazyDetector();
detector.activate();
await fireSave(uri);

const detectedCalls = callsFor(EventNames.PEP723_DETECTED);
const detectedCalls = callsFor(EventNames.INLINE_SCRIPT_DETECTED);
assert.strictEqual(detectedCalls.length, 1);
assert.strictEqual(detectedCalls[0].args[2].trigger, 'save');
detector.dispose();
});

test('PEP723.DETECTED does not fire when the file has no metadata block', async () => {
test('inlineScript.detected does not fire when the file has no metadata block', async () => {
const uri = Uri.file(path.resolve('/ws/plain.py'));
readMetadataStub.resolves(undefined);
const detector = new InlineScriptLazyDetector();
detector.activate();
await fireOpen(uri);
assert.strictEqual(callsFor(EventNames.PEP723_DETECTED).length, 0);
assert.strictEqual(callsFor(EventNames.INLINE_SCRIPT_DETECTED).length, 0);
detector.dispose();
});

test('PEP723.DETECTED is deduplicated across repeated opens and saves of the same URI', async () => {
test('inlineScript.detected is deduplicated across repeated opens and saves of the same URI', async () => {
const uri = Uri.file(path.resolve('/ws/repeat.py'));
readMetadataStub.resolves(VALID_METADATA);
const detector = new InlineScriptLazyDetector();
Expand All @@ -320,11 +320,11 @@ suite('InlineScriptLazyDetector', () => {
await fireSave(uri);
await fireSave(uri);
await fireOpen(uri);
assert.strictEqual(callsFor(EventNames.PEP723_DETECTED).length, 1, 'detection event must dedup per session');
assert.strictEqual(callsFor(EventNames.INLINE_SCRIPT_DETECTED).length, 1, 'detection event must dedup per session');
detector.dispose();
});

test('PEP723.DETECTED reports hasRequiresPython=false when not declared', async () => {
test('inlineScript.detected reports hasRequiresPython=false when not declared', async () => {
const uri = Uri.file(path.resolve('/ws/noPython.py'));
readMetadataStub.resolves({
requiresPython: undefined,
Expand All @@ -336,23 +336,23 @@ suite('InlineScriptLazyDetector', () => {
detector.activate();
await fireOpen(uri);

const [, measures, properties] = callsFor(EventNames.PEP723_DETECTED)[0].args;
const [, measures, properties] = callsFor(EventNames.INLINE_SCRIPT_DETECTED)[0].args;
assert.deepStrictEqual(measures, { dependencyCount: 0 });
assert.deepStrictEqual(properties, { trigger: 'open', hasRequiresPython: false });
detector.dispose();
});

// ---------- PEP723.EDITED telemetry ----------
// ---------- inlineScript.edited telemetry ----------

test('PEP723.EDITED fires once on first content change after detection', async () => {
test('inlineScript.edited fires once on first content change after detection', async () => {
const uri = Uri.file(path.resolve('/ws/edit.py'));
readMetadataStub.resolves(VALID_METADATA);
const detector = new InlineScriptLazyDetector();
detector.activate();
await fireOpen(uri);
fireChange(uri);

const editedCalls = callsFor(EventNames.PEP723_EDITED);
const editedCalls = callsFor(EventNames.INLINE_SCRIPT_EDITED);
assert.strictEqual(editedCalls.length, 1, 'edited event should fire exactly once');
// Second arg is the measure (number → { duration }); accept either form.
const measureArg = editedCalls[0].args[1];
Expand All @@ -361,7 +361,7 @@ suite('InlineScriptLazyDetector', () => {
detector.dispose();
});

test('PEP723.EDITED is deduplicated across repeated edits of the same URI', async () => {
test('inlineScript.edited is deduplicated across repeated edits of the same URI', async () => {
const uri = Uri.file(path.resolve('/ws/multiEdit.py'));
readMetadataStub.resolves(VALID_METADATA);
const detector = new InlineScriptLazyDetector();
Expand All @@ -370,22 +370,22 @@ suite('InlineScriptLazyDetector', () => {
fireChange(uri);
fireChange(uri);
fireChange(uri);
assert.strictEqual(callsFor(EventNames.PEP723_EDITED).length, 1);
assert.strictEqual(callsFor(EventNames.INLINE_SCRIPT_EDITED).length, 1);
detector.dispose();
});

test('PEP723.EDITED does not fire for changes on a URI that was never detected', async () => {
test('inlineScript.edited does not fire for changes on a URI that was never detected', async () => {
const uri = Uri.file(path.resolve('/ws/notDetected.py'));
readMetadataStub.resolves(undefined);
const detector = new InlineScriptLazyDetector();
detector.activate();
await fireOpen(uri);
fireChange(uri);
assert.strictEqual(callsFor(EventNames.PEP723_EDITED).length, 0);
assert.strictEqual(callsFor(EventNames.INLINE_SCRIPT_EDITED).length, 0);
detector.dispose();
});

test('PEP723.EDITED ignores change events with no content changes', async () => {
test('inlineScript.edited ignores change events with no content changes', async () => {
const uri = Uri.file(path.resolve('/ws/noOpChange.py'));
readMetadataStub.resolves(VALID_METADATA);
const detector = new InlineScriptLazyDetector();
Expand All @@ -395,14 +395,14 @@ suite('InlineScriptLazyDetector', () => {
// array for things like dirty-state toggles; that's not a user
// edit and must not count.
fireChange(uri, []);
assert.strictEqual(callsFor(EventNames.PEP723_EDITED).length, 0);
assert.strictEqual(callsFor(EventNames.INLINE_SCRIPT_EDITED).length, 0);
// A real edit still counts after the no-op was ignored.
fireChange(uri);
assert.strictEqual(callsFor(EventNames.PEP723_EDITED).length, 1);
assert.strictEqual(callsFor(EventNames.INLINE_SCRIPT_EDITED).length, 1);
detector.dispose();
});

test('PEP723.EDITED is suppressed after dispose()', async () => {
test('inlineScript.edited is suppressed after dispose()', async () => {
const uri = Uri.file(path.resolve('/ws/disposedEdit.py'));
readMetadataStub.resolves(VALID_METADATA);
const detector = new InlineScriptLazyDetector();
Expand All @@ -411,7 +411,7 @@ suite('InlineScriptLazyDetector', () => {
const grabbedChangeListener = changeListener!;
detector.dispose();
grabbedChangeListener(makeChange(uri));
assert.strictEqual(callsFor(EventNames.PEP723_EDITED).length, 0);
assert.strictEqual(callsFor(EventNames.INLINE_SCRIPT_EDITED).length, 0);
});
});

Expand Down
Loading