fix: feature toggle update total needs 4 labels (#5946) (#5966)

So, this was causing a lot of ERROR in our logs, due to the metric having gotten an extra label the last month. Two things for this fix. 1. add the missing label to the two calls that did not have it added 2. update the log line to include the error as another argument to the logger, so we actually get a stacktrace from the error.
Unleash · Jan 19, 2024 · 228d994 · 228d994
1 parent e6c7f44
commit 228d994
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 42 deletions.
diff --git a/src/lib/features/scheduler/scheduler-service.test.ts b/src/lib/features/scheduler/scheduler-service.test.ts
@@ -176,10 +176,15 @@ test('Can handle crash of a async job', async () => {
     await ms(75);
 
     schedulerService.stop();
-    expect(getRecords()).toEqual([
-        ['scheduled job failed | id: test-id-10 | async reason'],
-        ['scheduled job failed | id: test-id-10 | async reason'],
-    ]);
+    const records = getRecords();
+    expect(records[0][0]).toContain(
+        'initial scheduled job failed | id: test-id-10',
+    );
+    expect(records[0][1]).toContain('async reason');
+    expect(records[1][0]).toContain(
+        'interval scheduled job failed | id: test-id-10',
+    );
+    expect(records[1][1]).toContain('async reason');
 });
 
 test('Can handle crash of a sync job', async () => {
@@ -196,30 +201,14 @@ test('Can handle crash of a sync job', async () => {
     await ms(75);
 
     schedulerService.stop();
-    expect(getRecords()).toEqual([
-        ['scheduled job failed | id: test-id-11 | Error: sync reason'],
-        ['scheduled job failed | id: test-id-11 | Error: sync reason'],
-    ]);
-});
-
-test('Can handle crash of a async job', async () => {
-    const { logger, getRecords } = getLogger();
-    const { schedulerService } = createSchedulerTestService({
-        loggerOverride: logger,
-    });
-
-    const job = async () => {
-        await Promise.reject('async reason');
-    };
-
-    await schedulerService.schedule(job, 50, 'test-id-10');
-    await ms(75);
-
-    schedulerService.stop();
-    expect(getRecords()).toEqual([
-        ['scheduled job failed | id: test-id-10 | async reason'],
-        ['scheduled job failed | id: test-id-10 | async reason'],
-    ]);
+    const records = getRecords();
+    expect(records[0][0]).toContain(
+        'initial scheduled job failed | id: test-id-11',
+    );
+    expect(records[0][1].message).toContain('sync reason');
+    expect(records[1][0]).toContain(
+        'interval scheduled job failed | id: test-id-11',
+    );
 });
 
 it('should emit scheduler job time event when scheduled function is run', async () => {

diff --git a/src/lib/features/scheduler/scheduler-service.ts b/src/lib/features/scheduler/scheduler-service.ts
@@ -52,7 +52,8 @@ export class SchedulerService {
                     }
                 } catch (e) {
                     this.logger.error(
-                        `scheduled job failed | id: ${id} | ${e}`,
+                        `interval scheduled job failed | id: ${id}`,
+                        e,
                     );
                 }
             }, timeMs).unref(),
@@ -64,7 +65,7 @@ export class SchedulerService {
                 await runScheduledFunctionWithEvent();
             }
         } catch (e) {
-            this.logger.error(`scheduled job failed | id: ${id} | ${e}`);
+            this.logger.error(`initial scheduled job failed | id: ${id}`, e);
         }
     }
 

diff --git a/src/lib/metrics.ts b/src/lib/metrics.ts
@@ -450,10 +450,14 @@ export default class MetricsMonitor {
             },
         );
         eventStore.on(FEATURE_ARCHIVED, ({ featureName, project }) => {
-            featureToggleUpdateTotal.labels(featureName, project, 'n/a').inc();
+            featureToggleUpdateTotal
+                .labels(featureName, project, 'n/a', 'n/a')
+                .inc();
         });
         eventStore.on(FEATURE_REVIVED, ({ featureName, project }) => {
-            featureToggleUpdateTotal.labels(featureName, project, 'n/a').inc();
+            featureToggleUpdateTotal
+                .labels(featureName, project, 'n/a', 'n/a')
+                .inc();
         });
 
         eventBus.on(CLIENT_METRICS, (m: ValidatedClientMetrics) => {

diff --git a/src/lib/services/scheduler-service.test.ts b/src/lib/services/scheduler-service.test.ts
@@ -6,7 +6,7 @@ import SettingService from './setting-service';
 import EventService from './event-service';
 import MaintenanceService from '../features/maintenance/maintenance-service';
 
-function ms(timeMs) {
+function ms(timeMs: number) {
     return new Promise((resolve) => setTimeout(resolve, timeMs));
 }
 
@@ -29,7 +29,7 @@ const getLogger = () => {
 };
 
 let schedulerService: SchedulerService;
-let getRecords;
+let getRecords: () => any[];
 
 beforeEach(() => {
     const config = createTestConfig();
@@ -100,10 +100,13 @@ test('Can handle crash of a async job', async () => {
     await ms(75);
 
     schedulerService.stop();
-    expect(getRecords()).toEqual([
-        ['scheduled job failed | id: test-id-10 | async reason'],
-        ['scheduled job failed | id: test-id-10 | async reason'],
-    ]);
+    const records = getRecords();
+    expect(records[0][0]).toContain(
+        'initial scheduled job failed | id: test-id-10',
+    );
+    expect(records[1][0]).toContain(
+        'interval scheduled job failed | id: test-id-10',
+    );
 });
 
 test('Can handle crash of a sync job', async () => {
@@ -115,8 +118,11 @@ test('Can handle crash of a sync job', async () => {
     await ms(75);
 
     schedulerService.stop();
-    expect(getRecords()).toEqual([
-        ['scheduled job failed | id: test-id-11 | Error: sync reason'],
-        ['scheduled job failed | id: test-id-11 | Error: sync reason'],
-    ]);
+    const records = getRecords();
+    expect(records[0][0]).toContain(
+        'initial scheduled job failed | id: test-id-11',
+    );
+    expect(records[1][0]).toContain(
+        'interval scheduled job failed | id: test-id-11',
+    );
 });