Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: custom spans #2287

Merged
merged 17 commits into from
Nov 27, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: add more spans
KishenKumarrrrr committed Nov 25, 2024
commit 3af955f1b188c01c5a4b49f333f49878da63fcc6
5 changes: 3 additions & 2 deletions backend/src/email/services/email-callback.service.ts
Original file line number Diff line number Diff line change
@@ -25,8 +25,9 @@ const isAuthenticated = (authHeader?: string): boolean => {
}

const parseEvent = async (req: Request): Promise<void> => {
const parentSpan = tracer.scope().active() || undefined
const parseJsonSpan = tracer.startSpan('parseJson', { childOf: parentSpan })
const parseJsonSpan = tracer.startSpan('parseJson', {
childOf: tracer.scope().active() || undefined,
})
const parsed = JSON.parse(req.body)
parseJsonSpan.finish()
let records: Promise<void>[] = []
121 changes: 62 additions & 59 deletions backend/src/email/utils/callback/parsers/ses.ts
Original file line number Diff line number Diff line change
@@ -186,30 +186,28 @@ const validateRecord = async (
record: SesRecord,
smtpHeader: SmtpApiHeader | undefined
) => {
tracer.wrap('validateRecord', async () => {
const username = smtpHeader?.auth?.username
const hash = smtpHeader?.auth?.hash
const username = smtpHeader?.auth?.username
const hash = smtpHeader?.auth?.hash
if (
!username ||
!hash ||
!compareSha256Hash(config.get('emailCallback.hashSecret'), username, hash)
) {
logger.info({
message: 'Incorrect email callback hash',
username,
timestamp: record.Timestamp,
hash,
})

// if not passed with the new hash, retry with the old way
// TODO: remove this after all campaigns sent with the old way have completed
if (
!username ||
!hash ||
!compareSha256Hash(config.get('emailCallback.hashSecret'), username, hash)
!(record.SignatureVersion === '1' && (await validateSignature(record)))
) {
logger.info({
message: 'Incorrect email callback hash',
username,
timestamp: record.Timestamp,
hash,
})

// if not passed with the new hash, retry with the old way
// TODO: remove this after all campaigns sent with the old way have completed
if (
!(record.SignatureVersion === '1' && (await validateSignature(record)))
) {
throw new Error('Invalid record')
}
throw new Error('Invalid record')
}
})
}
}
const blacklistIfNeeded = async (message: any): Promise<void> => {
const notificationType = message?.notificationType || message?.eventType
@@ -228,48 +226,53 @@ const blacklistIfNeeded = async (message: any): Promise<void> => {
}
}
const parseRecord = async (record: SesRecord): Promise<void> => {
tracer.wrap('parseRecord', async () => {
logger.info({
message: 'Parsing SES callback record',
})
const parseRecordJson = tracer.startSpan('parseRecordJson')
const message = JSON.parse(record.Message)
parseRecordJson.finish()
const smtpApiHeader = getSmtpApiHeader(message)
await validateRecord(record, smtpApiHeader)

// Transactional emails don't have message IDs, so blacklist
// relevant email addresses before everything else
await blacklistIfNeeded(message)
const parseRecordSpan = tracer.startSpan('parseRecord', {
childOf: tracer.scope().active() || undefined,
})
logger.info({
message: 'Parsing SES callback record',
})
const parseRecordJson = tracer.startSpan('parseRecordJson')
const message = JSON.parse(record.Message)
parseRecordJson.finish()
const smtpApiHeader = getSmtpApiHeader(message)
const validateRecordSpan = tracer.startSpan('validateRecord', {
childOf: tracer.scope().active() || undefined,
})
await validateRecord(record, smtpApiHeader)
validateRecordSpan.finish()
// Transactional emails don't have message IDs, so blacklist
// relevant email addresses before everything else
const blacklistIfNeededSpan = tracer.startSpan('blacklistIfNeeded', {
childOf: tracer.scope().active() || undefined,
})
await blacklistIfNeeded(message)
blacklistIfNeededSpan.finish()

// primary key
const messageId = smtpApiHeader?.unique_args?.message_id
const isTransactional = smtpApiHeader?.isTransactional
const type = message?.notificationType || message?.eventType
// primary key
const messageId = smtpApiHeader?.unique_args?.message_id
const isTransactional = smtpApiHeader?.isTransactional
const type = message?.notificationType || message?.eventType

if (messageId && type) {
const metadata = { messageId, timestamp: record.Timestamp }
logger.info({
message: 'Update for notification/event type',
action: 'parseRecord',
messageId,
type,
if (messageId && type) {
const metadata = { messageId, timestamp: record.Timestamp }
logger.info({
message: 'Update for notification/event type',
action: 'parseRecord',
messageId,
type,
})
if (isTransactional) {
return EmailTransactionalService.handleStatusCallbacks(type, messageId, {
timestamp: new Date(record.Timestamp),
bounce: message.bounce,
complaint: message.complaint,
delivery: message.delivery,
})
if (isTransactional) {
return EmailTransactionalService.handleStatusCallbacks(
type,
messageId,
{
timestamp: new Date(record.Timestamp),
bounce: message.bounce,
complaint: message.complaint,
delivery: message.delivery,
}
)
}
return parseNotificationAndEvent(type, message, metadata)
}
})
return parseNotificationAndEvent(type, message, metadata)
}
parseRecordSpan.finish()
}

// Checks whether the notification/event is meant for the main recipient of the email.

Unchanged files with check annotations Beta

void fetchGovsgMessages(search, selectedPage).finally(() => {
setLoading(false)
})
}, [])

Check warning on line 112 in frontend/src/components/dashboard/create/govsg/GovsgMessages.tsx

GitHub Actions / test-frontend

React Hook useEffect has missing dependencies: 'fetchGovsgMessages', 'search', and 'selectedPage'. Either include them or remove the dependency array
const handlePageChange = (index: number) => {
void fetchGovsgMessages(search, index)
campaignId: number
}): (data: CSVParams[]) => Promise<void> {
return async function (data: CSVParams[]): Promise<void> {
const paramsWithoutPasscode = template.params!.filter(

Check warning on line 112 in backend/src/govsg/services/govsg.service.ts

GitHub Actions / test-backend

Forbidden non-null assertion

Check warning on line 112 in backend/src/govsg/services/govsg.service.ts

GitHub Actions / Deploy backend to AWS Elastic Beanstalk / lint-test

Forbidden non-null assertion
(param) => param !== 'passcode'
) // TODO: Un-hardcode this
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion