fix(telemetry): anonymise properly model id (#1654)

* fix(telemetry): anonymise properly model id Signed-off-by: axel7083 <[email protected]> * fix: prettier&linter Signed-off-by: axel7083 <[email protected]> * fix: uses sha512 instead of model-id Signed-off-by: axel7083 <[email protected]> * test: ensuring getHash works Signed-off-by: axel7083 <[email protected]> * fix: model manager test Signed-off-by: axel7083 <[email protected]> --------- Signed-off-by: axel7083 <[email protected]>
containers · Sep 13, 2024 · 6a229f4 · 6a229f4
1 parent 2bfa0b1
commit 6a229f4
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 14 deletions.
diff --git a/packages/backend/src/managers/inference/inferenceManager.ts b/packages/backend/src/managers/inference/inferenceManager.ts
@@ -32,6 +32,7 @@ import type { InferenceProviderRegistry } from '../../registries/InferenceProvid
 import type { InferenceProvider } from '../../workers/provider/InferenceProvider';
 import type { ModelInfo } from '@shared/src/models/IModelInfo';
 import type { CatalogManager } from '../catalogManager';
+import { getHash } from '../../utils/sha';
 
 export class InferenceManager extends Publisher<InferenceServer[]> implements Disposable {
   // Inference server map (containerId -> InferenceServer)
@@ -231,7 +232,7 @@ export class InferenceManager extends Publisher<InferenceServer[]> implements Di
 
     // Log usage
     this.telemetry.logUsage('inference.start', {
-      models: config.modelsInfo.map(model => model.id),
+      models: config.modelsInfo.map(model => getHash(model.id)),
     });
 
     this.notify();

diff --git a/packages/backend/src/managers/modelsManager.spec.ts b/packages/backend/src/managers/modelsManager.spec.ts
@@ -485,7 +485,7 @@ test('deleteModel deletes the model folder', async () => {
       },
     ],
   });
-  expect(mocks.logUsageMock).toHaveBeenNthCalledWith(1, 'model.delete', { 'model.id': 'model-id-1' });
+  expect(mocks.logUsageMock).toHaveBeenNthCalledWith(1, 'model.delete', { 'model.id': expect.any(String) });
 });
 
 describe('deleting models', () => {

diff --git a/packages/backend/src/managers/modelsManager.ts b/packages/backend/src/managers/modelsManager.ts
@@ -33,7 +33,7 @@ import { Uploader } from '../utils/uploader';
 import { deleteRemoteModel, getLocalModelFile, isModelUploaded } from '../utils/modelsUtils';
 import { getPodmanMachineName } from '../utils/podman';
 import type { CancellationTokenRegistry } from '../registries/CancellationTokenRegistry';
-import { hasValidSha } from '../utils/sha';
+import { getHash, hasValidSha } from '../utils/sha';
 import type { GGUFParseOutput } from '@huggingface/gguf';
 import { gguf } from '@huggingface/gguf';
 import type { PodmanConnection } from './podmanConnection';
@@ -200,7 +200,7 @@ export class ModelsManager implements Disposable {
       }
       await fs.promises.rm(modelPath, { recursive: true, force: true, maxRetries: 3 });
 
-      this.telemetry.logUsage('model.delete', { 'model.id': modelId });
+      this.telemetry.logUsage('model.delete', { 'model.id': getHash(modelId) });
       model.file = model.state = undefined;
     } catch (err: unknown) {
       this.telemetry.logError('model.delete', {
@@ -452,7 +452,7 @@ export class ModelsManager implements Disposable {
 
     const before = performance.now();
     const data: Record<string, unknown> = {
-      'model-id': model.url ? modelId : 'imported', // filter imported models
+      'model-id': getHash(modelId),
     };
 
     try {

diff --git a/packages/backend/src/managers/playgroundV2Manager.ts b/packages/backend/src/managers/playgroundV2Manager.ts
@@ -35,6 +35,7 @@ import { withDefaultConfiguration } from '../utils/inferenceUtils';
 import { getRandomString } from '../utils/randomUtils';
 import type { TaskRegistry } from '../registries/TaskRegistry';
 import type { CancellationTokenRegistry } from '../registries/CancellationTokenRegistry';
+import { getHash } from '../utils/sha';
 
 export class PlaygroundV2Manager implements Disposable {
   #conversationRegistry: ConversationRegistry;
@@ -53,7 +54,7 @@ export class PlaygroundV2Manager implements Disposable {
     const conversation = this.#conversationRegistry.get(conversationId);
     this.telemetry.logUsage('playground.delete', {
       totalMessages: conversation.messages.length,
-      modelId: conversation.modelId,
+      modelId: getHash(conversation.modelId),
     });
     this.#conversationRegistry.deleteConversation(conversationId);
   }
@@ -66,7 +67,7 @@ export class PlaygroundV2Manager implements Disposable {
 
     const telemetry: Record<string, unknown> = {
       hasName: !!name,
-      modelId: model.id,
+      modelId: getHash(model.id),
     };
     this.createPlayground(name, model, trackingId)
       .then((playgroundId: string) => {
@@ -150,7 +151,7 @@ export class PlaygroundV2Manager implements Disposable {
       timestamp: Date.now(),
     } as SystemPrompt);
     this.telemetry.logUsage('playground.system-prompt.create', {
-      modelId: this.#conversationRegistry.get(conversationId).modelId,
+      modelId: getHash(this.#conversationRegistry.get(conversationId).modelId),
     });
   }
 
@@ -166,7 +167,7 @@ export class PlaygroundV2Manager implements Disposable {
     if (content === undefined || content.length === 0) {
       this.#conversationRegistry.removeMessage(conversationId, conversation.messages[0].id);
       this.telemetry.logUsage('playground.system-prompt.delete', {
-        modelId: conversation.modelId,
+        modelId: getHash(conversation.modelId),
       });
       return;
     }
@@ -178,7 +179,7 @@ export class PlaygroundV2Manager implements Disposable {
         content,
       });
       this.telemetry.logUsage('playground.system-prompt.update', {
-        modelId: conversation.modelId,
+        modelId: getHash(conversation.modelId),
       });
     } else {
       throw new Error('Cannot change system prompt on started conversation.');
@@ -227,7 +228,7 @@ export class PlaygroundV2Manager implements Disposable {
       conversationId: conversationId,
       ...options,
       promptLength: userInput.length,
-      modelId: modelInfo.id,
+      modelId: getHash(modelInfo.id),
     };
 
     // create an abort controller
@@ -312,7 +313,7 @@ export class PlaygroundV2Manager implements Disposable {
     this.#conversationRegistry.completeMessage(conversationId, messageId);
     this.telemetry.logUsage('playground.message.complete', {
       duration: Date.now() - start,
-      modelId: conversation.modelId,
+      modelId: getHash(conversation.modelId),
     });
   }
 

diff --git a/packages/backend/src/utils/sha.spec.ts b/packages/backend/src/utils/sha.spec.ts
@@ -15,9 +15,9 @@
  *
  * SPDX-License-Identifier: Apache-2.0
  ***********************************************************************/
-import { beforeEach, expect, test, vi } from 'vitest';
+import { beforeEach, expect, test, vi, describe } from 'vitest';
 import * as fs from 'node:fs';
-import { hasValidSha } from './sha';
+import { getHash, hasValidSha } from './sha';
 import { Readable } from 'node:stream';
 
 beforeEach(() => {
@@ -50,3 +50,19 @@ test('return false if file has different hash of the expected one', () => {
   const isValid = hasValidSha('file', 'fakeSha');
   expect(isValid).toBeTruthy();
 });
+
+describe('sha512', () => {
+  test('basic string', () => {
+    const result = getHash('hello-world');
+    expect(result).toBe(
+      '6aeefc29122a3962c90ef834f6caad0033bffcd62941b7a6205a695cc39e2767db7778a7ad76d173a083b9e14b210dc0212923f481b285c784ab1fe340d7ff4d',
+    );
+  });
+
+  test('very long string', () => {
+    const result = getHash('x'.repeat(1024));
+    expect(result).toBe(
+      'fa41ec783342d4c23e7b6550f1e96e32a16269e390449e5fdda60f05611ecb08dd56a5b8cde90024b7da934cdb9a9cc8c8a310eb20e25227699bbf6518e23360',
+    );
+  });
+});
diff --git a/packages/backend/src/utils/sha.ts b/packages/backend/src/utils/sha.ts
@@ -27,3 +27,7 @@ export async function hasValidSha(filePath: string, expectedSha: string): Promis
   const actualSha = checkSum.digest('hex');
   return actualSha === expectedSha;
 }
+
+export function getHash(content: string): string {
+  return crypto.createHash('sha512').update(content).digest('hex');
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -485,7 +485,7 @@ test('deleteModel deletes the model folder', async () => { @@
           },
         ],
       });
-      expect(mocks.logUsageMock).toHaveBeenNthCalledWith(1, 'model.delete', { 'model.id': 'model-id-1' });
+      expect(mocks.logUsageMock).toHaveBeenNthCalledWith(1, 'model.delete', { 'model.id': expect.any(String) });
     });
     describe('deleting models', () => {
@@ Expand Down @@