Refactor telemetry and make reporter parameters optional

Co-authored-by: JoeRobich <611219+JoeRobich@users.noreply.github.com>

Author: Copilot

src/omnisharp/omnisharpDownloader.ts

@@ -28,7 +28,7 @@ export class OmnisharpDownloader {
         private packageJSON: any,
         private platformInfo: PlatformInformation,
         private extensionPath: string,
-        private reporter: ITelemetryReporter
+        private reporter?: ITelemetryReporter
     ) {}
 
     public async DownloadAndInstallOmnisharp(

src/omnisharp/omnisharpLanguageServer.ts

@@ -154,7 +154,8 @@ export async function activateOmniSharpLanguageServer(
             eventStream,
             context.extension.packageJSON,
             platformInfo,
-            context.extension.extensionPath
+            context.extension.extensionPath,
+            reporter
         );
 
         await razorOmnisharpDownloader.DownloadAndInstallRazorOmnisharp(

src/packageManager/downloadAndInstallPackages.ts

@@ -26,6 +26,29 @@ export async function downloadAndInstallPackages(
     telemetryReporter?: ITelemetryReporter,
     token?: CancellationToken
 ): Promise<boolean> {
+    function sendInstallationFailureTelemetry(pkg: AbsolutePathPackage, installationStage: string, error: any): void {
+        if (!telemetryReporter) {
+            return;
+        }
+
+        const telemetryProperties: { [key: string]: string } = {
+            installStage: installationStage,
+            packageId: pkg.id,
+            isOptional: pkg.isOptional ? 'true' : 'false',
+        };
+
+        if (error instanceof NestedError && error.err instanceof PackageError) {
+            telemetryProperties['error.message'] = error.err.message;
+            telemetryProperties['error.packageUrl'] = error.err.pkg.url;
+        } else if (error instanceof PackageError) {
+            telemetryProperties['error.message'] = error.message;
+            telemetryProperties['error.packageUrl'] = error.pkg.url;
+        }
+
+        const eventName = pkg.isOptional ? 'OptionalPackageInstallationFailed' : 'PackageInstallationFailed';
+        telemetryReporter.sendTelemetryEvent(eventName, telemetryProperties);
+    }
+
     eventStream.post(new PackageInstallStart());
     for (const pkg of packages) {
         let installationStage = 'touchBeginFile';
@@ -64,24 +87,7 @@ export async function downloadAndInstallPackages(
             }
 
             // Send telemetry for the failure
-            if (telemetryReporter) {
-                const telemetryProperties: { [key: string]: string } = {
-                    installStage: installationStage,
-                    packageId: pkg.id,
-                    isOptional: pkg.isOptional ? 'true' : 'false',
-                };
-
-                if (error instanceof NestedError && error.err instanceof PackageError) {
-                    telemetryProperties['error.message'] = error.err.message;
-                    telemetryProperties['error.packageUrl'] = error.err.pkg.url;
-                } else if (error instanceof PackageError) {
-                    telemetryProperties['error.message'] = error.message;
-                    telemetryProperties['error.packageUrl'] = error.pkg.url;
-                }
-
-                const eventName = pkg.isOptional ? 'OptionalPackageInstallationFailed' : 'PackageInstallationFailed';
-                telemetryReporter.sendTelemetryEvent(eventName, telemetryProperties);
-            }
+            sendInstallationFailureTelemetry(pkg, installationStage, error);
 
             // If the package is optional, log and continue with the next package
             if (pkg.isOptional) {

src/razor/razorOmnisharpDownloader.ts

@@ -11,14 +11,16 @@ import { downloadAndInstallPackages } from '../packageManager/downloadAndInstall
 import { getRuntimeDependenciesPackages } from '../tools/runtimeDependencyPackageUtils';
 import { getAbsolutePathPackagesToInstall } from '../packageManager/getAbsolutePathPackagesToInstall';
 import { isValidDownload } from '../packageManager/isValidDownload';
+import { ITelemetryReporter } from '../shared/telemetryReporter';
 
 export class RazorOmnisharpDownloader {
     public constructor(
         private networkSettingsProvider: NetworkSettingsProvider,
         private eventStream: EventStream,
         private packageJSON: any,
         private platformInfo: PlatformInformation,
-        private extensionPath: string
+        private extensionPath: string,
+        private reporter?: ITelemetryReporter
     ) {}
 
     public async DownloadAndInstallRazorOmnisharp(version: string): Promise<boolean> {
@@ -39,7 +41,7 @@ export class RazorOmnisharpDownloader {
                     this.networkSettingsProvider,
                     this.eventStream,
                     isValidDownload,
-                    undefined
+                    this.reporter
                 )
             ) {
                 this.eventStream.post(new InstallationSuccess());

test/omnisharp/omnisharpUnitTests/omnisharpDownloader.test.ts

@@ -54,8 +54,7 @@ import { modernNetVersion } from '../../../src/omnisharp/omnisharpPackageCreator
                 eventStream,
                 testPackageJSON,
                 platformInfo,
-                extensionPath,
-                undefined as any
+                extensionPath
             );
             server = await MockHttpsServer.CreateMockHttpsServer();
             testZip = await TestZip.createTestZipAsync(createTestFile('Foo', 'foo.txt'));

test/omnisharp/omnisharpUnitTests/omnisharpManager.test.ts

@@ -245,8 +245,7 @@ function GetTestOmniSharpManager(
         eventStream,
         testPackageJSON,
         platformInfo,
-        extensionPath,
-        undefined as any
+        extensionPath
     );
     return new OmnisharpManager(downloader, platformInfo, serverUrl);
 }

test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts

@@ -87,8 +87,7 @@ describe(`${downloadAndInstallPackages.name}`, () => {
                 downloadablePackage,
                 networkSettingsProvider,
                 eventStream,
-                downloadValidator,
-                undefined
+                downloadValidator
             );
             for (const elem of testZip.files) {
                 const filePath = path.join(tmpDirPath, elem.path);
@@ -101,8 +100,7 @@ describe(`${downloadAndInstallPackages.name}`, () => {
                 downloadablePackage,
                 networkSettingsProvider,
                 eventStream,
-                downloadValidator,
-                undefined
+                downloadValidator
             );
             expect(await util.fileExists(path.join(tmpDirPath, 'install.Lock'))).toBe(true);
         });
@@ -121,8 +119,7 @@ describe(`${downloadAndInstallPackages.name}`, () => {
                 downloadablePackage,
                 networkSettingsProvider,
                 eventStream,
-                downloadValidator,
-                undefined
+                downloadValidator
             );
             expect(eventBus.getEvents()).toStrictEqual(eventsSequence);
         });
@@ -156,8 +153,7 @@ describe(`${downloadAndInstallPackages.name}`, () => {
                 downloadablePackage,
                 networkSettingsProvider,
                 eventStream,
-                downloadValidator,
-                undefined
+                downloadValidator
             );
             expect(eventBus.getEvents()).toStrictEqual(eventsSequence);
         });
@@ -184,8 +180,7 @@ describe(`${downloadAndInstallPackages.name}`, () => {
                 downloadablePackage,
                 networkSettingsProvider,
                 eventStream,
-                downloadValidator,
-                undefined
+                downloadValidator
             );
             expect(eventBus.getEvents()).toStrictEqual(eventsSequence);
         });
@@ -201,8 +196,7 @@ describe(`${downloadAndInstallPackages.name}`, () => {
                 notDownloadablePackage,
                 networkSettingsProvider,
                 eventStream,
-                downloadValidator,
-                undefined
+                downloadValidator
             );
             const obtainedEvents = eventBus.getEvents();
             expect(obtainedEvents[0]).toStrictEqual(eventsSequence[0]);
@@ -218,8 +212,7 @@ describe(`${downloadAndInstallPackages.name}`, () => {
                 notDownloadablePackage,
                 networkSettingsProvider,
                 eventStream,
-                downloadValidator,
-                undefined
+                downloadValidator
             );
             expect(await util.fileExists(path.join(tmpDirPath, 'install.Lock'))).toBe(false);
         });
@@ -240,8 +233,7 @@ describe(`${downloadAndInstallPackages.name}`, () => {
                 optionalPackage,
                 networkSettingsProvider,
                 eventStream,
-                downloadValidator,
-                undefined
+                downloadValidator
             );
             expect(result).toBe(true);
         });
@@ -266,8 +258,7 @@ describe(`${downloadAndInstallPackages.name}`, () => {
                 mixedPackages,
                 networkSettingsProvider,
                 eventStream,
-                downloadValidator,
-                undefined
+                downloadValidator
             );
             expect(result).toBe(true);
             expect(await util.fileExists(path.join(tmpInstallDir2.name, 'install.Lock'))).toBe(true);
@@ -285,13 +276,7 @@ describe(`${downloadAndInstallPackages.name}`, () => {
             ];
 
             eventBus.getEvents(); // Clear any previous events
-            await downloadAndInstallPackages(
-                optionalPackage,
-                networkSettingsProvider,
-                eventStream,
-                downloadValidator,
-                undefined
-            );
+            await downloadAndInstallPackages(optionalPackage, networkSettingsProvider, eventStream, downloadValidator);
             const obtainedEvents = eventBus.getEvents();
             const installationFailureEvent = obtainedEvents.find(
                 (event) => event instanceof InstallationFailure