Grab correlation ID and send as a response header

[yunakim714]

Description

As part of log forwarding, we are now generating a v4 UUID Correlation ID to attach to our MRT logs so that our logs can be searched for outside of MRT. In MRT, this is now added as a new header, x-correlation-id, to the event object, which is the json representation of the HTTP request. In pwa-kit, we then need to pull this new header, modify the response, and add this header to the response so that the correlation ID value is accessible and visible to the user.

GUS: W-15706159
Linked PR to generate Correlation ID and add to Log Center logs: https://git.soma.salesforce.com/cc-mobify/portal_app/pull/5770
Linked PR to add Correlation ID to Splunk logs and log tailing: https://git.soma.salesforce.com/cc-mobify/portal_app/pull/5806

Types of Changes

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Documentation update
• Breaking change (could cause existing functionality to not work as expected)
• Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

• Removing a public function or component or prop
• Adding a required argument to a function
• Changing the data type of a function parameter or return value
• Adding a new peer dependency to package.json

Changes

• In _setRequestId(), instead of the x-api-gateway-event header, the x-correlation-id header is now the value used to set the new requestId
• In processLambdaResponse(), x-correlation-id is set as a response header if it exists

How to Test-Drive This PR

• Navigate to https://yuna-production.mobify-storefront-staging.com , which has the mrt-template-reference-app deployed to it with the changes from this branch
• Send a request
• Verify that the x-correlation-id is set as a response header

Checklists

General

• Changes are covered by test cases
• CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

• There are no changes to UI

or...

• Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
• Changes comply with WCAG 2.0 guidelines levels A and AA
• Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

• Changes include a UI text update in the Retail React App (which requires translation)

[kevinxh]

Before we merge this PR, is it possible to verify that the x-correlation-id from the incoming header, matches the SCAPI request headers, on both the server and the client side?

[kevinxh]

just double checking...

The header x-apigateway-event will still be available from MRT right? We need to make sure the old sdk versions continue to work on MRT.

[vcua-mobify]

What did we use the x-apigateway-event header for before?

Also, I see that we reference x-apigateway-event in a few other places, such as in tests. Do we need to replace those with x-correlation-id as well?

[yunakim714]

I believe that APIG is actually expected to be removed next release. @drewzboto could you confirm this?

[drewzboto]

We do intend to remove APIG. We haven't firmly committed which release it'll be done in, but definitely over the next ~6 months

[johnboxall]

This is a really important discussion.

We need to consider sites that are already live and that haven't been redeployed and sites running older versions of the PWA Kit.

In this case, we're in good shape because this code "guards" against the absence of the both the new and the old headers, but we might consider continuing to send the "old" header with our own generated value to "backport" the feature to folks using earlier versions of the PWA Kit.

This is especially important because if you look at the next line down, we log an error if we can't see this header – which means that any sites using an older version of the PWA Kit would fill up their logs with errors about this header being missing if we removed it.

[yunakim714]

Gotcha. @johnboxall Would it be okay to set res.locals.requestId to the x-apigateway-event header if x-correlation-id isn't available? Or is this undesirable as then we don't know which value res.locals.requestId is representing?

[adamraya]

I think with the correlation Id we can now safely remove these lines:

pwa-kit/packages/pwa-kit-runtime/src/ssr/server/build-remote-server.js

Lines 436 to 448 in 32094db

	// If the request has an X-Amz-Cf-Id header, log it now
	// to make it easier to associated CloudFront requests
	// with Lambda log entries. Generally we avoid logging
	// because it increases the volume of log data, but this
	// is important for log analysis.
	const cloudfrontId = req.headers['x-amz-cf-id']
	if (cloudfrontId) {
	// Log the Express app request id plus the cloudfront
	// x-edge-request-id value. The resulting line in the logs
	// will automatically include the lambda RequestId, so
	// one line links all ids.
	console.log(`Req ${res.locals.requestId} for x-edge-request-id ${cloudfrontId}`)
	}

[johnboxall]

I don't feel strongly, but an alternative place you could likely put this is here: https://git.soma.salesforce.com/cc-mobify/portal_app/blob/db4ee3d9fd3ec482d268e350e59d9287cf5b747f/.ssr-infrastructure/assets/lambdas/app.js#L35-L65

[johnboxall]

What tests would we expect to alter/update or add with this change?