From e4091ab3b5dda9f39daecdc57db6c545bdf98c71 Mon Sep 17 00:00:00 2001 From: Glen Chung <105310954+kuchungmsft@users.noreply.github.com> Date: Wed, 15 Jan 2025 17:35:10 -0800 Subject: [PATCH] Combine ProjectContext and RelatedFilesProvider Telemetry in One Event (#13157) - This allows calculating the time spent on the extension side vs the language server side. - Also send filter telemetry if available. --- .../src/LanguageServer/copilotProviders.ts | 2 +- Extension/src/LanguageServer/lmTool.ts | 16 +-- .../SingleRootProject/tests/lmTool.test.ts | 127 ++++++++++++------ 3 files changed, 97 insertions(+), 48 deletions(-) diff --git a/Extension/src/LanguageServer/copilotProviders.ts b/Extension/src/LanguageServer/copilotProviders.ts index ce232865fa..d955356c8f 100644 --- a/Extension/src/LanguageServer/copilotProviders.ts +++ b/Extension/src/LanguageServer/copilotProviders.ts @@ -48,7 +48,7 @@ export async function registerRelatedFilesProvider(): Promise { try { const getIncludesHandler = async () => (await getIncludes(uri, 1))?.includedFiles.map(file => vscode.Uri.file(file)) ?? []; const getTraitsHandler = async () => { - const projectContext = await getProjectContext(uri, context); + const projectContext = await getProjectContext(uri, context, telemetryProperties, telemetryMetrics); if (!projectContext) { return undefined; diff --git a/Extension/src/LanguageServer/lmTool.ts b/Extension/src/LanguageServer/lmTool.ts index be03ca46e1..1ede1d0264 100644 --- a/Extension/src/LanguageServer/lmTool.ts +++ b/Extension/src/LanguageServer/lmTool.ts @@ -124,18 +124,20 @@ function filterCompilerArguments(compiler: string, compilerArguments: string[], if (filteredCompilerArguments.length > 0) { // Telemetry to learn about the argument distribution. The filtered arguments are expected to be non-PII. telemetryProperties["filteredCompilerArguments"] = filteredCompilerArguments.join(','); - telemetryProperties["filters"] = Object.keys(filterMap).filter(filter => !!filter).join(','); + } + + const filters = Object.keys(filterMap).filter(filter => !!filter).join(','); + if (filters) { + telemetryProperties["filters"] = filters; } return result; } -export async function getProjectContext(uri: vscode.Uri, context: { flags: Record }): Promise { - const telemetryProperties: Record = {}; - const telemetryMetrics: Record = {}; +export async function getProjectContext(uri: vscode.Uri, context: { flags: Record }, telemetryProperties: Record, telemetryMetrics: Record): Promise { try { const projectContext = await checkDuration(async () => await getClients()?.ActiveClient?.getProjectContext(uri) ?? undefined); - telemetryMetrics["duration"] = projectContext.duration; + telemetryMetrics["projectContextDuration"] = projectContext.duration; if (!projectContext.result) { return undefined; } @@ -186,10 +188,8 @@ export async function getProjectContext(uri: vscode.Uri, context: { flags: Recor catch { // Intentionally swallow any exception. } - telemetryProperties["error"] = "true"; + telemetryProperties["projectContextError"] = "true"; throw exception; // Throw the exception for auto-retry. - } finally { - telemetry.logCopilotEvent('ProjectContext', telemetryProperties, telemetryMetrics); } } diff --git a/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts b/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts index a5cc0825f7..01f55c98bf 100644 --- a/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts +++ b/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts @@ -179,13 +179,17 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler, context, compilerArguments: compilerArguments, - expectedCompilerArguments + expectedCompilerArguments, + telemetryProperties, + telemetryMetrics }: { compiler: string; expectedCompiler: string; context: { flags: Record }; compilerArguments: string[]; expectedCompilerArguments: Record; + telemetryProperties: Record; + telemetryMetrics: Record; }) => { arrangeProjectContextFromCppTools({ projectContextFromCppTools: { @@ -200,7 +204,7 @@ describe('CppConfigurationLanguageModelTool Tests', () => { } }); - const result = await getProjectContext(mockTextDocumentStub.uri, context); + const result = await getProjectContext(mockTextDocumentStub.uri, context, telemetryProperties, telemetryMetrics); ok(result, 'result should not be undefined'); ok(result.language === 'C++'); @@ -217,7 +221,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler: 'MSVC', context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo-?": ""}' } }, compilerArguments: ['foo', 'bar', 'abc', 'foo-'], - expectedCompilerArguments: { 'foo-?': 'foo-' } + expectedCompilerArguments: { 'foo-?': 'foo-' }, + telemetryProperties: {}, + telemetryMetrics: {} }); }); @@ -227,7 +233,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler: 'Clang', context: { flags: { copilotcppClangCompilerArgumentFilter: '{"foo": "", "bar": ""}' } }, compilerArguments: ['foo', 'bar', 'abc'], - expectedCompilerArguments: { 'foo': 'foo', 'bar': 'bar' } + expectedCompilerArguments: { 'foo': 'foo', 'bar': 'bar' }, + telemetryProperties: {}, + telemetryMetrics: {} }); }); @@ -237,7 +245,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler: 'Clang', context: { flags: { copilotcppClangCompilerArgumentFilter: '{"-std\\\\sc\\\\+\\\\+\\\\d+": ""}' } }, compilerArguments: ['-std', 'c++17', '-std', 'foo', '-std', 'c++11', '-std', 'bar'], - expectedCompilerArguments: { '-std\\sc\\+\\+\\d+': '-std c++11' } + expectedCompilerArguments: { '-std\\sc\\+\\+\\d+': '-std c++11' }, + telemetryProperties: {}, + telemetryMetrics: {} }); }); @@ -247,7 +257,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler: 'GCC', context: { flags: { copilotcppGccCompilerArgumentFilter: '{"foo": "", "bar": ""}' } }, compilerArguments: ['foo', 'bar', 'abc', 'bar', 'foo', 'bar'], - expectedCompilerArguments: { 'foo': 'foo', 'bar': 'bar' } + expectedCompilerArguments: { 'foo': 'foo', 'bar': 'bar' }, + telemetryProperties: {}, + telemetryMetrics: {} }); }); @@ -257,7 +269,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler: 'MSVC', context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo": "", "bar": ""}' } }, compilerArguments: [], - expectedCompilerArguments: {} + expectedCompilerArguments: {}, + telemetryProperties: {}, + telemetryMetrics: {} }); }); @@ -267,7 +281,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler: 'GCC', context: { flags: {} }, compilerArguments: ['foo', 'bar'], - expectedCompilerArguments: {} + expectedCompilerArguments: {}, + telemetryProperties: {}, + telemetryMetrics: {} }); }); @@ -277,7 +293,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler: 'MSVC', context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"": ""}' } }, compilerArguments: ['foo', 'bar'], - expectedCompilerArguments: {} + expectedCompilerArguments: {}, + telemetryProperties: {}, + telemetryMetrics: {} }); }); @@ -291,7 +309,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { } }, compilerArguments: ['foo', 'bar'], - expectedCompilerArguments: {} + expectedCompilerArguments: {}, + telemetryProperties: {}, + telemetryMetrics: {} }); }); @@ -305,7 +325,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { } }, compilerArguments: ['foo', 'bar'], - expectedCompilerArguments: {} + expectedCompilerArguments: {}, + telemetryProperties: {}, + telemetryMetrics: {} }); }); @@ -321,33 +343,60 @@ describe('CppConfigurationLanguageModelTool Tests', () => { } }, compilerArguments: ['foo', 'bar'], - expectedCompilerArguments: {} + expectedCompilerArguments: {}, + telemetryProperties: {}, + telemetryMetrics: {} }); }); it('should send telemetry.', async () => { + const telemetryProperties: Record = {}; + const telemetryMetrics: Record = {}; const input = { compiler: 'msvc', expectedCompiler: 'MSVC', context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo-?": "", "": "", "bar": "", "xyz": ""}' } }, compilerArguments: ['foo', 'bar', 'foo-', 'abc'], - expectedCompilerArguments: { 'foo-?': 'foo-', 'bar': 'bar' } + expectedCompilerArguments: { 'foo-?': 'foo-', 'bar': 'bar' }, + telemetryProperties, + telemetryMetrics }; await testGetProjectContext(input); - ok(telemetryStub.calledOnce, 'Telemetry should be called once'); - ok(telemetryStub.calledWithMatch('ProjectContext', sinon.match({ - "language": 'C++', - "compiler": input.expectedCompiler, - "standardVersion": 'C++20', - "targetPlatform": 'Windows', - "targetArchitecture": 'x64', - "filteredCompilerArguments": "foo,foo-,bar", - "filters": "foo-?,bar,xyz" - }), sinon.match({ - "compilerArgumentCount": input.compilerArguments.length, - 'duration': sinon.match.number - }))); + ok(telemetryProperties['language'] === 'C++'); + ok(telemetryProperties['compiler'] === input.expectedCompiler); + ok(telemetryProperties['standardVersion'] === 'C++20'); + ok(telemetryProperties['targetPlatform'] === 'Windows'); + ok(telemetryProperties['targetArchitecture'] === 'x64'); + ok(telemetryProperties['filteredCompilerArguments'] === 'foo,foo-,bar'); + ok(telemetryProperties['filters'] === 'foo-?,bar,xyz'); + ok(telemetryMetrics['compilerArgumentCount'] === input.compilerArguments.length); + ok(telemetryMetrics['projectContextDuration'] !== undefined); + }); + + it('should send filter telemetry if available.', async () => { + const telemetryProperties: Record = {}; + const telemetryMetrics: Record = {}; + const input = { + compiler: 'msvc', + expectedCompiler: 'MSVC', + context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo-?": "", "": "", "bar": "", "xyz": ""}' } }, + compilerArguments: ['abc'], + expectedCompilerArguments: {}, + telemetryProperties, + telemetryMetrics + }; + await testGetProjectContext(input); + + ok(telemetryProperties['language'] === 'C++'); + ok(telemetryProperties['compiler'] === input.expectedCompiler); + ok(telemetryProperties['standardVersion'] === 'C++20'); + ok(telemetryProperties['targetPlatform'] === 'Windows'); + ok(telemetryProperties['targetArchitecture'] === 'x64'); + ok(telemetryProperties['filteredCompilerArguments'] === undefined); + ok(telemetryProperties['filters'] === 'foo-?,bar,xyz'); + ok(telemetryMetrics['compilerArgumentCount'] === input.compilerArguments.length); + ok(telemetryMetrics['projectContextDuration'] !== undefined); }); it('should not send telemetry for unknown values', async () => { @@ -363,21 +412,21 @@ describe('CppConfigurationLanguageModelTool Tests', () => { } } }); + const telemetryProperties: Record = {}; + const telemetryMetrics: Record = {}; - const result = await getProjectContext(mockTextDocumentStub.uri, { flags: {} }); + const result = await getProjectContext(mockTextDocumentStub.uri, { + flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo-?": "", "": "", "bar": "", "xyz": ""}' } + }, telemetryProperties, telemetryMetrics); - ok(telemetryStub.calledOnce, 'Telemetry should be called once'); - ok(telemetryStub.calledWithMatch('ProjectContext', sinon.match({ - "targetArchitecture": 'bar' - }), sinon.match({ - "compilerArgumentCount": 0 - }))); - ok(telemetryStub.calledWithMatch('ProjectContext', sinon.match(property => - property['language'] === undefined && - property['compiler'] === undefined && - property['standardVersion'] === undefined && - property['originalStandardVersion'] === 'gnu++17' && - property['targetPlatform'] === undefined))); + ok(telemetryProperties["targetArchitecture"] === 'bar'); + ok(telemetryProperties["filters"] === undefined); + ok(telemetryProperties["language"] === undefined); + ok(telemetryProperties["compiler"] === undefined); + ok(telemetryProperties["standardVersion"] === undefined); + ok(telemetryProperties["originalStandardVersion"] === 'gnu++17'); + ok(telemetryProperties["targetPlatform"] === undefined); + ok(telemetryMetrics["compilerArgumentCount"] === 0); ok(result, 'result should not be undefined'); ok(result.language === ''); ok(result.compiler === '');