move telemetry out of studio-api-impl

packages/backend/src/managers/applicationManager.spec.ts

@@ -222,6 +222,11 @@ describe('pullApplication', () => {
     }
     expect(doDownloadModelWrapperSpy).toHaveBeenCalledOnce();
     expect(mocks.builImageMock).toHaveBeenCalledOnce();
+    expect(mocks.logUsageMock).toHaveBeenNthCalledWith(1, 'model.download', { 'model.id': 'model1' });
+    expect(mocks.logUsageMock).toHaveBeenNthCalledWith(2, 'recipe.pull', {
+      'recipe.id': 'recipe1',
+      'recipe.name': 'Recipe 1',
+    });
   });
   test('pullApplication should not clone repository if folder already exists locally', async () => {
     mockForPullApplication({

packages/backend/src/managers/applicationManager.ts

@@ -101,6 +101,7 @@ export class ApplicationManager {
     const podInfo = await this.createApplicationPod(images, modelPath, taskUtil);
 
     await this.runApplication(podInfo, taskUtil);
+    this.telemetry.logUsage('recipe.pull', { 'recipe.id': recipe.id, 'recipe.name': recipe.name });
   }
 
   async runApplication(podInfo: PodInfo, taskUtil: RecipeStatusUtils) {

packages/backend/src/managers/modelsManager.spec.ts

@@ -32,6 +32,7 @@ const mocks = vi.hoisted(() => {
   return {
     showErrorMessageMock: vi.fn(),
     logUsageMock: vi.fn(),
+    logErrorMock: vi.fn(),
   };
 });
 
@@ -56,6 +57,7 @@ let setTaskStateMock: MockInstance;
 
 const telemetryLogger = {
   logUsage: mocks.logUsageMock,
+  logError: mocks.logErrorMock,
 } as unknown as TelemetryLogger;
 
 beforeEach(() => {
@@ -342,6 +344,7 @@ test('deleteLocalModel fails to delete the model folder', async () => {
     ],
   });
   expect(mocks.showErrorMessageMock).toHaveBeenCalledOnce();
+  expect(mocks.logErrorMock).toHaveBeenCalled();
 });
 
 describe('downloadModel', () => {

packages/backend/src/managers/modelsManager.ts

@@ -150,11 +150,16 @@ export class ModelsManager {
     const modelDir = this.getLocalModelFolder(modelId);
     this.#deleted.add(modelId);
     await this.sendModelsInfo();
-    this.telemetry.logUsage('model.delete', { 'model.id': modelId });
     try {
       await fs.promises.rm(modelDir, { recursive: true });
       this.#localModels.delete(modelId);
+      this.telemetry.logUsage('model.delete', { 'model.id': modelId });
     } catch (err: unknown) {
+      this.telemetry.logError('model.delete', {
+        'model.id': modelId,
+        message: 'error deleting model from disk',
+        error: err,
+      });
       await podmanDesktopApi.window.showErrorMessage(`Error deleting model ${modelId}. ${String(err)}`);
     } finally {
       this.#deleted.delete(modelId);
@@ -175,8 +180,9 @@ export class ModelsManager {
       });
 
       try {
+        const result = await this.doDownloadModelWrapper(model.id, model.url, taskUtil);
         this.telemetry.logUsage('model.download', { 'model.id': model.id });
-        return await this.doDownloadModelWrapper(model.id, model.url, taskUtil);
+        return result;
       } catch (e) {
         console.error(e);
         taskUtil.setTask({
@@ -187,7 +193,11 @@ export class ModelsManager {
             'model-pulling': model.id,
           },
         });
-        this.telemetry.logError('model.download', { message: 'error downloading model', error: e });
+        this.telemetry.logError('model.download', {
+          'model.id': model.id,
+          message: 'error downloading model',
+          error: e,
+        });
         throw e;
       }
     } else {

packages/backend/src/managers/playground.spec.ts

@@ -29,6 +29,7 @@ const mocks = vi.hoisted(() => ({
   stopContainer: vi.fn(),
   getFreePort: vi.fn(),
   containerRegistrySubscribeMock: vi.fn(),
+  logUsage: vi.fn(),
   logError: vi.fn(),
 }));
 
@@ -66,6 +67,7 @@ beforeEach(() => {
     } as unknown as Webview,
     containerRegistryMock,
     {
+      logUsage: mocks.logUsage,
       logError: mocks.logError,
     } as unknown as TelemetryLogger,
   );

packages/backend/src/managers/playground.ts

@@ -109,7 +109,10 @@ export class PlayGroundManager {
     const connection = findFirstProvider();
     if (!connection) {
       this.setPlaygroundStatus(modelId, 'error');
-      this.telemetry.logError('playground.start', { message: 'unable to find an engine to start playground' });
+      this.telemetry.logError('playground.start', {
+        'model.id': modelId,
+        message: 'unable to find an engine to start playground',
+      });
       throw new Error('Unable to find an engine to start playground');
     }
 
@@ -119,7 +122,10 @@ export class PlayGroundManager {
       image = await this.selectImage(connection, PLAYGROUND_IMAGE);
       if (!image) {
         this.setPlaygroundStatus(modelId, 'error');
-        this.telemetry.logError('playground.start', { message: 'unable to find playground image' });
+        this.telemetry.logError('playground.start', {
+          'model.id': modelId,
+          message: 'unable to find playground image',
+        });
         throw new Error(`Unable to find ${PLAYGROUND_IMAGE} image`);
       }
     }
@@ -178,6 +184,7 @@ export class PlayGroundManager {
       modelId,
     });
 
+    this.telemetry.logUsage('playground.start', { 'model.id': modelId });
     return result.id;
   }
 
@@ -196,13 +203,19 @@ export class PlayGroundManager {
       .catch(async (error: unknown) => {
         console.error(error);
         this.setPlaygroundStatus(modelId, 'error');
-        this.telemetry.logError('playground.stop', { message: 'error stopping playground', error: error });
+        this.telemetry.logError('playground.stop', {
+          'model.id': modelId,
+          message: 'error stopping playground',
+          error: error,
+        });
       });
+    this.telemetry.logUsage('playground.stop', { 'model.id': modelId });
   }
 
   async askPlayground(modelInfo: LocalModelInfo, prompt: string): Promise<number> {
     const state = this.playgrounds.get(modelInfo.id);
     if (state?.container === undefined) {
+      this.telemetry.logError('playground.ask', { 'model.id': modelInfo.id, message: 'model is not running' });
       throw new Error('model is not running');
     }
 
@@ -239,7 +252,7 @@ export class PlayGroundManager {
           const result = JSON.parse(resBody);
           const q = this.queries.get(query.id);
           if (!q) {
-            this.telemetry.logError('playground.ask', { message: 'error reading response' });
+            this.telemetry.logError('playground.ask', { 'model.id': modelInfo.id, message: 'error reading response' });
             throw new Error('query not found in state');
           }
           q.error = undefined;
@@ -261,6 +274,7 @@ export class PlayGroundManager {
 
     this.queries.set(query.id, query);
     this.sendQueriesState();
+    this.telemetry.logUsage('playground.ask', { 'model.id': modelInfo.id });
     return query.id;
   }
 

packages/backend/src/studio-api-impl.spec.ts

@@ -60,7 +60,6 @@ vi.mock('@podman-desktop/api', () => {
 
 const mocks = vi.hoisted(() => ({
   withProgressMock: vi.fn(),
-  logUsageMock: vi.fn(),
 }));
 
 vi.mock('@podman-desktop/api', async () => {
@@ -99,9 +98,7 @@ beforeEach(async () => {
     {} as unknown as PlayGroundManager,
     catalogManager,
     {} as unknown as ModelsManager,
-    {
-      logUsage: mocks.logUsageMock,
-    } as unknown as TelemetryLogger,
+    {} as TelemetryLogger,
   );
   vi.resetAllMocks();
   vi.mock('node:fs');
@@ -152,7 +149,7 @@ test('expect correct model is returned with valid id when the user catalog is va
   expect(model.url).toEqual('https://model1.example.com');
 });
 
-test('expect pull application to call the withProgress api method and send telemetry', async () => {
+test('expect pull application to call the withProgress api method', async () => {
   vi.spyOn(fs, 'existsSync').mockReturnValue(true);
   vi.spyOn(fs.promises, 'readFile').mockResolvedValue(JSON.stringify(userContent));
 
@@ -161,8 +158,4 @@ test('expect pull application to call the withProgress api method and send telem
   await catalogManager.loadCatalog();
   await studioApiImpl.pullApplication('recipe 1');
   expect(mocks.withProgressMock).toHaveBeenCalledOnce();
-  expect(mocks.logUsageMock).toHaveBeenNthCalledWith(1, 'recipe.pull', {
-    'recipe.id': 'recipe 1',
-    'recipe.name': 'Recipe 1',
-  });
 });

packages/backend/src/studio-api-impl.ts

@@ -72,8 +72,6 @@ export class StudioApiImpl implements StudioAPI {
     const recipe = this.catalogManager.getRecipes().find(recipe => recipe.id === recipeId);
     if (!recipe) throw new Error('Not found');
 
-    await this.telemetryLogUsage('recipe.pull', { 'recipe.id': recipe.id, 'recipe.name': recipe.name });
-
     // the user should have selected one model, we use the first one for the moment
     const modelId = recipe.models[0];
     const model = await this.getModelById(modelId);
@@ -93,18 +91,15 @@ export class StudioApiImpl implements StudioAPI {
   }
 
   async startPlayground(modelId: string): Promise<void> {
-    await this.telemetryLogUsage('playground.start', { 'model.id': modelId });
     const modelPath = this.modelsManager.getLocalModelPath(modelId);
     await this.playgroundManager.startPlayground(modelId, modelPath);
   }
 
   async stopPlayground(modelId: string): Promise<void> {
-    await this.telemetryLogUsage('playground.stop', { 'model.id': modelId });
     await this.playgroundManager.stopPlayground(modelId);
   }
 
   async askPlayground(modelId: string, prompt: string): Promise<number> {
-    await this.telemetryLogUsage('playground.ask', { 'model.id': modelId });
     const localModelInfo = this.modelsManager.getLocalModelInfo(modelId);
     return this.playgroundManager.askPlayground(localModelInfo, prompt);
   }