Skip to content

Conversation

@pepol
Copy link
Member

@pepol pepol commented Dec 16, 2025

Summary by CodeRabbit

  • Chores
    • Improved CLI telemetry to derive a deterministic, hashed machine identifier (replacing raw hostnames) for more consistent reporting and stronger user anonymity. This change does not alter which events are captured, only how the machine identifier is represented.

✏️ Tip: You can customize this high-level summary in your review settings.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Documentation has been updated on https://github.com/wundergraph/cosmo-docs.
  • I have read the Contributors Guide.

@coderabbitai
Copy link

coderabbitai bot commented Dec 16, 2025

Walkthrough

Telemetry now computes a deterministic machine_id by hashing the system hostname with SHA-256 (using node:crypto) and uses that hash in CLI metadata; event capture behavior is unchanged.

Changes

Cohort / File(s) Summary
Telemetry metadata computation
cli/src/core/telemetry.ts
Adds node:crypto import and computes a SHA-256 hash of the hostname as machineId; replaces direct hostname usage with this machineId in getMetadata. No other behavioural changes to event capture.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify the node:crypto import and SHA-256 implementation are correct and compatible with supported Node versions.
  • Confirm machineId is computed deterministically and consistently applied in getMetadata.
  • Inspect telemetry payloads to ensure only the machine_id value changed and no other fields were altered.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: masking hostname in CLI telemetry by computing a SHA-256 hash instead of using raw hostname.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 e246f3b and 20eeabb.

📒 Files selected for processing (1)
  • cli/src/core/telemetry.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • cli/src/core/telemetry.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: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)

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

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

@pepol pepol force-pushed the peter/eng-8578-ensure-best-practices-with-log-masking-redaction branch from 4509569 to 499ad6d Compare December 16, 2025 14:26
@codecov
Copy link

codecov bot commented Dec 16, 2025

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 28.67%. Comparing base (84c1ba4) to head (20eeabb).
⚠️ Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
cli/src/core/telemetry.ts 33.33% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2416       +/-   ##
===========================================
- Coverage   42.59%   28.67%   -13.93%     
===========================================
  Files        1013      127      -886     
  Lines      141063    11059   -130004     
  Branches     8660      241     -8419     
===========================================
- Hits        60086     3171    -56915     
+ Misses      79365     7886    -71479     
+ Partials     1612        2     -1610     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14792d1 and 499ad6d.

📒 Files selected for processing (1)
  • cli/src/core/telemetry.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cli/src/core/telemetry.ts (1)
cli/src/core/config.ts (1)
  • config (23-38)
⏰ 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: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
🔇 Additional comments (1)
cli/src/core/telemetry.ts (1)

161-161: Good privacy improvement.

Using the hashed machine_id instead of the raw hostname is the right approach for protecting user privacy in telemetry data. The implementation correctly applies the hashed value.

@pepol pepol enabled auto-merge (squash) December 17, 2025 09:57
@pepol pepol force-pushed the peter/eng-8578-ensure-best-practices-with-log-masking-redaction branch from a44082f to e246f3b Compare December 18, 2025 16:01
@pepol pepol force-pushed the peter/eng-8578-ensure-best-practices-with-log-masking-redaction branch from e246f3b to 20eeabb Compare December 22, 2025 18:55
@pepol pepol requested a review from StarpTech December 22, 2025 19:02
Copy link
Contributor

@StarpTech StarpTech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants