Skip to content

Commit

Permalink
send errors to telemetry
Browse files Browse the repository at this point in the history
  • Loading branch information
@feloy
feloy committed Jan 30, 2024
1 parent c613edc commit 242d352
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 6 deletions.
26 changes: 24 additions & 2 deletions packages/backend/src/managers/applicationManager.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const mocks = vi.hoisted(() => {
deleteContainerMock: vi.fn(),
inspectContainerMock: vi.fn(),
logUsageMock: vi.fn(),
logErrorMock: vi.fn(),
};
});
vi.mock('../models/AIConfig', () => ({
Expand Down Expand Up @@ -155,6 +156,7 @@ describe('pullApplication', () => {
} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
doDownloadModelWrapperSpy = vi.spyOn(manager, 'doDownloadModelWrapper');
Expand Down Expand Up @@ -302,6 +304,7 @@ describe('doCheckout', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
await manager.doCheckout('repo', 'folder', taskUtils);
Expand Down Expand Up @@ -332,6 +335,7 @@ describe('doCheckout', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
await manager.doCheckout('repo', 'folder', taskUtils);
Expand All @@ -357,6 +361,7 @@ describe('getConfiguration', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
vi.spyOn(fs, 'existsSync').mockReturnValue(false);
Expand All @@ -373,6 +378,7 @@ describe('getConfiguration', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
Expand Down Expand Up @@ -410,6 +416,7 @@ describe('downloadModel', () => {
{ isModelOnDisk: isModelOnDiskMock } as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
const doDownloadModelWrapperMock = vi
Expand Down Expand Up @@ -449,6 +456,7 @@ describe('downloadModel', () => {
} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
await manager.downloadModel(
Expand Down Expand Up @@ -498,6 +506,7 @@ describe('filterContainers', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
const containers = manager.filterContainers(aiConfig);
Expand Down Expand Up @@ -536,6 +545,7 @@ describe('filterContainers', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
const containers = manager.filterContainers(aiConfig);
Expand Down Expand Up @@ -584,6 +594,7 @@ describe('filterContainers', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
const containers = manager.filterContainers(aiConfig);
Expand All @@ -602,6 +613,7 @@ describe('getRandomName', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
const randomName = manager.getRandomName('base');
Expand All @@ -616,6 +628,7 @@ describe('getRandomName', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
const randomName = manager.getRandomName('');
Expand All @@ -641,23 +654,26 @@ describe('buildImages', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
test('setTaskState should be called with error if context does not exist', async () => {
test('setTaskState should be called with error and telemetry seent if context does not exist', async () => {
vi.spyOn(fs, 'existsSync').mockReturnValue(false);
mocks.listImagesMock.mockRejectedValue([]);
await expect(manager.buildImages(containers, 'config', taskUtils)).rejects.toThrow(
'Context configured does not exist.',
);
expect(mocks.logErrorMock).toHaveBeenCalled();
});
test('setTaskState should be called with error if buildImage executon fails', async () => {
test('setTaskState should be called with error and telemetry sent if buildImage executon fails', async () => {
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
mocks.builImageMock.mockRejectedValue('error');
mocks.listImagesMock.mockRejectedValue([]);
await expect(manager.buildImages(containers, 'config', taskUtils)).rejects.toThrow(
'Something went wrong while building the image: error',
);
expect(setTaskStateMock).toBeCalledWith('container1', 'error');
expect(mocks.logErrorMock).toHaveBeenCalled();
});
test('setTaskState should be called with error if unable to find the image after built', async () => {
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
Expand Down Expand Up @@ -713,6 +729,7 @@ describe('createPod', async () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
test('throw an error if there is no sample image', async () => {
Expand Down Expand Up @@ -770,6 +787,7 @@ describe('createApplicationPod', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
const images = [imageInfo1, imageInfo2];
Expand Down Expand Up @@ -825,6 +843,7 @@ describe('doDownloadModelWrapper', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
test('returning model path if model has been downloaded', async () => {
Expand Down Expand Up @@ -879,6 +898,7 @@ describe('restartContainerWhenModelServiceIsUp', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
test('restart container if endpoint is alive', async () => {
Expand All @@ -896,6 +916,7 @@ describe('runApplication', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
const pod: PodInfo = {
Expand Down Expand Up @@ -953,6 +974,7 @@ describe('createAndAddContainersToPod', () => {
{} as unknown as ModelsManager,
{
logUsage: mocks.logUsageMock,
logError: mocks.logErrorMock,
} as unknown as TelemetryLogger,
);
const pod: PodInfo = {
Expand Down
9 changes: 9 additions & 0 deletions packages/backend/src/managers/applicationManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ export class ApplicationManager {
await timeout(5000);
await this.restartContainerWhenModelServiceIsUp(engineId, modelServiceEndpoint, container).catch(
(error: unknown) => {
this.telemetry.logError('recipe.pull', { message: 'error monitoring endpoint', error: error });
console.error('Error monitoring endpoint', error);
},
);
Expand All @@ -185,6 +186,7 @@ export class ApplicationManager {
state: 'error',
name: 'Creating application',
});
this.telemetry.logError('recipe.pull', { message: 'error creating pod', error: e });
throw e;
}

Expand All @@ -204,6 +206,7 @@ export class ApplicationManager {
state: 'error',
name: 'Creating application',
});
this.telemetry.logError('recipe.pull', { message: 'error adding containers to pod', error: e });
throw e;
}

Expand Down Expand Up @@ -352,6 +355,7 @@ export class ApplicationManager {
if (!fs.existsSync(context)) {
console.error('The context provided does not exist.');
taskUtil.setTaskState(container.name, 'error');
this.telemetry.logError('recipe.pull', { message: 'configured context does not exist' });
throw new Error('Context configured does not exist.');
}

Expand All @@ -367,13 +371,15 @@ export class ApplicationManager {
// todo: do something with the event
if (event === 'error' || (event === 'finish' && data !== '')) {
console.error('Something went wrong while building the image: ', data);
this.telemetry.logError('recipe.pull', { message: 'error building image' });
taskUtil.setTaskState(container.name, 'error');
}
},
buildOptions,
)
.catch((err: unknown) => {
console.error('Something went wrong while building the image: ', err);
this.telemetry.logError('recipe.pull', { message: 'error building image', error: err });
taskUtil.setTaskState(container.name, 'error');
throw new Error(`Something went wrong while building the image: ${String(err)}`);
});
Expand Down Expand Up @@ -432,6 +438,7 @@ export class ApplicationManager {
} catch (e) {
loadingConfiguration.state = 'error';
taskUtil.setTask(loadingConfiguration);
this.telemetry.logError('recipe.pull', { message: 'error loading configuration', error: e });
throw e;
}

Expand All @@ -445,6 +452,7 @@ export class ApplicationManager {
// Mark as failure.
loadingConfiguration.state = 'error';
taskUtil.setTask(loadingConfiguration);
this.telemetry.logError('recipe.pull', { message: 'no container available' });
throw new Error('No containers available.');
}

Expand Down Expand Up @@ -485,6 +493,7 @@ export class ApplicationManager {
'model-pulling': model.id,
},
});
this.telemetry.logError('model.download', { message: 'error downloading model', error: e });
throw e;
}
} else {
Expand Down
10 changes: 9 additions & 1 deletion packages/backend/src/managers/playground.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
type Webview,
type ProviderContainerConnection,
type ImageInfo,
type TelemetryLogger,
} from '@podman-desktop/api';
import type { LocalModelInfo } from '@shared/src/models/ILocalModelInfo';
import type { ModelResponse } from '@shared/src/models/IModelResponse';
Expand Down Expand Up @@ -51,7 +52,10 @@ export class PlayGroundManager {
private playgrounds: Map<string, PlaygroundState>;
private queries: Map<number, QueryState>;

constructor(private webview: Webview) {
constructor(
private webview: Webview,
private telemetry: TelemetryLogger,
) {
this.playgrounds = new Map<string, PlaygroundState>();
this.queries = new Map<number, QueryState>();
}
Expand Down Expand Up @@ -99,6 +103,7 @@ export class PlayGroundManager {
const connection = findFirstProvider();
if (!connection) {
await this.setPlaygroundStatus(modelId, 'error');
this.telemetry.logError('playground.start', { message: 'unable to find an engine to start playground' });
throw new Error('Unable to find an engine to start playground');
}

Expand All @@ -108,6 +113,7 @@ export class PlayGroundManager {
image = await this.selectImage(connection, PLAYGROUND_IMAGE);
if (!image) {
await this.setPlaygroundStatus(modelId, 'error');
this.telemetry.logError('playground.start', { message: 'unable to find playground image' });
throw new Error(`Unable to find ${PLAYGROUND_IMAGE} image`);
}
}
Expand Down Expand Up @@ -168,6 +174,7 @@ export class PlayGroundManager {
})
.catch(async (error: unknown) => {
console.error(error);
this.telemetry.logError('playground.stop', { message: 'error stopping playground', error: error });
await this.setPlaygroundStatus(modelId, 'error');
});
}
Expand Down Expand Up @@ -211,6 +218,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' });
throw new Error('query not found in state');
}
q.response = result as ModelResponse;
Expand Down
11 changes: 9 additions & 2 deletions packages/backend/src/studio-api-impl.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ vi.mock('@podman-desktop/api', () => {

const mocks = vi.hoisted(() => ({
withProgressMock: vi.fn(),
logUsageMock: vi.fn(),
}));

vi.mock('@podman-desktop/api', async () => {
Expand Down Expand Up @@ -98,7 +99,9 @@ beforeEach(async () => {
{} as unknown as PlayGroundManager,
catalogManager,
{} as unknown as ModelsManager,
{} as TelemetryLogger,
{
logUsage: mocks.logUsageMock,
} as unknown as TelemetryLogger,
);
vi.resetAllMocks();
vi.mock('node:fs');
Expand Down Expand Up @@ -149,7 +152,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', async () => {
test('expect pull application to call the withProgress api method and send telemetry', async () => {
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
vi.spyOn(fs.promises, 'readFile').mockResolvedValue(JSON.stringify(userContent));

Expand All @@ -158,4 +161,8 @@ test('expect pull application to call the withProgress api method', async () =>
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',
});
});
2 changes: 1 addition & 1 deletion packages/backend/src/studio.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export class Studio {
const gitManager = new GitManager();
const taskRegistry = new TaskRegistry();
const recipeStatusRegistry = new RecipeStatusRegistry(taskRegistry, this.#panel.webview);
this.playgroundManager = new PlayGroundManager(this.#panel.webview);
this.playgroundManager = new PlayGroundManager(this.#panel.webview, this.telemetry);
// Create catalog manager, responsible for loading the catalog files and watching for changes
this.catalogManager = new CatalogManager(appUserDirectory, this.#panel.webview);
this.modelsManager = new ModelsManager(appUserDirectory, this.#panel.webview, this.catalogManager, this.telemetry);
Expand Down

0 comments on commit 242d352

Please sign in to comment.