feat(client): generate local machine id and send x-machine-id on all connections - #1729
Closed
mikhailm-coder wants to merge 1 commit into
Closed
feat(client): generate local machine id and send x-machine-id on all connections#1729mikhailm-coder wants to merge 1 commit into
mikhailm-coder wants to merge 1 commit into
Conversation
…connections Port of the machine-id-header client work from openframe-oss-tenant (hotfix/machine-id-header). MachineIdService persists a locally generated UUID in the shared app-support dir (read by mesh/fleet tool agents) and stamps it as x-machine-id on the HTTP clients, the NATS connection, and the NATS log stream (replacing the openframe-client placeholder). The server-assigned machine_id still names the NATS connection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe client now persists and caches a UUID machine ID. It sends this ID through HTTP and NATS headers. NATS connections retain the configured ID as their connection name. ChangesMachine ID propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MachineIdService
participant HTTPClients
participant NatsConnectionManager
participant LogStreamingRunManager
participant NatsLogConnection
Client->>MachineIdService: get_or_create machine ID
Client->>HTTPClients: set MACHINE_ID_HEADER
Client->>NatsConnectionManager: pass MachineIdService
NatsConnectionManager->>MachineIdService: get machine ID
NatsConnectionManager->>NatsConnectionManager: set NATS machine ID header
LogStreamingRunManager->>MachineIdService: get_or_create machine ID
LogStreamingRunManager->>NatsLogConnection: pass machine ID
NatsLogConnection->>NatsLogConnection: set MACHINE_ID_HEADER
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@clients/openframe-client/src/services/machine_id_service.rs`:
- Around line 28-43: Update MachineIdService::get_or_create to propagate read
errors other than NotFound instead of generating a replacement ID. For a missing
file, serialize initialization across processes using an interprocess lock or
atomic create-and-re-read, write the generated ID through a temporary file, and
atomically rename it into place; then cache and return the persisted winner so
concurrent service instances share one machine ID. Implement the same behavior
in the upstream tenant repository first, then port it 1:1 here.
🪄 Autofix
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 Plus
Run ID: 4217d3de-55a4-448e-a69d-484acfe4888c
📒 Files selected for processing (5)
clients/openframe-client/src/lib.rsclients/openframe-client/src/logging/nats_streaming.rsclients/openframe-client/src/services/machine_id_service.rsclients/openframe-client/src/services/mod.rsclients/openframe-client/src/services/nats_connection_manager.rs
Comment on lines
+28
to
+43
| pub fn get_or_create(&self) -> Result<String> { | ||
| if let Some(id) = self.cached_id.read().unwrap().clone() { | ||
| return Ok(id); | ||
| } | ||
|
|
||
| if let Ok(id) = self.read() { | ||
| debug!("Using existing machine ID: {}", id); | ||
| *self.cached_id.write().unwrap() = Some(id.clone()); | ||
| return Ok(id); | ||
| } | ||
|
|
||
| let id = Uuid::new_v4().to_string(); | ||
| self.write(&id)?; | ||
| info!("Generated new machine ID: {}", id); | ||
| *self.cached_id.write().unwrap() = Some(id.clone()); | ||
| Ok(id) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Create the persisted machine ID atomically.
Line 33 ignores every read error. An empty, unreadable, or transiently unavailable file then causes Lines 39-42 to generate a new ID.
Concurrent callers can also both observe a missing file and write different IDs. The RwLock does not protect the full read-create-write sequence across service instances or processes. HTTP, NATS control, and NATS logs can then use different x-machine-id values.
Generate only when the file is NotFound. Serialize initialization with an interprocess lock or atomic create-and-re-read. Write through a temporary file and rename it atomically.
Based on learnings, implement this behavior in the upstream tenant repository first, then retain a 1:1 port here.
🤖 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 `@clients/openframe-client/src/services/machine_id_service.rs` around lines 28
- 43, Update MachineIdService::get_or_create to propagate read errors other than
NotFound instead of generating a replacement ID. For a missing file, serialize
initialization across processes using an interprocess lock or atomic
create-and-re-read, write the generated ID through a temporary file, and
atomically rename it into place; then cache and return the persisted winner so
concurrent service instances share one machine ID. Implement the same behavior
in the upstream tenant repository first, then port it 1:1 here.
Source: Learnings
Contributor
Author
|
Folded into #1688, which now covers both the bundled mesh core (with the Host fix) and the openframe-client machine-id work as a single oss-lib PR. Deleting this branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the machine-id-header client work from
openframe-oss-tenant(hotfix/machine-id-header) into oss-lib's copy of theopenframe-clientcrate — this is the repo releases are cut from, and the porting round #1725 didn't carry it (the work lives on a branch in oss-tenant, not merged to its main, so the merged-commit ports never saw it).What it does
MachineIdService: generates a localUuid::v4(), persists it to the shared app-supportmachine_idfile (read by the mesh/fleet tool agents), cached behind anArc<RwLock>.x-machine-idheader on the HTTP clients (main + download), the NATS control connection, and the NATS log stream (replacing the"openframe-client"placeholder).machine_idstill names the NATS connection; only the header value is the new local id.Scope / notes
cargo fmtwrapping differs).cargo check --features binand pre-commitcargo fmt --checkpass.X-Machine-Idon the reinstall flow (registration_client.rs) is a different header/flow and is untouched.🤖 Generated with Claude Code
Summary by CodeRabbit