From f9af883e6512bf3330ee753f5d7a2aa80b552ad2 Mon Sep 17 00:00:00 2001
From: Stephen Daly <stephen.daly@digital.cabinet-office.gov.uk>
Date: Thu, 12 Oct 2023 15:27:31 +0100
Subject: [PATCH] PP-11120 Add support for additionalLoggingFields

---
 .../clients/base-client/axios-base-client.js  |  7 +-
 .../clients/base-client/config.test.js        | 81 ++++++++++++++-----
 2 files changed, 65 insertions(+), 23 deletions(-)

diff --git a/app/services/clients/base-client/axios-base-client.js b/app/services/clients/base-client/axios-base-client.js
index 09dab23da2..f21331a345 100644
--- a/app/services/clients/base-client/axios-base-client.js
+++ b/app/services/clients/base-client/axios-base-client.js
@@ -38,6 +38,7 @@ class Client {
         method: config.method,
         url: config.url,
         description: config.description,
+        additionalLoggingFields: config.additionalLoggingFields,
         ...config.retryCount && { retryCount: config.retryCount }
       }
       if (options.onRequestStart) {
@@ -64,7 +65,8 @@ class Client {
         params: response.config.params,
         status: response.status,
         url: response.config.url,
-        description: response.config.description
+        description: response.config.description,
+        additionalLoggingFields: response.config.additionalLoggingFields
       }
       if (options.onSuccessResponse) {
         options.onSuccessResponse(responseContext)
@@ -87,7 +89,8 @@ class Client {
         errorIdentifier: error.response && error.response.data && error.response.data.error_identifier,
         reason: error.response && error.response.data && error.response.data.reason,
         message: errors || error.response.data || 'Unknown error',
-        description: config.description
+        description: config.description,
+        additionalLoggingFields: config.additionalLoggingFields
       }
 
       // TODO: could use axios-retry to achieve this if desired
diff --git a/app/services/clients/base-client/config.test.js b/app/services/clients/base-client/config.test.js
index 70e78f8990..6d9ec7a8a4 100644
--- a/app/services/clients/base-client/config.test.js
+++ b/app/services/clients/base-client/config.test.js
@@ -1,6 +1,7 @@
 'use strict'
 
 const nock = require('nock')
+const sinon = require('sinon')
 const proxyquire = require('proxyquire')
 const chai = require('chai')
 const chaiAsPromised = require('chai-as-promised')
@@ -12,39 +13,77 @@ const { expect } = chai
 const baseUrl = 'http://localhost:8000'
 const app = 'an-app'
 
-function getConfigWithMockedRequestContext (correlationId) {
+const logInfoSpy = sinon.spy()
+
+function getConfigWithMocks (correlationId) {
   const config = proxyquire('./config.js', {
     '../../../utils/request-context': {
       getRequestCorrelationIDField: () => correlationId
-    }
+    },
+    '../../../utils/request-logger': proxyquire('../../../utils/request-logger', {
+      './logger': () => ({
+        info: logInfoSpy
+      })
+    })
   })
   return config
 }
 
 describe('Client config', () => {
-  it('should add correlation ID as header when correlation ID exists on request context', () => {
-    const client = new Client(app)
-    const config = getConfigWithMockedRequestContext('abc123')
-    config.configureClient(client, baseUrl)
-
-    nock(baseUrl)
-      .get('/')
-      .reply(200)
-    return expect(client.get('/', 'foo')).to.be.fulfilled.then((response) => {
-      expect(response.request.headers).to.have.property('x-request-id', 'abc123')
+  beforeEach(() => {
+    logInfoSpy.resetHistory()
+  })
+
+  describe('Headers', () => {
+    it('should add correlation ID as header when correlation ID exists on request context', () => {
+      const client = new Client(app)
+      const config = getConfigWithMocks('abc123')
+      config.configureClient(client, baseUrl)
+
+      nock(baseUrl)
+        .get('/')
+        .reply(200)
+      return expect(client.get('/', 'foo')).to.be.fulfilled.then((response) => {
+        expect(response.request.headers).to.have.property('x-request-id', 'abc123')
+      })
+    })
+
+    it('should not add correlation ID as header when correlation ID does not exist on request context', () => {
+      const client = new Client(app)
+      const config = getConfigWithMocks()
+      config.configureClient(client, baseUrl)
+
+      nock(baseUrl)
+        .get('/')
+        .reply(200)
+      return expect(client.get('/', 'foo')).to.be.fulfilled.then((response) => {
+        expect(response.request.headers).to.not.have.key('x-request-id')
+      })
     })
   })
 
-  it('should not add correlation ID as header when correlation ID does not exist on request context', () => {
-    const client = new Client(app)
-    const config = getConfigWithMockedRequestContext()
-    config.configureClient(client, baseUrl)
+  describe('Logging', () => {
+    it('should log request start', () => {
+      const client = new Client(app)
+      const config = getConfigWithMocks('abc123')
+      config.configureClient(client, baseUrl)
 
-    nock(baseUrl)
-      .get('/')
-      .reply(200)
-    return expect(client.get('/', 'foo')).to.be.fulfilled.then((response) => {
-      expect(response.request.headers).to.not.have.key('x-request-id')
+      nock(baseUrl)
+        .get('/')
+        .reply(200)
+      return expect(client.get('/', 'do something', {
+        additionalLoggingFields: {
+          foo: 'bar'
+        }
+      })).to.be.fulfilled.then((response) => {
+        sinon.assert.calledWith(logInfoSpy, 'Calling an-app to do something', {
+          service: app,
+          method: 'get',
+          url: '/',
+          description: 'do something',
+          foo: 'bar'
+        })
+      })
     })
   })
 })