refactor(scorecard): logic to load and cleanup metrics

Signed-off-by: Ihor Mykhno <[email protected]>

Author: imykhno

workspaces/scorecard/plugins/scorecard-backend-module-github/README.md

@@ -93,12 +93,12 @@ Thresholds define conditions that determine which category a metric value belong
 
 ## Schedule Configuration
 
-The Scorecard plugin uses Backstage's built-in scheduler service to automatically collect metrics from all registered providers.
+The Scorecard plugin uses Backstage's built-in scheduler service to automatically collect metrics from all registered providers every hour by default. However, this configuration can be changed in the `app-config.yaml` file. Here is an example of how to do that:
 
 ```yaml
 scorecard:
   plugins:
-    jira:
+    github:
       open_prs:
         schedule:
           frequency:

workspaces/scorecard/plugins/scorecard-backend-module-jira/README.md

@@ -100,7 +100,7 @@ scorecard:
 
 ## Schedule Configuration
 
-The Scorecard plugin uses Backstage's built-in scheduler service to automatically collect metrics from all registered providers.
+The Scorecard plugin uses Backstage's built-in scheduler service to automatically collect metrics from all registered providers every hour by default. However, this configuration can be changed in the `app-config.yaml` file. Here is an example of how to do that:
 
 ```yaml
 scorecard:

workspaces/scorecard/plugins/scorecard-backend-module-jira/src/clients/base.ts

@@ -17,10 +17,7 @@
 import type { Config } from '@backstage/config';
 import type { Entity } from '@backstage/catalog-model';
 import { JiraEntityFilters, JiraOptions, RequestOptions } from './types';
-import {
-  JIRA_MANDATORY_FILTER,
-  OPEN_ISSUES_CONFIG_PATH,
-} from '../constants';
+import { JIRA_MANDATORY_FILTER, OPEN_ISSUES_CONFIG_PATH } from '../constants';
 import { ScorecardJiraAnnotations } from '../annotations';
 import { sanitizeValue, validateIdentifier, validateJQLValue } from './utils';
 import { ConnectionStrategy } from '../strategies/ConnectionStrategy';

workspaces/scorecard/plugins/scorecard-backend/README.md

@@ -97,6 +97,16 @@ Thresholds are evaluated in order, and the first matching rule determines the ca
 
 For comprehensive threshold configuration guide, examples, and best practices, see [thresholds.md](./docs/thresholds.md).
 
+## Configuration cleanup Job
+
+The plugin has a predefined job that runs every day to check and clean old metrics. By default, metrics are saved for **365 days**, however, this period can be changed in the `app-config.yaml` file. Here is an example of how to do that:
+
+```YAML app-config.yaml
+---
+scorecard:
+  dataRetentionDays: 12
+```
+
 ## Development
 
 This plugin backend can be started in a standalone mode from directly in this

workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockDatabase.ts renamed to workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockDatabaseMetricValues.ts

@@ -14,10 +14,10 @@
  * limitations under the License.
  */
 
-import { MetricValuesStore } from '../src/database/MetricValuesStore';
+import { DatabaseMetricValues } from '../src/database/DatabaseMetricValues';
 
-export const mockMetricValuesStore = {
+export const mockDatabaseMetricValues = {
   createMetricValues: jest.fn(),
-  readMetricValues: jest.fn(),
   readLatestEntityMetricValues: jest.fn(),
-} as unknown as jest.Mocked<MetricValuesStore>;
+  cleanupExpiredMetrics: jest.fn(),
+} as unknown as jest.Mocked<DatabaseMetricValues>;

workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockMetricProvidersRegistry.ts

@@ -0,0 +1,28 @@
+/*
+ * Copyright Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { MetricProvidersRegistry } from '../src/providers/MetricProvidersRegistry';
+
+export const mockMetricProvidersRegistry = {
+  register: jest.fn(),
+  getProvider: jest.fn(),
+  getMetric: jest.fn(),
+  calculateMetric: jest.fn(),
+  calculateMetrics: jest.fn(),
+  listProviders: jest.fn().mockReturnValue([]),
+  listMetrics: jest.fn().mockReturnValue([]),
+  listMetricsByDatasource: jest.fn().mockReturnValue([]),
+} as unknown as jest.Mocked<MetricProvidersRegistry>;

workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.test.ts

@@ -0,0 +1,194 @@
+/*
+ * Copyright Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {
+  mockServices,
+  TestDatabaseId,
+  TestDatabases,
+} from '@backstage/backend-test-utils';
+import { DatabaseMetricValues } from './DatabaseMetricValues';
+import { DbMetricValue } from './types';
+import { migrate } from './migration';
+
+jest.setTimeout(60000);
+
+const metricValues: Omit<DbMetricValue, 'id'>[] = [
+  {
+    catalog_entity_ref: 'component:default/test-service',
+    metric_id: 'github.metric1',
+    value: 41,
+    timestamp: new Date('2023-01-01T00:00:00Z'),
+    error_message: undefined,
+  },
+  {
+    catalog_entity_ref: 'component:default/another-service',
+    metric_id: 'github.metric1',
+    value: 25,
+    timestamp: new Date('2023-01-01T00:00:00Z'),
+    error_message: undefined,
+  },
+  {
+    catalog_entity_ref: 'component:default/another-service',
+    metric_id: 'github.metric2',
+    value: undefined,
+    timestamp: new Date('2023-01-01T00:00:00Z'),
+    error_message: 'Failed to fetch metric',
+  },
+];
+
+describe('DatabaseMetricValues', () => {
+  const databases = TestDatabases.create({
+    ids: ['SQLITE_3', 'POSTGRES_15'],
+  });
+
+  async function createDatabase(databaseId: TestDatabaseId) {
+    const client = await databases.init(databaseId);
+    const mockDatabaseService = mockServices.database.mock({
+      getClient: async () => client,
+      migrations: { skip: false },
+    });
+
+    await migrate(mockDatabaseService);
+
+    return {
+      client,
+      db: new DatabaseMetricValues(client),
+    };
+  }
+
+  describe('createMetricValues', () => {
+    it.each(databases.eachSupportedId())(
+      'should successfully insert metric values - %p',
+      async databaseId => {
+        const { client, db } = await createDatabase(databaseId);
+
+        await expect(
+          db.createMetricValues(metricValues),
+        ).resolves.not.toThrow();
+
+        const insertedValues = await client('metric_values').select('*');
+
+        expect(insertedValues).toHaveLength(3);
+
+        expect(insertedValues[0]).toMatchObject({
+          catalog_entity_ref: 'component:default/test-service',
+          metric_id: 'github.metric1',
+          value: 41,
+          error_message: null,
+        });
+
+        expect(insertedValues[1]).toMatchObject({
+          catalog_entity_ref: 'component:default/another-service',
+          metric_id: 'github.metric1',
+          value: 25,
+          error_message: null,
+        });
+
+        expect(insertedValues[2]).toMatchObject({
+          catalog_entity_ref: 'component:default/another-service',
+          metric_id: 'github.metric2',
+          value: null,
+          error_message: 'Failed to fetch metric',
+        });
+      },
+    );
+  });
+
+  describe('readLatestEntityMetricValues', () => {
+    it.each(databases.eachSupportedId())(
+      'should return latest metric values for entity and metrics - %p',
+      async databaseId => {
+        const { client, db } = await createDatabase(databaseId);
+
+        const baseTime = new Date('2023-01-01T00:00:00Z');
+        const laterTime = new Date('2023-01-01T01:00:00Z');
+
+        await client('metric_values').insert([
+          {
+            ...metricValues[0],
+            timestamp: baseTime, // older time
+          },
+          {
+            ...metricValues[1],
+            timestamp: laterTime, // newer time, value should be returned
+          },
+          {
+            ...metricValues[2],
+            timestamp: laterTime, // newer time, different entity
+          },
+          {
+            catalog_entity_ref: 'component:default/test-service',
+            metric_id: 'github.metric2',
+            value: undefined,
+            timestamp: baseTime,
+            error_message: 'Failed to fetch metric',
+          },
+        ]);
+
+        const result = await db.readLatestEntityMetricValues(
+          'component:default/test-service',
+          ['github.metric1', 'github.metric2'],
+        );
+
+        expect(result).toHaveLength(2);
+
+        const metric1Result = result.find(
+          r => r.metric_id === 'github.metric1',
+        );
+        const metric2Result = result.find(
+          r => r.metric_id === 'github.metric2',
+        );
+
+        expect(metric1Result).toMatchObject({
+          catalog_entity_ref: 'component:default/test-service',
+          metric_id: 'github.metric1',
+          value: 41,
+        });
+
+        expect(metric2Result).toMatchObject({
+          catalog_entity_ref: 'component:default/test-service',
+          metric_id: 'github.metric2',
+          value: null,
+        });
+      },
+    );
+  });
+
+  describe('cleanupExpiredMetrics', () => {
+    it.each(databases.eachSupportedId())(
+      'should delete metric values that are older than the given date - %p',
+      async databaseId => {
+        const { client, db } = await createDatabase(databaseId);
+
+        await client('metric_values').insert([
+          {
+            ...metricValues[0],
+            timestamp: new Date('2022-01-01T00:00:00Z'),
+          },
+          {
+            ...metricValues[1],
+          },
+        ]);
+
+        const result = await db.cleanupExpiredMetrics(
+          new Date('2023-01-01T00:00:00Z'),
+        );
+
+        expect(result).toBe(1);
+      },
+    );
+  });
+});

workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValuesStore.ts renamed to workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts

@@ -15,20 +15,20 @@
  */
 
 import { Knex } from 'knex';
-import { DbMetricValue, MetricValuesStore } from './MetricValuesStore';
+import { DbMetricValue } from './types';
 
-export class DatabaseMetricValuesStore implements MetricValuesStore {
+export class DatabaseMetricValues {
   private readonly tableName = 'metric_values';
 
-  constructor(private readonly knex: Knex<any, any[]>) {}
+  constructor(private readonly dbClient: Knex<any, any[]>) {}
 
   /**
    * Insert multiple metric values
    */
   async createMetricValues(
     metricValues: Omit<DbMetricValue, 'id'>[],
   ): Promise<void> {
-    await this.knex(this.tableName).insert(metricValues);
+    await this.dbClient(this.tableName).insert(metricValues);
   }
 
   /**
@@ -38,11 +38,11 @@ export class DatabaseMetricValuesStore implements MetricValuesStore {
     catalog_entity_ref: string,
     metric_ids: string[],
   ): Promise<DbMetricValue[]> {
-    return await this.knex(this.tableName)
+    return await this.dbClient(this.tableName)
       .select('*')
       .whereIn(
         'id',
-        this.knex(this.tableName)
+        this.dbClient(this.tableName)
           .max('id')
           .whereIn('metric_id', metric_ids)
           .where('catalog_entity_ref', catalog_entity_ref)
@@ -54,7 +54,7 @@ export class DatabaseMetricValuesStore implements MetricValuesStore {
    * Delete metric values that are older than the given date
    */
   async cleanupExpiredMetrics(olderThan: Date): Promise<number> {
-    return await this.knex(this.tableName)
+    return await this.dbClient(this.tableName)
       .where('timestamp', '<', olderThan)
       .del();
   }