feat: Add codeOwner to logging context

libs/infra-tracing/src/lib/code-owner.spec.ts

@@ -1,12 +1,12 @@
-import { setCodeOwner } from './code-owner'
+import { withCodeOwner } from './code-owner'
 import { CodeOwners } from '@island.is/shared/constants'
-import { logger } from '@island.is/logging'
+import { logger, withLoggingContext } from '@island.is/logging'
 import tracer from 'dd-trace'
 
 jest.mock('dd-trace')
 jest.mock('@island.is/logging')
 
-describe('setCodeOwner', () => {
+describe('withCodeOwner', () => {
   let mockSpan: { setTag: jest.Mock }
   let mockScope: jest.Mock
 
@@ -19,31 +19,64 @@ describe('setCodeOwner', () => {
     }))
     ;(tracer.scope as jest.Mock) = mockScope
     ;(logger.warn as jest.Mock).mockClear()
+    ;(withLoggingContext as jest.Mock).mockImplementation(
+      (_, callback, ...args) => callback(...args),
+    )
   })
 
-  it('should set code owner tag on active span', () => {
+  it('should set code owner tag on active span and call the callback', () => {
+    // Arrange
+    const mockCallback = jest.fn()
+    const mockArgs = ['arg1', 'arg2']
+
     // Act
-    setCodeOwner(CodeOwners.Core)
+    withCodeOwner(CodeOwners.Core, mockCallback, ...mockArgs)
 
     // Assert
     expect(mockSpan.setTag).toHaveBeenCalledWith('codeOwner', CodeOwners.Core)
+    expect(withLoggingContext).toHaveBeenCalledWith(
+      { codeOwner: CodeOwners.Core },
+      mockCallback,
+      ...mockArgs,
+    )
+    expect(mockCallback).toHaveBeenCalledWith(...mockArgs)
     expect(logger.warn).not.toHaveBeenCalled()
   })
 
-  it('should log warning when no active span exists', () => {
+  it('should log warning when no active span exists and still call the callback', () => {
     // Arrange
     mockScope = jest.fn(() => ({
       active: () => null,
     }))
     ;(tracer.scope as jest.Mock) = mockScope
+    const mockCallback = jest.fn()
+    const mockArgs = ['arg1', 'arg2']
 
     // Act
-    setCodeOwner(CodeOwners.Core)
+    withCodeOwner(CodeOwners.Core, mockCallback, ...mockArgs)
 
     // Assert
     expect(logger.warn).toHaveBeenCalledWith(
       'Setting code owner "core" with no active dd-trace span',
       { stack: expect.any(String) },
     )
+    expect(withLoggingContext).toHaveBeenCalledWith(
+      { codeOwner: CodeOwners.Core },
+      mockCallback,
+      ...mockArgs,
+    )
+    expect(mockCallback).toHaveBeenCalledWith(...mockArgs)
+  })
+
+  it('should return the callback result', () => {
+    // Arrange
+    const expectedResult = { foo: 'bar' }
+    const mockCallback = jest.fn().mockReturnValue(expectedResult)
+
+    // Act
+    const result = withCodeOwner(CodeOwners.Core, mockCallback)
+
+    // Assert
+    expect(result).toBe(expectedResult)
   })
 })

libs/infra-tracing/src/lib/code-owner.ts

@@ -1,14 +1,18 @@
-import { logger } from '@island.is/logging'
+import { logger, withLoggingContext } from '@island.is/logging'
 import { CodeOwners } from '@island.is/shared/constants'
 import tracer from 'dd-trace'
 
 /**
- * Sets a code owner for the current dd-trace span.
+ * Sets a code owner for the current dd-trace span and all nested log entries.
  *
  * The assumption here is that each trace / request has only one "dynamic"
  * code owner. This way we skip cluttering the trace with extra spans.
  */
-export const setCodeOwner = (codeOwner: CodeOwners) => {
+export const withCodeOwner = <R, TArgs extends unknown[]>(
+  codeOwner: CodeOwners,
+  callback: (...args: TArgs) => R,
+  ...args: TArgs
+) => {
   const span = tracer.scope().active()
   if (span) {
     span.setTag('codeOwner', codeOwner)
@@ -19,4 +23,5 @@ export const setCodeOwner = (codeOwner: CodeOwners) => {
       { stack },
     )
   }
+  return withLoggingContext({ codeOwner }, callback, ...args)
 }

libs/logging/src/lib/context.spec.ts

@@ -1,3 +1,4 @@
+import { CodeOwners } from '@island.is/shared/constants'
 import { includeContextFormatter, withLoggingContext } from './context'
 
 describe('Winston context', () => {
@@ -11,6 +12,50 @@ describe('Winston context', () => {
     process.env = originalEnv
   })
 
+  it('should add default CODE_OWNER when environment variable is set', () => {
+    // Arrange
+    process.env.CODE_OWNER = 'default-team'
+    const formatter = includeContextFormatter()
+    const logInfo = {
+      level: 'info',
+      message: 'Test message',
+    }
+
+    // Act
+    const formattedLog = formatter.transform(logInfo)
+
+    // Assert
+    expect(formattedLog).toEqual({
+      level: 'info',
+      message: 'Test message',
+      codeOwner: 'default-team',
+    })
+  })
+
+  it('should override default CODE_OWNER with context codeOwner', () => {
+    // Arrange
+    process.env.CODE_OWNER = 'default-team'
+    const formatter = includeContextFormatter()
+    const logInfo = {
+      level: 'info',
+      message: 'Test message',
+    }
+    const context = { codeOwner: 'context-team' as CodeOwners }
+
+    // Act
+    let formattedLog: unknown
+    withLoggingContext(context, () => {
+      formattedLog = formatter.transform(logInfo)
+    })
+
+    // Assert
+    expect(formattedLog).toEqual({
+      level: 'info',
+      message: 'Test message',
+      codeOwner: 'context-team',
+    })
+  })
+
   it('should add context to log info object', () => {
     // Arrange
     const formatter = includeContextFormatter()
@@ -35,7 +80,7 @@ describe('Winston context', () => {
     })
   })
 
-  it('should not modify log info when no context exists', () => {
+  it('should not modify log info when no context or CODE_OWNER exists', () => {
     // Arrange
     const formatter = includeContextFormatter()
     const logInfo = {
@@ -52,6 +97,7 @@ describe('Winston context', () => {
 
   it('should preserve existing log info properties when adding context', () => {
     // Arrange
+    process.env.CODE_OWNER = 'default-team'
     const formatter = includeContextFormatter()
     const logInfo = {
       level: 'info',
@@ -71,6 +117,7 @@ describe('Winston context', () => {
       level: 'info',
       message: 'Test message',
       existingProp: 'should remain',
+      codeOwner: 'default-team',
       requestId: '123',
     })
   })

libs/logging/src/lib/context.ts

@@ -19,8 +19,12 @@ export const withLoggingContext = <R, TArgs extends unknown[]>(
 }
 
 export const includeContextFormatter = format((info) => {
+  const defaultCodeOwner = process.env.CODE_OWNER
   const context = loggingContextStorage.getStore()
 
+  if (defaultCodeOwner) {
+    info.codeOwner = defaultCodeOwner
+  }
   if (context) {
     Object.assign(info, context)
   }

libs/nest/core/src/lib/code-owner/code-owner.interceptor.spec.ts

@@ -1,4 +1,4 @@
-import { setCodeOwner } from '@island.is/infra-tracing'
+import { withCodeOwner } from '@island.is/infra-tracing'
 import { CodeOwners } from '@island.is/shared/constants'
 import { Controller, Get, INestApplication } from '@nestjs/common'
 import { APP_INTERCEPTOR } from '@nestjs/core'
@@ -9,7 +9,7 @@ import { CodeOwnerInterceptor } from './code-owner.interceptor'
 
 // Mock the logging module
 jest.mock('@island.is/infra-tracing', () => ({
-  setCodeOwner: jest.fn(),
+  withCodeOwner: jest.fn((codeOwner, callback) => callback()),
 }))
 
 // Test controller with decorated endpoints
@@ -50,26 +50,29 @@ describe('CodeOwnerInterceptor', () => {
     jest.clearAllMocks()
   })
 
-  it('should call setCodeOwner when CodeOwner decorator is present', async () => {
+  it('should call withCodeOwner when CodeOwner decorator is present', async () => {
     // Make request to endpoint with CodeOwner decorator
     await request(app.getHttpServer())
       .get('/test/with-owner')
       .expect(200)
       .expect({ message: 'with owner' })
 
-    // Verify that setCodeOwner was called with correct parameters
-    expect(setCodeOwner).toHaveBeenCalledWith(CodeOwners.Core)
+    // Verify that withCodeOwner was called with correct parameters
+    expect(withCodeOwner).toHaveBeenCalledWith(
+      CodeOwners.Core,
+      expect.any(Function),
+    )
   })
 
-  it('should not call setCodeOwner when CodeOwner decorator is not present', async () => {
+  it('should not call withCodeOwner when CodeOwner decorator is not present', async () => {
     // Make request to endpoint without CodeOwner decorator
     await request(app.getHttpServer())
       .get('/test/without-owner')
       .expect(200)
       .expect({ message: 'without owner' })
 
-    // Verify that setCodeOwner was not called
-    expect(setCodeOwner).not.toHaveBeenCalled()
+    // Verify that withCodeOwner was not called
+    expect(withCodeOwner).not.toHaveBeenCalled()
   })
 
   it('should handle multiple requests correctly', async () => {
@@ -80,8 +83,33 @@ describe('CodeOwnerInterceptor', () => {
       request(app.getHttpServer()).get('/test/with-owner'),
     ])
 
-    // Verify that setCodeOwner was called exactly twice (for the two 'with-owner' requests)
-    expect(setCodeOwner).toHaveBeenCalledTimes(2)
-    expect(setCodeOwner).toHaveBeenCalledWith(CodeOwners.Core)
+    // Verify that withCodeOwner was called exactly twice (for the two 'with-owner' requests)
+    expect(withCodeOwner).toHaveBeenCalledTimes(2)
+    expect(withCodeOwner).toHaveBeenCalledWith(
+      CodeOwners.Core,
+      expect.any(Function),
+    )
+  })
+
+  it('should properly wrap and execute the handler', async () => {
+    // Arrange
+    let handlerExecuted = false
+    ;(withCodeOwner as jest.Mock).mockImplementation((codeOwner, callback) => {
+      handlerExecuted = true
+      return callback()
+    })
+
+    // Act
+    await request(app.getHttpServer())
+      .get('/test/with-owner')
+      .expect(200)
+      .expect({ message: 'with owner' })
+
+    // Assert
+    expect(handlerExecuted).toBe(true)
+    expect(withCodeOwner).toHaveBeenCalledWith(
+      CodeOwners.Core,
+      expect.any(Function),
+    )
   })
 })

libs/nest/core/src/lib/code-owner/code-owner.interceptor.ts

@@ -1,4 +1,4 @@
-import { setCodeOwner } from '@island.is/infra-tracing'
+import { withCodeOwner } from '@island.is/infra-tracing'
 import { CodeOwners } from '@island.is/shared/constants'
 import {
   Injectable,
@@ -19,9 +19,8 @@ export class CodeOwnerInterceptor implements NestInterceptor {
       CODE_OWNER_KEY,
       [context.getHandler(), context.getClass()],
     )
-
     if (codeOwner) {
-      setCodeOwner(codeOwner)
+      return withCodeOwner(codeOwner, () => next.handle())
     }
     return next.handle()
   }