fix: ensure log messages are not dynamic so they're easier to search

Author: ivanstanev

src/supervisor/watchers/handlers/index.ts

@@ -105,11 +105,9 @@ export async function setupNamespacedInformer(
   informer.on(ERROR, (err) => {
     // Types from client library insists that callback is of type KubernetesObject
     const code = (err as any).code || '';
+    logContext.code = code;
     if (RETRYABLE_NETWORK_ERRORS.includes(code)) {
-      logger.debug(
-        logContext,
-        `informer ${code} occurred, restarting informer`,
-      );
+      logger.debug(logContext, 'informer error occurred, restarting informer');
 
       // Restart informer after 1sec
       setTimeout(async () => {
@@ -182,11 +180,9 @@ export async function setupClusterInformer(
   informer.on(ERROR, (err) => {
     // Types from client library insists that callback is of type KubernetesObject
     const code = (err as any).code || '';
+    logContext.code = code;
     if (RETRYABLE_NETWORK_ERRORS.includes(code)) {
-      logger.debug(
-        logContext,
-        `informer ${code} occurred, restarting informer`,
-      );
+      logger.debug(logContext, 'informer error occurred, restarting informer');
 
       // Restart informer after 1sec
       setTimeout(async () => {

src/supervisor/watchers/handlers/namespace.ts

@@ -33,7 +33,7 @@ import { PAGE_SIZE } from './pagination';
  * This feature should be removed at some point!
  */
 export async function trackNamespaces(): Promise<void> {
-  const logContext = {};
+  const logContext: Record<string, unknown> = {};
   const endpoint = '/api/v1/namespaces';
 
   const loggedListMethod = async () => {
@@ -60,11 +60,9 @@ export async function trackNamespaces(): Promise<void> {
   informer.on(ERROR, (err) => {
     // Types from client library insists that callback is of type KubernetesObject
     const code = (err as any).code || '';
+    logContext.code = code;
     if (RETRYABLE_NETWORK_ERRORS.includes(code)) {
-      logger.debug(
-        logContext,
-        `informer ${code} occurred, restarting informer`,
-      );
+      logger.debug(logContext, 'informer error occurred, restarting informer');
 
       // Restart informer after 1sec
       setTimeout(async () => {
@@ -93,7 +91,7 @@ export async function trackNamespaces(): Promise<void> {
  * This feature should be removed at some point!
  */
 export async function trackNamespace(namespace: string): Promise<void> {
-  const logContext = {};
+  const logContext: Record<string, unknown> = {};
   const endpoint = `/api/v1/watch/namespaces/${namespace}`;
 
   const loggedListMethod = async () => {
@@ -127,11 +125,9 @@ export async function trackNamespace(namespace: string): Promise<void> {
   informer.on(ERROR, (err) => {
     // Types from client library insists that callback is of type KubernetesObject
     const code = (err as any).code || '';
+    logContext.code = code;
     if (RETRYABLE_NETWORK_ERRORS.includes(code)) {
-      logger.debug(
-        logContext,
-        `informer ${code} occurred, restarting informer`,
-      );
+      logger.debug(logContext, 'informer error occurred, restarting informer');
 
       // Restart informer after 1sec
       setTimeout(async () => {

test/unit/supervisor/watchers.spec.ts

@@ -1,8 +1,9 @@
 import { V1Namespace } from '@kubernetes/client-node';
 import { config } from '../../../src/common/config';
 
-import * as watchers from '../../../src/supervisor/watchers';
 import {
+  extractNamespaceName,
+  isExcludedNamespace,
   kubernetesInternalNamespaces,
   openshiftInternalNamespaces,
 } from '../../../src/supervisor/watchers/internal-namespaces';
@@ -14,14 +15,14 @@ describe('extractNamespaceName()', () => {
     ['undefined name', { metadata: { name: undefined } } as V1Namespace],
     ['empty name', { metadata: { name: '' } } as V1Namespace],
   ])('throws on %s', (_testCaseName, input) => {
-    expect(() => watchers.extractNamespaceName(input)).toThrowError(
+    expect(() => extractNamespaceName(input)).toThrowError(
       'Namespace missing metadata.name',
     );
   });
 
   test('returns namespace.metadata.name', () => {
     expect(
-      watchers.extractNamespaceName({
+      extractNamespaceName({
         metadata: { name: 'literally anything else' },
       }),
     ).toEqual('literally anything else');
@@ -35,7 +36,7 @@ describe('isExcludedNamespace() internal Kubernetes namespaces', () => {
 
   for (const internalNamespace of kubernetesInternalNamespaces) {
     test(`isExcludedNamespace(${internalNamespace}) -> true`, () => {
-      expect(watchers.isExcludedNamespace(internalNamespace)).toEqual(true);
+      expect(isExcludedNamespace(internalNamespace)).toEqual(true);
     });
   }
 
@@ -47,7 +48,7 @@ describe('isExcludedNamespace() internal Kubernetes namespaces', () => {
     [''],
     [undefined as unknown as string],
   ])('isExcludedNamespace(%s) -> false', (input) => {
-    expect(watchers.isExcludedNamespace(input)).toEqual(false);
+    expect(isExcludedNamespace(input)).toEqual(false);
   });
 });
 
@@ -58,7 +59,7 @@ describe('isExcludedNamespace() openshift internal namespaces', () => {
 
   for (const internalNamespace of openshiftInternalNamespaces) {
     test(`isExcludedNamespace(${internalNamespace}) -> true`, () => {
-      expect(watchers.isExcludedNamespace(internalNamespace)).toEqual(true);
+      expect(isExcludedNamespace(internalNamespace)).toEqual(true);
     });
   }
 
@@ -69,7 +70,7 @@ describe('isExcludedNamespace() openshift internal namespaces', () => {
     [''],
     [undefined as unknown as string],
   ])('isExcludedNamespace(%s) -> false', (input) => {
-    expect(watchers.isExcludedNamespace(input)).toEqual(false);
+    expect(isExcludedNamespace(input)).toEqual(false);
   });
 });
 
@@ -85,20 +86,20 @@ describe('isExcludedNamespace() excluded namespaces from config', () => {
 
   excludedNamespacesFromConfig.forEach((namespace) => {
     test(`[excluded namespaces from config] isExcludedNamespace(${namespace}) -> true`, () => {
-      expect(watchers.isExcludedNamespace(namespace)).toEqual(true);
+      expect(isExcludedNamespace(namespace)).toEqual(true);
     });
   });
 
   for (const internalNamespace of openshiftInternalNamespaces) {
     test(`[openshift internal namespaces] isExcludedNamespace(${internalNamespace}) -> true`, () => {
-      expect(watchers.isExcludedNamespace(internalNamespace)).toEqual(true);
+      expect(isExcludedNamespace(internalNamespace)).toEqual(true);
     });
   }
 
   test.each([['kube-system']['egg'], [''], [undefined as unknown as string]])(
     'isExcludedNamespace(%s) -> false',
     (input) => {
-      expect(watchers.isExcludedNamespace(input)).toEqual(false);
+      expect(isExcludedNamespace(input)).toEqual(false);
     },
   );
 });