Merge pull request #2478 from RedisInsight/be/feature/RI-4811-add-telemtry-event

vlad-dargel · web-flow · commit f18d06585c8c · 2023-09-07T15:00:10.000+02:00
#RI-4811 - collect Client information when connecting to a database
diff --git a/redisinsight/api/src/__mocks__/database-info.ts b/redisinsight/api/src/__mocks__/database-info.ts
@@ -0,0 +1,60 @@
+export const mockRedisClientList = 'id=29 addr=172.17.0.1:60702 laddr=172.17.0.4:6379 fd=20 '
+  + 'name=redisinsight-common-f9d59780 age=319 idle=0 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=26 '
+  + 'qbuf-free=40928 argv-mem=10 obl=0 oll=0 omem=0 tot-mem=61466 events=r cmd=client user=default resp=2 redir=-1\n'
+  + 'id=31 addr=172.17.0.1:63984 laddr=172.17.0.4:6379 fd=22 name=redisinsight-cli-bc36ecf0 age=15 '
+  + 'idle=15 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=0 qbuf-free=0 argv-mem=0 obl=0 oll=0 omem=0 '
+  + 'tot-mem=20512 events=r cmd=client user=default redir=-1\n';
+
+export const mockRedisClientListResult = [
+  {
+    addr: '172.17.0.1:60702',
+    age: '319',
+    'argv-mem': '10',
+    cmd: 'client',
+    db: '0',
+    events: 'r',
+    fd: '20',
+    flags: 'N',
+    id: '29',
+    idle: '0',
+    laddr: '172.17.0.4:6379',
+    multi: '-1',
+    name: 'redisinsight-common-f9d59780',
+    obl: '0',
+    oll: '0',
+    omem: '0',
+    psub: '0',
+    qbuf: '26',
+    'qbuf-free': '40928',
+    redir: '-1',
+    sub: '0',
+    'tot-mem': '61466',
+    user: 'default',
+    resp: '2',
+  },
+  {
+    addr: '172.17.0.1:63984',
+    age: '15',
+    'argv-mem': '0',
+    cmd: 'client',
+    db: '0',
+    events: 'r',
+    fd: '22',
+    flags: 'N',
+    id: '31',
+    idle: '15',
+    laddr: '172.17.0.4:6379',
+    multi: '-1',
+    name: 'redisinsight-cli-bc36ecf0',
+    obl: '0',
+    oll: '0',
+    omem: '0',
+    psub: '0',
+    qbuf: '0',
+    'qbuf-free': '0',
+    redir: '-1',
+    sub: '0',
+    'tot-mem': '20512',
+    user: 'default',
+  },
+];
diff --git a/redisinsight/api/src/__mocks__/databases.ts b/redisinsight/api/src/__mocks__/databases.ts
@@ -17,6 +17,7 @@ import {
 } from 'src/__mocks__/ssh';
 import { CloudDatabaseDetailsEntity } from 'src/modules/cloud/database/entities/cloud-database-details.entity';
 import { mockCloudDatabaseDetails, mockCloudDatabaseDetailsEntity } from 'src/__mocks__/cloud-database';
+import { mockRedisClientListResult } from 'src/__mocks__/database-info';
 
 export const mockDatabaseId = 'a77b23c1-7816-4ea4-b61f-d37795a0f805-db-id';
 
@@ -239,6 +240,7 @@ export const mockDatabaseInfoProvider = jest.fn(() => ({
   determineSentinelMasterGroups: jest.fn().mockReturnValue([mockSentinelMasterDto]),
   determineClusterNodes: jest.fn().mockResolvedValue(mockClusterNodes),
   getRedisGeneralInfo: jest.fn().mockResolvedValueOnce(mockRedisGeneralInfo),
+  getClientListInfo: jest.fn().mockReturnValue(mockRedisClientListResult),
 }));
 
 export const mockDatabaseOverviewProvider = jest.fn(() => ({
@@ -259,4 +261,5 @@ export const mockDatabaseAnalytics = jest.fn(() => ({
   sendInstanceAddFailedEvent: jest.fn(),
   sendInstanceEditedEvent: jest.fn(),
   sendInstanceDeletedEvent: jest.fn(),
+  sendDatabaseConnectedClientListEvent: jest.fn(),
 }));
diff --git a/redisinsight/api/src/__mocks__/index.ts b/redisinsight/api/src/__mocks__/index.ts
@@ -32,4 +32,5 @@ export * from './cloud-user';
 export * from './cloud-common';
 export * from './session';
 export * from './cloud-session';
+export * from './database-info';
 export * from './cloud-job';
diff --git a/redisinsight/api/src/constants/telemetry-events.ts b/redisinsight/api/src/constants/telemetry-events.ts
@@ -5,6 +5,7 @@ export enum TelemetryEvents {
   AnalyticsPermission = 'ANALYTICS_PERMISSION',
   SettingsScanThresholdChanged = 'SETTINGS_KEYS_TO_SCAN_CHANGED',
   SettingsWorkbenchPipelineChanged = 'SETTINGS_WORKBENCH_PIPELINE_CHANGED',
+  DatabaseConnectedClientList = 'DATABASE_CONNECTED_CLIENT_LIST',
 
   // Events for redis instances
   RedisInstanceAdded = 'CONFIG_DATABASES_DATABASE_ADDED',
diff --git a/redisinsight/api/src/modules/database/database-connection.service.spec.ts b/redisinsight/api/src/modules/database/database-connection.service.spec.ts
@@ -1,5 +1,6 @@
 import { UnauthorizedException } from '@nestjs/common';
 import { Test, TestingModule } from '@nestjs/testing';
+import { get } from 'lodash';
 import {
   mockCommonClientMetadata,
   mockDatabase,
@@ -13,6 +14,7 @@ import {
   mockDatabaseRecommendationService,
   MockType,
   mockRedisGeneralInfo,
+  mockRedisClientListResult,
 } from 'src/__mocks__';
 import { DatabaseAnalytics } from 'src/modules/database/database.analytics';
 import { DatabaseService } from 'src/modules/database/database.service';
@@ -122,6 +124,23 @@ describe('DatabaseConnectionService', () => {
       expect(databaseInfoProvider.determineDatabaseServer).toHaveBeenCalled();
       expect(databaseInfoProvider.determineDatabaseModules).toHaveBeenCalled();
     });
+
+    it('should call getClientListInfo', async () => {
+      expect(await service.connect(mockCommonClientMetadata)).toEqual(undefined);
+
+      expect(databaseInfoProvider.getClientListInfo).toHaveBeenCalled();
+      expect(analytics.sendDatabaseConnectedClientListEvent).toHaveBeenCalledWith(
+        mockDatabase.id,
+        {
+          clients: mockRedisClientListResult.map((c) => ({
+            version: mockRedisGeneralInfo.version,
+            resp: get(c, 'resp', 'n/a'),
+            libVer: get(c, 'lib-ver', 'n/a'),
+            libName: get(c, 'lib-name', 'n/a'),
+          })),
+        },
+      );
+    });
   });
 
   describe('getOrCreateClient', () => {
diff --git a/redisinsight/api/src/modules/database/database-connection.service.ts b/redisinsight/api/src/modules/database/database-connection.service.ts
@@ -89,6 +89,8 @@ export class DatabaseConnectionService {
       { client, databaseId: clientMetadata.databaseId },
     );
 
+    this.collectClientInfo(clientMetadata, client, generalInfo?.version);
+
     this.logger.log(`Succeed to connect to database ${clientMetadata.databaseId}`);
   }
 
@@ -159,4 +161,25 @@ export class DatabaseConnectionService {
       throw exception;
     }
   }
+
+  private async collectClientInfo(clientMetadata: ClientMetadata, client: any, version?: string) {
+    try {
+      const intVersion = parseInt(version, 10) || 0;
+      const clients = await this.databaseInfoProvider.getClientListInfo(client) || [];
+
+      this.analytics.sendDatabaseConnectedClientListEvent(
+        clientMetadata.databaseId,
+        {
+          clients: clients.map((c) => ({
+            version: version || 'n/a',
+            resp: intVersion < 7 ? undefined : c?.['resp'] || 'n/a',
+            libName: intVersion < 7 ? undefined : c?.['lib-name'] || 'n/a',
+            libVer: intVersion < 7 ? undefined : c?.['lib-ver'] || 'n/a',
+          })),
+        },
+      );
+    } catch (error) {
+      // ignore errors
+    }
+  }
 }
diff --git a/redisinsight/api/src/modules/database/database.analytics.spec.ts b/redisinsight/api/src/modules/database/database.analytics.spec.ts
@@ -344,4 +344,22 @@ describe('DatabaseAnalytics', () => {
       );
     });
   });
+
+  describe('sendDatabaseConnectedClientListEvent', () => {
+    it('should emit event', () => {
+      service.sendDatabaseConnectedClientListEvent(mockDatabase.id, {
+        version: mockDatabase.version,
+        resp: '2',
+      });
+
+      expect(sendEventSpy).toHaveBeenCalledWith(
+        TelemetryEvents.DatabaseConnectedClientList,
+        {
+          instanceId: mockDatabase.id,
+          version: mockDatabase.version,
+          resp: '2',
+        },
+      );
+    });
+  });
 });
diff --git a/redisinsight/api/src/modules/database/database.analytics.ts b/redisinsight/api/src/modules/database/database.analytics.ts
@@ -132,4 +132,18 @@ export class DatabaseAnalytics extends TelemetryBaseService {
       },
     );
   }
+
+  sendDatabaseConnectedClientListEvent(instanceId: string, additionalData: object = {}): void {
+    try {
+      this.sendEvent(
+        TelemetryEvents.DatabaseConnectedClientList,
+        {
+          instanceId,
+          ...additionalData,
+        },
+      );
+    } catch (e) {
+      // continue regardless of error
+    }
+  }
 }
diff --git a/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts b/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts
@@ -7,6 +7,8 @@ import {
   mockIOClusterNode2,
   mockIORedisClient,
   mockIORedisCluster,
+  mockRedisClientList,
+  mockRedisClientListResult,
   mockRedisClientsInfoResponse,
   mockRedisClusterFailInfoResponse,
   mockRedisClusterNodesResponse,
@@ -23,7 +25,7 @@ import {
 import { REDIS_MODULES_COMMANDS, AdditionalRedisModuleName } from 'src/constants';
 import { DatabaseInfoProvider } from 'src/modules/database/providers/database-info.provider';
 import { RedisDatabaseInfoResponse } from 'src/modules/database/dto/redis-info.dto';
-import { BadRequestException, ForbiddenException } from '@nestjs/common';
+import { BadRequestException, ForbiddenException, InternalServerErrorException } from '@nestjs/common';
 import { SentinelMasterStatus } from 'src/modules/redis-sentinel/models/sentinel-master';
 import ERROR_MESSAGES from 'src/constants/error-messages';
 import { FeatureService } from 'src/modules/feature/feature.service';
@@ -219,6 +221,29 @@ describe('DatabaseInfoProvider', () => {
     });
   });
 
+  describe('getClientListInfo', () => {
+    it('get client list info', async () => {
+      when(mockIORedisClient.call)
+        .calledWith('client', ['list'])
+        .mockResolvedValue(mockRedisClientList);
+
+      const result = await service.getClientListInfo(mockIORedisClient);
+
+      expect(result).toEqual(mockRedisClientListResult);
+    });
+    it('failed to get client list', async () => {
+      when(mockIORedisClient.call)
+        .calledWith('client', ['list'])
+        .mockRejectedValue(new Error("unknown command 'client'"));
+
+      try {
+        await service.getClientListInfo(mockIORedisClient)
+      } catch (err) {
+        expect(err).toBeInstanceOf(InternalServerErrorException);
+      }
+    });
+  });
+
   describe('getDatabaseCountFromKeyspace', () => {
     it('should return 1 since db0 keys presented only', async () => {
       const result = await service['getDatabaseCountFromKeyspace']({
diff --git a/redisinsight/api/src/modules/database/providers/database-info.provider.ts b/redisinsight/api/src/modules/database/providers/database-info.provider.ts
@@ -302,6 +302,19 @@ export class DatabaseInfoProvider {
     }
   }
 
+  public async getClientListInfo(client: any): Promise<any[]> {
+    try {
+      const clientListResponse = await client.call('client', ['list']);
+
+      return clientListResponse
+        .split(/\r?\n/)
+        .filter(Boolean)
+        .map((r) => convertBulkStringsToObject(r, ' ', '='));
+    } catch (error) {
+      throw catchAclError(error);
+    }
+  }
+
   /**
    * Try to determine number of logical database from the `info keyspace`
    *

Original file line number	Diff line number	Diff line change
`@@ -302,6 +302,19 @@ export class DatabaseInfoProvider {`
`302`	`302`	`}`
`303`	`303`	`}`
`304`	`304`
	`305`	`+ public async getClientListInfo(client: any): Promise<any[]> {`
	`306`	`+ try {`
	`307`	`+ const clientListResponse = await client.call('client', ['list']);`
	`308`	`+`
	`309`	`+ return clientListResponse`
	`310`	`+ .split(/\r?\n/)`
	`311`	`+ .filter(Boolean)`
	`312`	`+ .map((r) => convertBulkStringsToObject(r, ' ', '='));`
	`313`	`+ } catch (error) {`
	`314`	`+ throw catchAclError(error);`
	`315`	`+ }`
	`316`	`+ }`
	`317`	`+`
`305`	`318`	`/**`
`306`	`319`	* Try to determine number of logical database from the `info keyspace`
`307`	`320`	`*`