Skip to content

Commit 5c5924a

Browse files
tstephen-nhsdependabot[bot]connoravo-nhs
authored
Chore: [AEA-5970] - expand batched logs (#2437)
## Summary - 🤖 Operational or Infrastructure Change ### Details Instead of logging requests to Notify in the batches they are sent, log individually to facilitate reporting. --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Connor Avery <[email protected]>
1 parent 05eb423 commit 5c5924a

File tree

7 files changed

+931
-805
lines changed

7 files changed

+931
-805
lines changed

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
"dependencies": {
5353
"@psu-common/commonTypes": "^1.0.0",
5454
"@psu-common/middyErrorHandler": "^1.0.0",
55+
"@psu-common/utilities": "^1.0.0",
5556
"esbuild": "^0.27.0"
5657
}
5758
}
Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,8 @@
1-
export {testPrescriptionsConfig, TestPrescriptionsConfig, getTestPrescriptions} from "./testConfig"
2-
export {initiatedSSMProvider} from "./ssmUtil"
1+
export {testPrescriptionsConfig, TestPrescriptionsConfig, getTestPrescriptions} from "./testConfig.js"
2+
export const LOG_MESSAGES = {
3+
PSU0001: "Transitioning item status.",
4+
PSU0002: "Notify request",
5+
PSU0003: "Updated notification state"
6+
} as const
7+
8+
export {initiatedSSMProvider} from "./ssmUtil.js"

packages/nhsNotifyLambda/src/utils/notify.ts

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import {Logger} from "@aws-lambda-powertools/logger"
22

33
import axios from "axios"
4-
54
import {setupAxios} from "./axios"
5+
import {LOG_MESSAGES} from "@psu-common/utilities"
6+
67
import {
78
NotifyDataItemMessage,
89
CreateMessageBatchRequest,
@@ -96,6 +97,23 @@ async function makeFakeNotifyRequest(
9697
const messageStatus = "silent running"
9798
const messageBatchReference = crypto.randomUUID()
9899

100+
logNotificationRequest(logger, messageBatchReference, messages, data, messageStatus)
101+
102+
// Map each input item to a "successful" NotifyDataItemMessage
103+
return data.map(item => {
104+
return {
105+
...item,
106+
messageBatchReference,
107+
messageStatus,
108+
notifyMessageId: crypto.randomUUID() // Create a dummy UUID
109+
}
110+
})
111+
}
112+
113+
function logNotificationRequest(logger: Logger,
114+
messageBatchReference: string, messages: Array<MessageBatchItem>,
115+
data: Array<NotifyDataItemMessage>, messageStatus: string) {
116+
// TODO: preserve legacy logging until reports updated
99117
logger.info("Requested notifications OK!", {
100118
messageBatchReference,
101119
messageReferences: messages.map(e => ({
@@ -105,15 +123,21 @@ async function makeFakeNotifyRequest(
105123
})),
106124
messageStatus: messageStatus
107125
})
108-
109-
// Map each input item to a "successful" NotifyDataItemMessage
110-
return data.map(item => {
111-
return {
112-
...item,
126+
// Log each message individually for less memory intensive reporting
127+
const code = Object.keys(LOG_MESSAGES)
128+
.find(key => LOG_MESSAGES[key as keyof typeof LOG_MESSAGES] === LOG_MESSAGES.PSU0002)
129+
messages.forEach((message, index) => {
130+
const correspondingData = data.find(item => item.messageReference === message.messageReference)
131+
logger.info(LOG_MESSAGES.PSU0002, {
132+
reportCode: code,
113133
messageBatchReference,
114-
messageStatus,
115-
notifyMessageId: crypto.randomUUID() // Create a dummy UUID
116-
}
134+
messageIndex: index,
135+
nhsNumber: message.recipient.nhsNumber,
136+
messageReference: message.messageReference,
137+
psuRequestId: correspondingData?.PSUDataItem.RequestID,
138+
notifyMessageId: messageStatus === "silent running" ? crypto.randomUUID() : correspondingData?.notifyMessageId,
139+
messageStatus: messageStatus
140+
})
117141
})
118142
}
119143

@@ -186,16 +210,7 @@ export async function makeRealNotifyRequest(
186210
// From here is just logging stuff for reporting, and mapping the response back to the input data
187211

188212
if (resp.status === 201) {
189-
logger.info("Requested notifications OK!", {
190-
messageBatchReference,
191-
messageReferences: messages.map(e => ({
192-
nhsNumber: e.recipient.nhsNumber,
193-
messageReference: e.messageReference,
194-
psuRequestId: data.find((el) => el.messageReference === e.messageReference)?.PSUDataItem.RequestID,
195-
pharmacyODSCode: e.originator.odsCode
196-
})),
197-
messageStatus: "requested"
198-
})
213+
logNotificationRequest(logger, messageBatchReference, messages, data, "requested")
199214

200215
// Map each input item to a NotifyDataItemMessage, marking success and attaching the notify ID.
201216
// Only return items that belong to *this* batch (so we handle recursive splits correctly).

packages/nhsNotifyLambda/tests/testUtils.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import axiosRetry from "axios-retry"
66
import {Logger} from "@aws-lambda-powertools/logger"
77
import {DynamoDBDocumentClient, PutCommand} from "@aws-sdk/lib-dynamodb"
88
import {GetQueueAttributesCommand, DeleteMessageBatchCommand, Message} from "@aws-sdk/client-sqs"
9+
import {LOG_MESSAGES} from "@psu-common/utilities"
910

1011
import {constructMessage, constructPSUDataItemMessage, mockSQSClient} from "./testHelpers"
1112

@@ -564,6 +565,24 @@ describe("NHS notify lambda helper functions", () => {
564565
messageBatchReference: expect.any(String),
565566
messageReference: expect.any(String)
566567
})
568+
569+
// Check that both log messages are present
570+
// TODO, first will be removed once reports updated
571+
expect(infoSpy).toHaveBeenCalledWith(
572+
"Requested notifications OK!",
573+
expect.objectContaining({
574+
messageBatchReference: expect.any(String),
575+
messageReferences: expect.any(Array),
576+
messageStatus: "requested"
577+
})
578+
)
579+
expect(infoSpy).toHaveBeenCalledWith(
580+
LOG_MESSAGES.PSU0002,
581+
expect.objectContaining({
582+
messageBatchReference: expect.any(String),
583+
messageIndex: expect.any(Number)
584+
})
585+
)
567586
})
568587

569588
it("handles non-ok response by marking all as failed", async () => {
@@ -814,6 +833,25 @@ describe("NHS notify lambda helper functions", () => {
814833
messageBatchReference: expect.any(String),
815834
messageReference: expect.any(String)
816835
})
836+
837+
// Check that both log messages are present
838+
// TODO, first will be removed once reports updated
839+
expect(infoSpy).toHaveBeenCalledWith(
840+
"Requested notifications OK!",
841+
expect.objectContaining({
842+
messageBatchReference: expect.any(String),
843+
messageReferences: expect.any(Array),
844+
messageStatus: "silent running"
845+
})
846+
)
847+
expect(infoSpy).toHaveBeenCalledWith(
848+
LOG_MESSAGES.PSU0002,
849+
expect.objectContaining({
850+
reportCode: "PSU0002",
851+
messageBatchReference: expect.any(String),
852+
messageIndex: expect.any(Number)
853+
})
854+
)
817855
})
818856
})
819857

packages/nhsNotifyUpdateCallback/src/helpers.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {getSecret} from "@aws-lambda-powertools/parameters/secrets"
88
import {createHmac, timingSafeEqual} from "crypto"
99

1010
import {LastNotificationStateType, NotificationUpdate} from "@psu-common/commonTypes"
11+
import {LOG_MESSAGES} from "@psu-common/utilities"
1112

1213
import {CallbackResource, CallbackResponse, CallbackType} from "./types"
1314

@@ -230,7 +231,7 @@ export async function updateNotificationsTable(
230231
}))
231232

232233
logger.info(
233-
"Updated notification state",
234+
LOG_MESSAGES.PSU0003,
234235
{
235236
NotifyMessageID: item.NotifyMessageID,
236237
nhsNumber: item.NHSNumber,

packages/updatePrescriptionStatus/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
"@nhs/fhir-middy-error-handler": "^2.1.59"
2626
},
2727
"devDependencies": {
28-
"@psu-common/utilities": "^1.0.0",
2928
"@faker-js/faker": "^10.1.0",
3029
"@types/fhir": "^0.0.41",
3130
"aws-sdk-client-mock": "^4.1.0"

0 commit comments

Comments
 (0)