feat: add @evlog/telemetry package - #417
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces ChangesTelemetry package
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant TelemetryHandle
participant Outbox
participant IngestEndpoint
CLI->>TelemetryHandle: execute command
TelemetryHandle->>Outbox: store sanitized run event
TelemetryHandle->>IngestEndpoint: POST buffered events
IngestEndpoint-->>TelemetryHandle: return validation/delivery result
TelemetryHandle->>Outbox: remove delivered events
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
|
Thank you for following the naming conventions! 🙏 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
commit: |
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/telemetry-playground/smoke.ts`:
- Around line 17-20: Update outboxPath to use logical OR when selecting the
XDG_CONFIG_HOME value, so an empty or otherwise falsy environment variable falls
back to join(homedir(), '.config') while preserving the existing path
construction.
In `@packages/telemetry/src/create.ts`:
- Around line 213-222: Update disableTelemetry and enableTelemetry so consent
changes also modify the active telemetry instance’s runtime state, rather than
only persisting the preference. Ensure disabling stops recording and delivery
before purgeOutbox runs, preventing the outbox from being recreated, and
enabling reactivates an existing disabled instance while preserving the
persisted preference behavior.
- Around line 76-79: Update the context construction in the telemetry creation
flow to sanitize opts?.systemCustom before assigning ctx.custom. Reuse the
existing sanitizeCustom mechanism if available, configured with an explicit
allowlist and bounded keys, string lengths, and values; ensure no raw
systemCustom fields bypass sanitization while preserving the expected custom
record type.
In `@packages/telemetry/src/idempotency.ts`:
- Around line 20-27: Update fnv1a32 to encode input with TextEncoder and iterate
over the resulting UTF-8 bytes, removing the charCodeAt-based truncation while
preserving the seeded FNV-1a accumulation and unsigned return value.
In `@packages/telemetry/src/outbox.ts`:
- Around line 57-67: Update readAll() to validate each parsed StoredEvent’s
storedAt timestamp and exclude events whose age exceeds maxEventAgeMs before
adding them to the returned events. Preserve skipping malformed JSON and only
enqueue valid run events that are within the configured retention window.
- Around line 6-10: Add JSDoc for the exported contracts at
packages/telemetry/src/outbox.ts lines 6-10, documenting option units, defaults,
and behavior; packages/telemetry/src/drain.ts line 3, documenting
delivery-result semantics; and packages/telemetry/src/consent.ts line 6,
documenting each preference state. Ensure all public API documentation is
concise and reflects the existing runtime behavior.
- Around line 39-42: The outbox mutations must share one synchronization
mechanism to prevent append, rewrite, purge, and unlink races. In
packages/telemetry/src/outbox.ts at lines 39-42, update the append flow to
acquire the existing shared lock before writing; at lines 107-113, acquire that
same lock before unlinking. In packages/telemetry/src/consent.ts at lines 45-47,
route consent purging through the synchronized outbox abstraction instead of
performing an unsynchronized direct mutation.
- Around line 160-173: Update the withLock method to recover abandoned lock
files: add bounded acquisition retries, and when the existing outbox.lock is
stale, verify its ownership/expiry metadata before removing and reclaiming it.
Preserve exclusive locking for active owners, ensure retries eventually fail
rather than loop indefinitely, and keep cleanup limited to locks owned by this
mechanism.
In `@packages/telemetry/src/paths.ts`:
- Around line 5-7: Update getTelemetryDir so an empty XDG_CONFIG_HOME is treated
like an unset value before selecting the base directory. Preserve the existing
XDG_CONFIG_HOME path for non-empty values and continue falling back to
homedir()/.config otherwise.
In `@packages/telemetry/src/types.ts`:
- Around line 94-116: Update CustomFields and TelemetryHandle in types.ts to
support declared generic custom fields, including string values for allowlisted
fields. Add the public set method used by createTelemetry, parameterized with
the handle’s custom-field type so declared fields receive autocomplete while
preserving the existing telemetry API.
In `@packages/telemetry/test/telemetry.test.ts`:
- Around line 18-22: Update the beforeEach setup to remove XDG_CONFIG_HOME with
delete instead of assigning undefined, while preserving the existing cleanup of
DO_NOT_TRACK and EVLOG_TELEMETRY so the variable is truly absent during each
test.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c891aef7-f79c-4136-8eaf-63abe50a1440
⛔ Files ignored due to path filters (2)
packages/telemetry/test/__snapshots__/telemetry.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
.changeset/telemetry-package.md.github/pull_request_template.md.github/workflows/ci.yml.github/workflows/semantic-pull-request.ymlapps/docs/content/4.use-cases/0.overview.mdapps/docs/content/4.use-cases/6.telemetry/01.overview.mdexamples/telemetry-playground/cli.tsexamples/telemetry-playground/package.jsonexamples/telemetry-playground/smoke.tspackage.jsonpackages/telemetry/README.mdpackages/telemetry/package.jsonpackages/telemetry/src/citty.tspackages/telemetry/src/commands.tspackages/telemetry/src/consent.tspackages/telemetry/src/create.tspackages/telemetry/src/disclosure.tspackages/telemetry/src/drain.tspackages/telemetry/src/enrich.tspackages/telemetry/src/github-actions.tspackages/telemetry/src/idempotency.tspackages/telemetry/src/index.tspackages/telemetry/src/notice.tspackages/telemetry/src/outbox.tspackages/telemetry/src/paths.tspackages/telemetry/src/sanitize.tspackages/telemetry/src/storage.tspackages/telemetry/src/types.tspackages/telemetry/test/telemetry-typing.test.tspackages/telemetry/test/telemetry.test.tspackages/telemetry/tsconfig.jsonpackages/telemetry/tsdown.config.tspackages/telemetry/vitest.config.ts
@evlog/telemetry package
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/telemetry-playground/smoke.ts (1)
31-37: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDo not swallow every outbox read failure.
The catch currently treats permission errors, invalid paths, and filesystem failures as “opt-out or first run,” so the smoke check can pass while persistence is broken. Handle only
ENOENTas an expected missing-outbox case; rethrow or fail for other errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/telemetry-playground/smoke.ts` around lines 31 - 37, Update the outbox read catch around readFile in the smoke check to handle only ENOENT as the expected missing-outbox case. Preserve the zero-line behavior for a nonexistent outbox, but rethrow or otherwise fail for permission, invalid-path, and other filesystem errors instead of swallowing them.packages/telemetry/README.md (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix grammatical error.
Change "a analytics" to "an analytics".
📝 Proposed fix
-Ship usage insight without shipping a analytics SDK: wrap your [citty](https://github.com/unjs/citty) command tree (or call `createTelemetry()` in scripts) and get **one structured event per command** — command name, sanitized flags, duration, outcome, and optional counters via `telemetry.set()`. +Ship usage insight without shipping an analytics SDK: wrap your [citty](https://github.com/unjs/citty) command tree (or call `createTelemetry()` in scripts) and get **one structured event per command** — command name, sanitized flags, duration, outcome, and optional counters via `telemetry.set()`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/telemetry/README.md` at line 18, In the README description, correct the article before “analytics” from “a” to “an” while preserving the rest of the usage explanation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/telemetry-package.md:
- Line 5: Add the top-level Markdown heading “@evlog/telemetry” immediately
after the frontmatter in the changeset, before the existing release description,
to satisfy the MD041 requirement.
In `@packages/telemetry/src/outbox.ts`:
- Around line 189-198: Optimize the size-based compaction loop after the
“oldest-first drop” comment by pre-serializing each event once and tracking its
UTF-8 byte length in a running total. Remove repeated whole-array JSON
stringification and byte-length calculation from the while loop, decrementing
the total as events are shifted; reuse the serialized entries when writing the
final outbox contents.
---
Outside diff comments:
In `@examples/telemetry-playground/smoke.ts`:
- Around line 31-37: Update the outbox read catch around readFile in the smoke
check to handle only ENOENT as the expected missing-outbox case. Preserve the
zero-line behavior for a nonexistent outbox, but rethrow or otherwise fail for
permission, invalid-path, and other filesystem errors instead of swallowing
them.
In `@packages/telemetry/README.md`:
- Line 18: In the README description, correct the article before “analytics”
from “a” to “an” while preserving the rest of the usage explanation unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b3462a73-5876-4f6e-a225-6d78c84fc829
⛔ Files ignored due to path filters (1)
packages/telemetry/test/__snapshots__/telemetry.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (20)
.changeset/telemetry-package.mdapps/docs/content/4.use-cases/0.overview.mdapps/docs/content/4.use-cases/6.telemetry.mdexamples/telemetry-playground/smoke.tspackages/telemetry/README.mdpackages/telemetry/package.jsonpackages/telemetry/src/consent.tspackages/telemetry/src/create.tspackages/telemetry/src/disclosure.tspackages/telemetry/src/drain.tspackages/telemetry/src/idempotency.tspackages/telemetry/src/index.tspackages/telemetry/src/ingest.tspackages/telemetry/src/notice.tspackages/telemetry/src/outbox.tspackages/telemetry/src/paths.tspackages/telemetry/src/sanitize.tspackages/telemetry/test/ingest.test.tspackages/telemetry/test/telemetry.test.tspackages/telemetry/tsdown.config.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/telemetry/src/github-actions.ts (1)
28-40: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMerge
systemCustominstead of overwriting it.By spreading
...optsand then definingsystemCustom, anysystemCustomfields provided by the caller inoptswill be completely overwritten. Merge the existingopts?.systemCustomwith the new fields to prevent data loss.♻️ Proposed fix
return { ...base, run(command, fn, opts) { return base.run(command, fn, { ...opts, systemCustom: { + ...(opts?.systemCustom || {}), ghaAction: action, ghaEvent: eventType, }, }) }, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/telemetry/src/github-actions.ts` around lines 28 - 40, Update the run method in the returned wrapper to merge opts?.systemCustom with the ghaAction and ghaEvent fields instead of replacing systemCustom, preserving all caller-provided custom fields while ensuring the GitHub Actions values are applied.packages/telemetry/src/create.ts (1)
82-86: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winPrevent unhandled promise rejections from crashing the process.
instance.init()is called as a floating promise (void instance.init()) increateTelemetry. IfresolveMachineIdorflushBacklogreject, it will cause anUnhandledPromiseRejection, which crashes the host application in modern Node.js environments. Wrap the initialization logic in atry/catchblock to ensure background telemetry failures never disrupt the user's CLI execution.🔒️ Proposed fix
async init(): Promise<void> { if (!this._enabled) return - this.machineId = await resolveMachineId(this.options.name) - await this.flushBacklog() + try { + this.machineId = await resolveMachineId(this.options.name) + await this.flushBacklog() + } catch { + // Silent failure: never crash the host application for telemetry initialization + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/telemetry/src/create.ts` around lines 82 - 86, Update the Telemetry instance’s init method to wrap resolveMachineId and flushBacklog in a try/catch, swallowing or safely handling initialization errors so the floating init promise never produces an unhandled rejection. Preserve the existing disabled early return and initialization order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/telemetry/src/types.ts`:
- Around line 90-101: Update the CustomFields type to map explicitly over keyof
TFields for allowlisted keys, then intersect that mapped type with a broad
string index for non-allowlisted keys. Preserve CustomFieldValue<TFields[K]> for
declared fields while retaining boolean, number, or undefined fallback values
elsewhere.
---
Outside diff comments:
In `@packages/telemetry/src/create.ts`:
- Around line 82-86: Update the Telemetry instance’s init method to wrap
resolveMachineId and flushBacklog in a try/catch, swallowing or safely handling
initialization errors so the floating init promise never produces an unhandled
rejection. Preserve the existing disabled early return and initialization order.
In `@packages/telemetry/src/github-actions.ts`:
- Around line 28-40: Update the run method in the returned wrapper to merge
opts?.systemCustom with the ghaAction and ghaEvent fields instead of replacing
systemCustom, preserving all caller-provided custom fields while ensuring the
GitHub Actions values are applied.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 79be3104-a12c-442a-b4a6-852786560a05
📒 Files selected for processing (8)
.changeset/telemetry-package.mdexamples/telemetry-playground/smoke.tspackages/telemetry/README.mdpackages/telemetry/src/create.tspackages/telemetry/src/github-actions.tspackages/telemetry/src/outbox.tspackages/telemetry/src/types.tspackages/telemetry/test/telemetry-typing.test.ts
🔗 Linked issue
📚 Description
📝 Checklist
Summary by CodeRabbit
New Features
@evlog/telemetrypackage for privacy-conscious CLI/automation telemetry, including event wrapping, local buffering, generated disclosures, and a non-blocking flush with debug mode.status,enable,disable) and server ingestion validation with aningestexport.Documentation
@evlog/telemetry.Tests
Chores