Feat/logtail pino #427
Feat/logtail pino #427
Conversation
WalkthroughThe changes introduce structured logging and heartbeat monitoring to both the relayer and validator CLI tools. New environment variables ( Changes
Sequence Diagram(s)Heartbeat Integration (Relayer/Validator)sequenceDiagram
participant CLI
participant Env
participant Logger
participant MonitoringService
CLI->>Env: Load HEARTBEAT_URL
CLI->>Logger: Initialize logger (with LOGTAIL_TOKEN if set)
CLI->>MonitoringService: sendHeartbeat() at startup
loop Main Loop
CLI->>MonitoringService: sendHeartbeat() periodically
CLI->>Logger: Log events
end
CLI->>MonitoringService: sendHeartbeat() at shutdown
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for veascan ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
validator-cli/src/utils/env.ts (1)
2-8: Keep logic consistent with relayer fixIf you accept the tightened
value === undefinedcheck in the relayer utility, mirror it here to prevent subtle behavioural differences between the two CLIs.relayer-cli/src/utils/logger.ts (1)
17-36: Same dependency / env handling concernsReplicate the fixes applied to the validator logger:
- Move
pino-prettyto runtime deps.- Fetch
LOGTAIL_TOKENvia theenvhelper for consistency.
🧹 Nitpick comments (6)
relayer-cli/src/utils/env.ts (2)
2-8: Empty string is treated as “missing” – is that intentional?
if (!value)rejects an env var that is set but intentionally blank (e.g.VAR=""). If you only want to fail when the key is truly undefined, checkvalue === undefinedinstead.- if (!value) { + if (value === undefined) {
1-13: Avoid copy-pasted utilities across packagesAn identical
envhelper is introduced in two different packages. Duplicating code increases maintenance cost and the risk of divergence. Export a small shared package (e.g.@kleros/vea-env) or move the helper to the monorepo root and import it from both CLI tools.validator-cli/.env.dist (1)
24-26: Order keys & keep sample files tidyThe linter warning is legit: place
HEARTBEAT_URLbeforeLOGTAIL_TOKENto keep alphabetical order and minimise merge noise.-LOGTAIL_TOKEN= -HEARTBEAT_URL= +HEARTBEAT_URL= +LOGTAIL_TOKEN=relayer-cli/.env.dist (1)
26-28: Alphabetical order & trailing newline
- Alphabetise:
HEARTBEAT_URLshould precedeLOGTAIL_TOKEN.- Add a trailing blank line to satisfy common linters and POSIX tools.
-LOGTAIL_TOKEN= -HEARTBEAT_URL= +HEARTBEAT_URL= +LOGTAIL_TOKEN= +validator-cli/src/watcher.ts (1)
88-96: Tight loop may overload monitoring endpoint
sendHeartbeat()is called every loop iteration (≈ 10 s) per watched network.
If multiple networks are configured the frequency scales linearly and could breach rate-limits.
Consider extracting a singlesetInterval-based heartbeat or using a token-bucket.validator-cli/src/utils/logger.ts (1)
19-40: Runtime dependency & env util consistency
pino-prettyis loaded viatransport– ensure it’s under"dependencies"(see package.json comment).- Prefer
env.optional("LOGTAIL_TOKEN")instead of readingprocess.envdirectly to keep all env look-ups centralised.-const logtailToken = process.env.LOGTAIL_TOKEN; +const logtailToken = env.optional("LOGTAIL_TOKEN");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
relayer-cli/.env.dist(1 hunks)relayer-cli/package.json(1 hunks)relayer-cli/src/relayer.ts(4 hunks)relayer-cli/src/utils/env.ts(1 hunks)relayer-cli/src/utils/logger.ts(2 hunks)validator-cli/.env.dist(1 hunks)validator-cli/package.json(1 hunks)validator-cli/src/utils/env.ts(1 hunks)validator-cli/src/utils/logger.ts(3 hunks)validator-cli/src/watcher.ts(3 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
relayer-cli/.env.dist
[warning] 24-24: [QuoteCharacter] The value has quote characters (', ")
[warning] 28-28: [EndingBlankLine] No blank line at the end of the file
[warning] 28-28: [UnorderedKey] The HEARTBEAT_URL key should go before the LOGTAIL_TOKEN key
validator-cli/.env.dist
[warning] 26-26: [UnorderedKey] The HEARTBEAT_URL key should go before the LOGTAIL_TOKEN key
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Redirect rules - veascan
- GitHub Check: Header rules - veascan
- GitHub Check: Pages changed - veascan
- GitHub Check: Analyze (javascript)
- GitHub Check: Scorecard analysis
- GitHub Check: test
- GitHub Check: dependency-review
| "@logtail/pino": "^0.5.5", | ||
| "@typechain/ethers-v6": "^0.5.1", | ||
| "dotenv": "^16.4.5", | ||
| "pino": "^9.7.0", | ||
| "pm2": "^6.0.5", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin critical runtime deps instead of using caret ranges
@logtail/pino and pino are pulled in at ^ ranges. Both libraries publish frequent breaking‐change majors; letting npm float to the next major on every fresh install can silently break the CLI in production. Consider hard-pinning to the exact, tested versions and bump them deliberately in a dedicated PR.
- "@logtail/pino": "^0.5.5",
+ "@logtail/pino": "0.5.5",
...
- "pino": "^9.7.0",
+ "pino": "9.7.0",📝 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.
| "@logtail/pino": "^0.5.5", | |
| "@typechain/ethers-v6": "^0.5.1", | |
| "dotenv": "^16.4.5", | |
| "pino": "^9.7.0", | |
| "pm2": "^6.0.5", | |
| "@logtail/pino": "0.5.5", | |
| "@typechain/ethers-v6": "^0.5.1", | |
| "dotenv": "^16.4.5", | |
| "pino": "9.7.0", | |
| "pm2": "^6.0.5", |
🤖 Prompt for AI Agents
In relayer-cli/package.json around lines 17 to 21, the dependencies
@logtail/pino and pino use caret (^) version ranges, which can cause automatic
upgrades to breaking major versions. Change these version specifications to
exact versions by removing the caret prefix, pinning them to the currently
tested versions to prevent unintended breaking changes during installs. Update
these versions deliberately in future PRs when needed.
| STATE_DIR="/home/user/vea/relayer-cli/state/" |
There was a problem hiding this comment.
Avoid quoting values in sample .env
The quotes around STATE_DIR will be part of the variable value when parsed by dotenv, which is rarely what you want.
-STATE_DIR="/home/user/vea/relayer-cli/state/"
+STATE_DIR=/home/user/vea/relayer-cli/state/📝 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.
| STATE_DIR="/home/user/vea/relayer-cli/state/" | |
| STATE_DIR=/home/user/vea/relayer-cli/state/ |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 24-24: [QuoteCharacter] The value has quote characters (', ")
🤖 Prompt for AI Agents
In relayer-cli/.env.dist at line 24, remove the double quotes around the
STATE_DIR value so that the path is assigned without quotes. This prevents the
quotes from being included in the environment variable value when parsed by
dotenv.
| "devDependencies": { | ||
| "@types/jest": "^29.5.14", | ||
| "jest": "^29.7.0", | ||
| "pino-pretty": "^13.0.0", | ||
| "ts-jest": "^29.2.5", | ||
| "ts-node": "^10.9.2" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
pino-pretty must be a runtime dependency
pino-pretty is resolved at runtime via the transport declared in logger.ts. Packaging it under devDependencies means it will be absent in production installs (yarn --prod, Docker multi-stage builds, etc.) and the app will throw on bootstrap (ERR_MODULE_NOT_FOUND: pino-pretty).
Move it to dependencies.
"devDependencies": {
- "pino-pretty": "^13.0.0",
+ },
+ "dependencies": {
+ "pino-pretty": "^13.0.0",Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In validator-cli/package.json around lines 26 to 32, move "pino-pretty" from
"devDependencies" to "dependencies" because it is required at runtime by the
transport in logger.ts. This ensures it is installed in production environments
and prevents runtime module not found errors.
| const sendHeartbeat = async (): Promise<void> => { | ||
| const HEARTBEAT_URL = process.env.HEARTBEAT_URL; | ||
| if (!HEARTBEAT_URL) { | ||
| return; | ||
| } | ||
| try { | ||
| const url = new URL(HEARTBEAT_URL); | ||
| const options = { | ||
| hostname: url.hostname, | ||
| port: url.port || 443, | ||
| path: url.pathname, | ||
| method: "GET", | ||
| headers: { | ||
| "User-Agent": "Vea-Validator-CLI/1.0", | ||
| }, | ||
| }; | ||
|
|
||
| return new Promise((resolve, reject) => { | ||
| const req = https.request(options, (res) => { | ||
| let data = ""; | ||
| res.on("data", (chunk) => { | ||
| data += chunk; | ||
| }); | ||
| res.on("end", () => { | ||
| resolve(); | ||
| }); | ||
| }); | ||
|
|
||
| req.on("error", (error) => { | ||
| reject(error); | ||
| }); | ||
|
|
||
| req.setTimeout(10000, () => { | ||
| req.destroy(); | ||
| reject(new Error("Heartbeat request timeout")); | ||
| }); | ||
|
|
||
| req.end(); | ||
| }); | ||
| } catch (error) { | ||
| // Silently fail - heartbeat errors shouldn't affect the main relayer operation | ||
| } | ||
| }; |
There was a problem hiding this comment.
Heartbeat promise may crash the bot
Inside sendHeartbeat you wrap the sync code in a try/catch, but the rejections coming from the returned Promise are not caught, e.g.:
- network error (
req.on("error")) - timeout (
setTimeoutbranch)
These propagate to await sendHeartbeat() and can terminate the whole watcher with an unhandled-rejection.
- await sendHeartbeat();
+ await sendHeartbeat().catch(() => {
+ // best-effort – ignore heartbeat failures
+ });Additionally, options.path omits query-strings. Use url.pathname + url.search.
Silently fixing both avoids unexpected downtime.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In validator-cli/src/watcher.ts lines 24 to 66, the sendHeartbeat function's
returned Promise can reject on network errors or timeouts, but these rejections
are not caught within the function, risking unhandled promise rejections that
can crash the bot. To fix this, handle the Promise rejections inside
sendHeartbeat by adding a catch block or wrapping the call site with try/catch.
Also, update the options.path to include both url.pathname and url.search to
preserve query strings in the heartbeat request URL.
| // Send heartbeat before waiting | ||
| await sendHeartbeat(); | ||
|
|
There was a problem hiding this comment.
Await without catch
Same crash vector on periodic heartbeat:
- await sendHeartbeat();
+ await sendHeartbeat().catch(() => {});📝 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.
| // Send heartbeat before waiting | |
| await sendHeartbeat(); | |
| // Send heartbeat before waiting | |
| await sendHeartbeat().catch(() => {}); |
🤖 Prompt for AI Agents
In relayer-cli/src/relayer.ts around lines 91 to 93, the await call to
sendHeartbeat() is not wrapped in a try-catch block, which can cause the process
to crash if sendHeartbeat throws an error. To fix this, wrap the await
sendHeartbeat() call in a try-catch block and handle or log any errors to
prevent unhandled promise rejections and crashes.
| /** | ||
| * Sends a heartbeat signal to the monitoring service | ||
| */ | ||
| const sendHeartbeat = async (): Promise<void> => { | ||
| const HEARTBEAT_URL = process.env.HEARTBEAT_URL; | ||
| if (!HEARTBEAT_URL) { | ||
| return; | ||
| } | ||
| try { | ||
| const url = new URL(HEARTBEAT_URL); | ||
| const options = { | ||
| hostname: url.hostname, | ||
| port: url.port || 443, | ||
| path: url.pathname, | ||
| method: "GET", | ||
| headers: { | ||
| "User-Agent": "Vea-Validator-CLI/1.0", | ||
| }, | ||
| }; | ||
|
|
||
| return new Promise((resolve, reject) => { | ||
| const req = https.request(options, (res) => { | ||
| let data = ""; | ||
| res.on("data", (chunk) => { | ||
| data += chunk; | ||
| }); | ||
| res.on("end", () => { | ||
| resolve(); | ||
| }); | ||
| }); | ||
|
|
||
| req.on("error", (error) => { | ||
| reject(error); | ||
| }); | ||
|
|
||
| req.setTimeout(10000, () => { | ||
| req.destroy(); | ||
| reject(new Error("Heartbeat request timeout")); | ||
| }); | ||
|
|
||
| req.end(); | ||
| }); | ||
| } catch (error) { | ||
| // Silently fail - heartbeat errors shouldn't affect the main relayer operation | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same unhandled-rejection issue and duplicated implementation
- The Promise returned by
sendHeartbeatcan reject → potential crash. Apply the same guard as suggested for the validator. User-AgentclaimsVea-Validator-CLI/1.0; copy-paste artefact.- Nearly identical code now exists in two places. Extract to a shared util (
heartbeat.ts) to avoid divergence.
- "User-Agent": "Vea-Validator-CLI/1.0",
+ "User-Agent": "Vea-Relayer-CLI/1.0",📝 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.
| /** | |
| * Sends a heartbeat signal to the monitoring service | |
| */ | |
| const sendHeartbeat = async (): Promise<void> => { | |
| const HEARTBEAT_URL = process.env.HEARTBEAT_URL; | |
| if (!HEARTBEAT_URL) { | |
| return; | |
| } | |
| try { | |
| const url = new URL(HEARTBEAT_URL); | |
| const options = { | |
| hostname: url.hostname, | |
| port: url.port || 443, | |
| path: url.pathname, | |
| method: "GET", | |
| headers: { | |
| "User-Agent": "Vea-Validator-CLI/1.0", | |
| }, | |
| }; | |
| return new Promise((resolve, reject) => { | |
| const req = https.request(options, (res) => { | |
| let data = ""; | |
| res.on("data", (chunk) => { | |
| data += chunk; | |
| }); | |
| res.on("end", () => { | |
| resolve(); | |
| }); | |
| }); | |
| req.on("error", (error) => { | |
| reject(error); | |
| }); | |
| req.setTimeout(10000, () => { | |
| req.destroy(); | |
| reject(new Error("Heartbeat request timeout")); | |
| }); | |
| req.end(); | |
| }); | |
| } catch (error) { | |
| // Silently fail - heartbeat errors shouldn't affect the main relayer operation | |
| } | |
| }; | |
| /** | |
| * Sends a heartbeat signal to the monitoring service | |
| */ | |
| const sendHeartbeat = async (): Promise<void> => { | |
| const HEARTBEAT_URL = process.env.HEARTBEAT_URL; | |
| if (!HEARTBEAT_URL) { | |
| return; | |
| } | |
| try { | |
| const url = new URL(HEARTBEAT_URL); | |
| const options = { | |
| hostname: url.hostname, | |
| port: url.port || 443, | |
| path: url.pathname, | |
| method: "GET", | |
| headers: { | |
| "User-Agent": "Vea-Relayer-CLI/1.0", | |
| }, | |
| }; | |
| return new Promise((resolve, reject) => { | |
| const req = https.request(options, (res) => { | |
| let data = ""; | |
| res.on("data", (chunk) => { | |
| data += chunk; | |
| }); | |
| res.on("end", () => { | |
| resolve(); | |
| }); | |
| }); | |
| req.on("error", (error) => { | |
| reject(error); | |
| }); | |
| req.setTimeout(10000, () => { | |
| req.destroy(); | |
| reject(new Error("Heartbeat request timeout")); | |
| }); | |
| req.end(); | |
| }); | |
| } catch (error) { | |
| // Silently fail - heartbeat errors shouldn't affect the main relayer operation | |
| } | |
| }; |
🤖 Prompt for AI Agents
In relayer-cli/src/relayer.ts between lines 25 and 70, the sendHeartbeat
function returns a Promise that can reject without being caught, risking
unhandled rejections; fix this by adding a catch block or handling the rejection
where sendHeartbeat is called. Also, update the User-Agent header to correctly
reflect the relayer CLI instead of the copied "Vea-Validator-CLI/1.0" string.
Finally, extract this heartbeat sending logic into a shared utility file (e.g.,
heartbeat.ts) to avoid code duplication and ensure maintainability.
PR-Codex overview
This PR introduces monitoring capabilities to the
validator-cliandrelayer-cliprojects by adding heartbeat signals and logging features. It enhances environment variable management and updates dependencies to support the new functionality.Detailed summary
LOGTAIL_TOKENandHEARTBEAT_URLto.env.distfiles for monitoring.sendHeartbeatfunction inrelayer-cli/src/watcher.tsandrelayer-cli/src/relayer.tsfor sending heartbeat signals.watchandstartfunctions.relayer-cli/src/utils/logger.tsandvalidator-cli/src/utils/logger.tsto usepinofor structured logging.package.jsonfiles to include new dependencies:@logtail/pino,pino, andpino-pretty.envutility for managing environment variables in bothrelayer-cli/src/utils/env.tsandvalidator-cli/src/utils/env.ts.Summary by CodeRabbit
New Features
LOGTAIL_TOKEN,HEARTBEAT_URL).Refactor
pinolibrary, with optional Logtail integration for advanced log management.Chores