format gateway logs as json, update express to v5 on gateway, add debug page to docs#1302
format gateway logs as json, update express to v5 on gateway, add debug page to docs#1302joshunrau wants to merge 2 commits intoDouglasNeuroInformatics:mainfrom
Conversation
WalkthroughThis pull request introduces a centralized logging system using Pino throughout the gateway application. Dependencies are updated to support Express 5.x and Pino logging. All console logging is replaced with a new logger instance, and type casting is added for route parameters. Debugging documentation is included. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/gateway/package.json`:
- Line 32: Remove the unused dependency "pino-pretty" from package.json by
deleting its entry, update the lockfile (run npm install / yarn install) to
persist the change, and verify there are no imports/usages of "pino-pretty"
across the gateway codebase; confirm logger formatting remains handled by the
custom writer in apps/gateway/src/logger.ts and run the test/build to ensure no
breakage.
In `@docs/en/2-tutorials/2.2-debugging.md`:
- Line 10: The markdown heading levels jump to H3 causing MD001 failures; change
the "Client Logs" heading (currently "### Client Logs") and the two other
section headings mentioned (lines 14 and 28) to H2 (use "##") so heading levels
increment correctly; update each heading text only (replace leading "###" with
"##") and run the linter to confirm MD001 is resolved.
- Line 25: Update the jq filter in the example docker compose logs command so it
matches pino v10's numeric level values instead of string names: replace the
filter '.level == "ERROR"' with a numeric comparison that matches pino's ERROR
level (50) so the command "docker compose logs api --no-log-prefix --since 24h |
jq ..." actually selects error entries; ensure the example uses .level == 50 (or
an equivalent numeric comparison) in the docs snippet.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/gateway/package.jsonapps/gateway/src/logger.tsapps/gateway/src/middleware/error-handler.middleware.tsapps/gateway/src/routers/api.router.tsapps/gateway/src/routers/root.router.tsapps/gateway/src/server/server.base.tsdocs/en/2-tutorials/2.2-debugging.md
| "pino-pretty": "^11.2.2", | ||
| "pino": "^10.3.1", | ||
| "pino-http": "^11.0.0", | ||
| "pino-pretty": "^13.1.3", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for pino-pretty usage in gateway
rg -n "pino-pretty|PinoPretty" apps/gateway --type ts --type js
# Also check if it's imported in logger.ts or server.base.ts
echo "=== Checking logger.ts ==="
rg -n "pino-pretty" apps/gateway/src/logger.ts
echo "=== Checking server.base.ts ==="
rg -n "pino-pretty" apps/gateway/src/server.base.ts
# Check all files for require/import of pino-pretty
echo "=== All imports of pino-pretty ==="
rg -n "require.*pino-pretty|from.*pino-pretty|import.*pino-pretty" apps/gatewayRepository: DouglasNeuroInformatics/OpenDataCapture
Length of output: 265
🏁 Script executed:
#!/bin/bash
# Examine logger.ts to understand how formatting is configured
echo "=== Checking logger.ts for pino transport setup ==="
cat -n apps/gateway/src/logger.ts | head -100Repository: DouglasNeuroInformatics/OpenDataCapture
Length of output: 388
Remove the unused pino-pretty dependency.
The package is declared in package.json but not imported or used anywhere in the gateway codebase. Logger formatting is already handled in apps/gateway/src/logger.ts via a custom write function that pretty-prints JSON output directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/gateway/package.json` at line 32, Remove the unused dependency
"pino-pretty" from package.json by deleting its entry, update the lockfile (run
npm install / yarn install) to persist the change, and verify there are no
imports/usages of "pino-pretty" across the gateway codebase; confirm logger
formatting remains handled by the custom writer in apps/gateway/src/logger.ts
and run the test/build to ensure no breakage.
|
|
||
| When debugging, it’s important to distinguish between **client-side errors** (errors in the user’s browser) and **server-side errors** (errors from your backend services). Client-side issues appear in the browser console, while server-side issues are recorded by your running containers. | ||
|
|
||
| ### Client Logs |
There was a problem hiding this comment.
Fix heading level increments to satisfy MD001.
Line 10 jumps to ### from body text; use ## section headings so heading levels increment correctly.
Proposed fix
-### Client Logs
+## Client Logs
@@
-### Server Logs
+## Server Logs
@@
-### Reporting Issues
+## Reporting IssuesAlso applies to: 14-14, 28-28
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 10-10: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/en/2-tutorials/2.2-debugging.md` at line 10, The markdown heading levels
jump to H3 causing MD001 failures; change the "Client Logs" heading (currently
"### Client Logs") and the two other section headings mentioned (lines 14 and
28) to H2 (use "##") so heading levels increment correctly; update each heading
text only (replace leading "###" with "##") and run the linter to confirm MD001
is resolved.
| The API and Gateway logs are in JSON format, so you can filter and inspect them using jq. For example, to show only error-level logs from the last 24 hours: | ||
|
|
||
| ```bash | ||
| docker compose logs api --no-log-prefix --since 24h | jq 'select(.level == "ERROR")' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for pino usage and configuration in the gateway
find . -type f \( -name "package.json" -o -name "package-lock.json" -o -name "yarn.lock" \) | head -5Repository: DouglasNeuroInformatics/OpenDataCapture
Length of output: 218
🏁 Script executed:
# Search for pino configuration or logger setup
rg "pino" --type json --type js --type ts -l | head -10Repository: DouglasNeuroInformatics/OpenDataCapture
Length of output: 261
🌐 Web query:
pino v10 log level format output numeric or string values
💡 Result:
In Pino v10, the JSON log record’s level field is numeric by default (e.g. info → 30). [1] (dash0.com)
To output string levels instead, you have two common options:
-
Enable
useLevelLabels(outputs the level “as a string instead of integer”). [2] (jsdocs.io) -
Customize
formatters.level(label, number)(default shape is{ level: number }; you can return{ level: label }to emit strings like"info"). [3] (jsdocs.io)
And if you’re using pino-pretty, it prints level labels in the pretty output (e.g. INFO). [4] (github.com)
Sources
- Dash0 guide showing default numeric
levelin Pino output. (dash0.com) - Pino v10.3.1 API docs (
useLevelLabels). (jsdocs.io) - Pino v10.3.1 API docs (
formatters.level(label, number), default{ level: number }). (jsdocs.io) pino-prettyREADME example showing labeled level output (INFO). (github.com)
Citations:
- 1: https://www.dash0.com/guides/logging-in-node-js-with-pino/?utm_source=openai
- 2: https://www.jsdocs.io/package/pino
- 3: https://www.jsdocs.io/package/pino
- 4: https://github.com/pinojs/pino-pretty?utm_source=openai
- 5: https://www.dash0.com/guides/logging-in-node-js-with-pino/?utm_source=openai
- 6: https://www.jsdocs.io/package/pino
- 7: https://www.jsdocs.io/package/pino
- 8: https://github.com/pinojs/pino-pretty?utm_source=openai
🏁 Script executed:
cat apps/gateway/src/logger.tsRepository: DouglasNeuroInformatics/OpenDataCapture
Length of output: 266
🏁 Script executed:
cat apps/gateway/package.json | grep -A 2 -B 2 "pino"Repository: DouglasNeuroInformatics/OpenDataCapture
Length of output: 323
Update jq filter to use numeric error level.
The gateway uses pino v10 with default configuration, which emits numeric level values (50 for ERROR), not strings. The filter .level == "ERROR" will match nothing.
Proposed fix
-docker compose logs api --no-log-prefix --since 24h | jq 'select(.level == "ERROR")'
+docker compose logs api --no-log-prefix --since 24h | jq 'select(.level == 50)'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docker compose logs api --no-log-prefix --since 24h | jq 'select(.level == "ERROR")' | |
| docker compose logs api --no-log-prefix --since 24h | jq 'select(.level == 50)' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/en/2-tutorials/2.2-debugging.md` at line 25, Update the jq filter in the
example docker compose logs command so it matches pino v10's numeric level
values instead of string names: replace the filter '.level == "ERROR"' with a
numeric comparison that matches pino's ERROR level (50) so the command "docker
compose logs api --no-log-prefix --since 24h | jq ..." actually selects error
entries; ensure the example uses .level == 50 (or an equivalent numeric
comparison) in the docs snippet.
1 participant
Summary by CodeRabbit