fix: correct package.json path in daemon tracing#389
Conversation
The OTel tracing module used `require('../../package.json')`, which
resolves to `packages/package.json` (nonexistent) from both the source
and compiled file. The correct relative path is `../package.json`.
This caused the daemon to crash on startup in environments where OTel
was enabled and SERVICE_VERSION was not set.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughModified OpenTelemetry Resource initialization in the daemon to read Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/daemon/src/tracing.ts`:
- Line 38: The telemetry field 'service.version' currently falls back to
require('../package.json').version which is undefined because this package.json
lacks a "version"; update the value used for 'service.version' (the expression
using process.env.SERVICE_VERSION and require('../package.json').version) to
provide a safe default string (e.g., '0.0.0' or 'unknown') when neither env nor
package.json version exist, and also add a proper "version" field in the
package.json for the daemon package so telemetry reflects the real package
version.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d2bcd69-1668-4b40-aa72-d087cfc9a8c2
📒 Files selected for processing (1)
packages/daemon/src/tracing.ts
The daemon package.json has no "version" field — the monorepo root package.json is the single source of truth for the version (kept in sync with git tags). Pointing at the daemon's own package.json would still leave service.version undefined in OTel spans. Also add an 'unknown' fallback to guard against the version being missing at runtime. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/daemon/src/tracing.ts`:
- Around line 38-40: The ESLint suppression currently sits before the comment
and does not cover the require call; move the eslint-disable-next-line
`@typescript-eslint/no-var-requires` directive so it directly precedes the
require('../../../package.json').version expression (the line that assigns
'service.version') so the rule is properly suppressed for that require usage;
update the location of the comment so it sits immediately above the line
containing require('../../../package.json').version (the 'service.version'
assignment).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c84a689-0ccb-470c-848b-ef1f738631f4
📒 Files selected for processing (1)
packages/daemon/src/tracing.ts
| // The daemon package.json has no "version" field — the monorepo root | ||
| // package.json is the single source of truth (kept in sync with git tags). | ||
| 'service.version': process.env.SERVICE_VERSION || require('../../../package.json').version || 'unknown', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the eslint disable directive adjacency around the require() call.
rg -n -C1 "eslint-disable-next-line `@typescript-eslint/no-var-requires`|require\('\.\./\.\./\.\./package\.json'\)" packages/daemon/src/tracing.tsRepository: HathorNetwork/hathor-wallet-service
Length of output: 779
Move the ESLint suppression comment immediately before the require() call.
The eslint-disable-next-line directive on line 37 only suppresses the next line (line 38, a comment), not line 40 where the require() occurs. This allows @typescript-eslint/no-var-requires to trigger on the actual require statement.
Suggested fix
- // eslint-disable-next-line `@typescript-eslint/no-var-requires`
- // The daemon package.json has no "version" field — the monorepo root
- // package.json is the single source of truth (kept in sync with git tags).
- 'service.version': process.env.SERVICE_VERSION || require('../../../package.json').version || 'unknown',
+ // The daemon package.json has no "version" field — the monorepo root
+ // package.json is the single source of truth (kept in sync with git tags).
+ // eslint-disable-next-line `@typescript-eslint/no-var-requires`
+ 'service.version': process.env.SERVICE_VERSION || require('../../../package.json').version || 'unknown',🧰 Tools
🪛 ESLint
[error] 40-40: Require statement not part of import statement.
(@typescript-eslint/no-var-requires)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/daemon/src/tracing.ts` around lines 38 - 40, The ESLint suppression
currently sits before the comment and does not cover the require call; move the
eslint-disable-next-line `@typescript-eslint/no-var-requires` directive so it
directly precedes the require('../../../package.json').version expression (the
line that assigns 'service.version') so the rule is properly suppressed for that
require usage; update the location of the comment so it sits immediately above
the line containing require('../../../package.json').version (the
'service.version' assignment).
|
Good catch @coderabbitai — you're right that Pushed 8102f35 to address this. Rather than adding a Also added Verified empirically against the actual Docker build:
|
|
Tip For best results, initiate chat on the files or code changes.
The approach is sound:
✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
3 participants
Motivation
The daemon crashed on startup in the
devenvironment with:This was triggered by the OTel tracing module added in #383. At
packages/daemon/src/tracing.ts:38,require('../../package.json')resolves topackages/package.json(nonexistent) from both the source and the compileddist/tracing.js. The correct relative path is../package.json, which resolves topackages/daemon/package.json.The fallback path always runs because
SERVICE_VERSIONis not set in any environment. Dev fired first because it was deployed withOTEL_EXPORTER_OTLP_ENDPOINTset (OTel enabled); prod would hit the same crash as soon as #383 is rolled out.The sibling
packages/wallet-service/src/tracing.tsdoes not setservice.versionat all, so it is not affected.Acceptance Criteria
packages/daemon/src/tracing.tsusesrequire('../package.json')soservice.versionresolves correctly at runtimedevenvironment with OTel enabled (noCannot find moduleerror)service.versionreported in OTel spans matches the version inpackages/daemon/package.jsonChecklist
master, confirm this code is production-ready and can be included in future releases as soon as it gets mergedGenerated with Claude Code
Summary by CodeRabbit