Added request ID to the request logs

[JoeeGrigg]

ref NY-363

This ID is passed in as the x-request-id header to requests.

[cursor]

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on February 3.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[coderabbitai]

Walkthrough

The logging plugin's onRequest hook now reads x-request-id, normalizes it to a single requestId value, merges that with any existing traceContext into a single childContext, and attaches that merged context to the per-request logger when non-empty. Log creation uses the consolidated childContext. onResponse logging is unchanged. A new Vitest suite exercises inclusion/exclusion of requestId and combined handling of Google Cloud trace context with GOOGLE_CLOUD_PROJECT.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding request ID to request logs, which aligns with the changeset modifications in the logging plugin.
Description check	✅ Passed	The description is directly related to the changeset, explaining that a request ID from the x-request-id header is added to request logs.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5377a20 and 006d15e.

📒 Files selected for processing (2)

• src/plugins/logging.ts
• test/unit/plugins/logging.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• test/unit/plugins/logging.test.ts

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Build TypeScript code using yarn build
Run linter using yarn lint
Use TypeScript with strict mode enabled for all source code

Files:

• src/plugins/logging.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}: Ensure all new code has test coverage when adding new features
Use Fastify for HTTP handling in the service
Implement preValidation hook for HMAC validation and preHandler hook for processing user agent and referrer data in the request flow

Files:

• src/plugins/logging.ts

🔇 Additional comments (2)

src/plugins/logging.ts (2)

11-21: LGTM! Clean implementation of request ID extraction and context merging.

The array handling for the x-request-id header correctly addresses the string | string[] | undefined type, and the conditional spread pattern for building childContext is idiomatic. The guard on line 19 appropriately avoids creating an unnecessary child logger when no context is present.

---

54-56: No concerns with the export.

The fp wrapper pattern is correctly applied to make the logging hooks global across all routes, as documented in the comment.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/plugins/logging.ts`:
- Line 11: The code assumes request.headers['x-request-id'] is a string; update
the extraction for requestId to handle string | string[] | undefined by reading
the raw header into a variable (e.g., rawRequestId) and then set requestId to
the first element if Array.isArray(rawRequestId), otherwise use rawRequestId (or
undefined) so you never log an array; update any references to requestId
accordingly.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f9efa5 and 44e0878.

📒 Files selected for processing (1)

• src/plugins/logging.ts

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Build TypeScript code using yarn build
Run linter using yarn lint
Use TypeScript with strict mode enabled for all source code

Files:

• src/plugins/logging.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}: Ensure all new code has test coverage when adding new features
Use Fastify for HTTP handling in the service
Implement preValidation hook for HMAC validation and preHandler hook for processing user agent and referrer data in the request flow

Files:

• src/plugins/logging.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Lint and Test (Node 20)
• GitHub Check: Lint and Test (Node 22)

🔇 Additional comments (1)

src/plugins/logging.ts (1)

13-19: LGTM!

The child context creation pattern is clean. The conditional spread for requestId and the check for non-empty context before creating a child logger are both appropriate.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@test/unit/plugins/logging.test.ts`:
- Around line 9-39: Tests create a Fastify instance in beforeEach (app) but
never close it, causing resource leaks; add an afterEach hook that awaits
closing the Fastify app (await app.close()) and guards against undefined app (if
(app) await app.close()) to ensure the server and handles are cleaned up after
each test; update the test file to register this afterEach alongside the
existing beforeEach and keep referencing the same app and loggingPlugin symbols.

♻️ Duplicate comments (1)

src/plugins/logging.ts (1)

11-11: Handle potential array value for x-request-id header.

HTTP headers can appear multiple times, resulting in string | string[] type. The current cast doesn't safely handle the array case.

This concern was raised in a previous review. Consider the suggested fix:

-const requestId = request.headers['x-request-id'] as string | undefined;
+const requestIdHeader = request.headers['x-request-id'];
+const requestId = Array.isArray(requestIdHeader) ? requestIdHeader[0] : requestIdHeader;

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 44e0878 and ce1e73c.

📒 Files selected for processing (2)

• src/plugins/logging.ts
• test/unit/plugins/logging.test.ts

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Build TypeScript code using yarn build
Run linter using yarn lint
Use TypeScript with strict mode enabled for all source code

Files:

• test/unit/plugins/logging.test.ts
• src/plugins/logging.ts

**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Run tests using yarn test for all tests, yarn test:unit for unit tests, yarn test:integration for integration tests, and yarn test:e2e for e2e tests

Files:

• test/unit/plugins/logging.test.ts

test/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

test/**/*.{ts,tsx}: Test files must use Vitest framework and follow the same directory structure as source code with corresponding test files in the test/ directory
Do not add constant timeouts to tests

Files:

• test/unit/plugins/logging.test.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}: Ensure all new code has test coverage when adding new features
Use Fastify for HTTP handling in the service
Implement preValidation hook for HMAC validation and preHandler hook for processing user agent and referrer data in the request flow

Files:

• src/plugins/logging.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Lint and Test (Node 20)
• GitHub Check: Lint and Test (Node 22)

🔇 Additional comments (3)

src/plugins/logging.ts (1)

13-19: LGTM!

The context merging logic is clean - spreading traceContext and conditionally including requestId ensures both pieces of correlation data are captured when present. The non-empty check before creating the child logger avoids unnecessary overhead.

test/unit/plugins/logging.test.ts (2)

24-29: Neat approach for intercepting child logger bindings.

Wrapping app.log.child to capture bindings is a clean way to verify log context without needing to parse actual log output. This keeps tests fast and deterministic.

---

74-102: LGTM on environment variable handling.

Properly using try/finally to restore GOOGLE_CLOUD_PROJECT ensures test isolation regardless of assertion failures.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[github-actions]

✅ Deployed to staging (tree: 621c4e89810200b1f191ce0757ce78d84f8beb79)

[JoeeGrigg]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.