Skip to content

Commit

Permalink
Merge branch 'main' into seanmcm/1_23_4_changelog2
Browse files Browse the repository at this point in the history
  • Loading branch information
@sean-mcmanus
sean-mcmanus authored Jan 16, 2025
2 parents b886190 + e4091ab commit 3601cd2
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 48 deletions.
2 changes: 1 addition & 1 deletion Extension/src/LanguageServer/copilotProviders.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export async function registerRelatedFilesProvider(): Promise<void> {
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;
Expand Down
16 changes: 8 additions & 8 deletions Extension/src/LanguageServer/lmTool.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> }): Promise<ProjectContext | undefined> {
const telemetryProperties: Record<string, string> = {};
const telemetryMetrics: Record<string, number> = {};
export async function getProjectContext(uri: vscode.Uri, context: { flags: Record<string, unknown> }, telemetryProperties: Record<string, string>, telemetryMetrics: Record<string, number>): Promise<ProjectContext | undefined> {
try {
const projectContext = await checkDuration<ProjectContextResult | undefined>(async () => await getClients()?.ActiveClient?.getProjectContext(uri) ?? undefined);
telemetryMetrics["duration"] = projectContext.duration;
telemetryMetrics["projectContextDuration"] = projectContext.duration;
if (!projectContext.result) {
return undefined;
}
Expand Down Expand Up @@ -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);
}
}

Expand Down
127 changes: 88 additions & 39 deletions Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,17 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
expectedCompiler,
context,
compilerArguments: compilerArguments,
expectedCompilerArguments
expectedCompilerArguments,
telemetryProperties,
telemetryMetrics
}: {
compiler: string;
expectedCompiler: string;
context: { flags: Record<string, unknown> };
compilerArguments: string[];
expectedCompilerArguments: Record<string, string>;
telemetryProperties: Record<string, string>;
telemetryMetrics: Record<string, number>;
}) => {
arrangeProjectContextFromCppTools({
projectContextFromCppTools: {
Expand All @@ -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++');
Expand All @@ -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: {}
});
});

Expand All @@ -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: {}
});
});

Expand All @@ -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: {}
});
});

Expand All @@ -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: {}
});
});

Expand All @@ -257,7 +269,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
expectedCompiler: 'MSVC',
context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo": "", "bar": ""}' } },
compilerArguments: [],
expectedCompilerArguments: {}
expectedCompilerArguments: {},
telemetryProperties: {},
telemetryMetrics: {}
});
});

Expand All @@ -267,7 +281,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
expectedCompiler: 'GCC',
context: { flags: {} },
compilerArguments: ['foo', 'bar'],
expectedCompilerArguments: {}
expectedCompilerArguments: {},
telemetryProperties: {},
telemetryMetrics: {}
});
});

Expand All @@ -277,7 +293,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
expectedCompiler: 'MSVC',
context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"": ""}' } },
compilerArguments: ['foo', 'bar'],
expectedCompilerArguments: {}
expectedCompilerArguments: {},
telemetryProperties: {},
telemetryMetrics: {}
});
});

Expand All @@ -291,7 +309,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
}
},
compilerArguments: ['foo', 'bar'],
expectedCompilerArguments: {}
expectedCompilerArguments: {},
telemetryProperties: {},
telemetryMetrics: {}
});
});

Expand All @@ -305,7 +325,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
}
},
compilerArguments: ['foo', 'bar'],
expectedCompilerArguments: {}
expectedCompilerArguments: {},
telemetryProperties: {},
telemetryMetrics: {}
});
});

Expand All @@ -321,33 +343,60 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
}
},
compilerArguments: ['foo', 'bar'],
expectedCompilerArguments: {}
expectedCompilerArguments: {},
telemetryProperties: {},
telemetryMetrics: {}
});
});

it('should send telemetry.', async () => {
const telemetryProperties: Record<string, string> = {};
const telemetryMetrics: Record<string, number> = {};
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<string, string> = {};
const telemetryMetrics: Record<string, number> = {};
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 () => {
Expand All @@ -363,21 +412,21 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
}
}
});
const telemetryProperties: Record<string, string> = {};
const telemetryMetrics: Record<string, number> = {};

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 === '');
Expand Down

0 comments on commit 3601cd2

Please sign in to comment.