Skip to content

fix(sdks): drop rivet-util metrics dep from protocol crates to unlink rocksdb - #5558

Merged
abcxff merged 1 commit into
mainfrom
stack/fix-sdks-drop-rivet-util-metrics-dep-from-protocol-crates-to-unlink-rocksdb-woolkmvt
Aug 11, 2026
Merged

fix(sdks): drop rivet-util metrics dep from protocol crates to unlink rocksdb#5558
abcxff merged 1 commit into
mainfrom
stack/fix-sdks-drop-rivet-util-metrics-dep-from-protocol-crates-to-unlink-rocksdb-woolkmvt

Conversation

@abcxff

@abcxff abcxff commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@railway-app

railway-app Bot commented Aug 10, 2026

Copy link
Copy Markdown

🚅 Deployed to the rivet-pr-5558 environment in rivet-frontend

Service Status Web Updated (UTC)
kitchen-sink 😴 Sleeping (View Logs) Web Aug 10, 2026 at 10:22 pm
website 😴 Sleeping (View Logs) Web Aug 10, 2026 at 10:19 pm
frontend-cloud 😴 Sleeping (View Logs) Web Aug 10, 2026 at 10:17 pm
frontend-inspector 😴 Sleeping (View Logs) Web Aug 10, 2026 at 10:16 pm
ladle ✅ Success (View Logs) Web Aug 10, 2026 at 10:12 pm
mcp-hub ✅ Success (View Logs) Web Aug 10, 2026 at 10:10 pm

@claude

claude Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review

Mechanical, well-scoped change: it drops the rivet-util dependency from five lightweight SDK protocol crates (data, depot-protocol, envoy-protocol, epoxy-protocol, ups-protocol) and replaces the rivet_util::serde::bare_to_vec! / bare_from_slice! macros with plain serde_bare::to_vec / from_slice calls, to avoid pulling in rivet-util's heavier transitive dependency chain in these SDK crates.

Verified:

  • No remaining rivet_util references in any of the five crates' src/ (confirmed via grep), so the Cargo.toml/Cargo.lock dependency removal is complete and consistent.
  • These calls sit inside deserialize_version/serialize_version arms of OwnedVersionedData impls, where the version is already recorded by the outer vbare wrapper — using raw serde_bare per fixed-version arm here matches the documented vbare pattern in engine/CLAUDE.md (Ok(ToClientMk2::V7(serde_bare::from_slice(payload)?))), not the "raw serde_bare for wire/persisted data" anti-pattern the root CLAUDE.md warns about.
  • bare_to_vec!/bare_from_slice! only wrap the call with observe! (duration + size histogram recording); no error-handling or semantics differ, so this is a behavior-preserving refactor. Existing round-trip tests in envoy-protocol/src/versioned/mod.rs were updated in place and still cover correctness.

Minor nits:

  1. Formatting: engine/sdks/rust/epoxy-protocol/src/versioned.rs and engine/sdks/rust/ups-protocol/src/versioned.rs have leftover awkward multi-line wraps from the macro removal, e.g.:
    2 => Ok(CommittedValue::V2(serde_bare::from_slice(
        payload
    )?)),
    These should collapse to a single line (as was correctly done in envoy-protocol/src/versioned/mod.rs and depot-protocol/src/versioned.rs). Worth running node scripts/format/agent-format.mjs over the changed Rust files before merge.
  2. Observability tradeoff: this removes the SERIALIZE_SIZE/DESERIALIZE_SIZE Prometheus histograms (and generic OBSERVATION_DURATION) for these particular wire-format hot paths (envoy/epoxy/ups messages). If those metrics were being used for capacity planning or debugging payload sizes on these protocols, that visibility is now gone. Likely an acceptable and intentional tradeoff given the goal (unlinking a heavy dependency from published SDK crates), but worth a explicit callout in the PR description since it's a behavior change beyond the mechanical rename.

Test coverage: no new tests, but none are needed — this is a pure dependency/call-site swap with identical serialization behavior, and existing tests already exercise the changed code paths.

Overall this looks safe to merge modulo the formatting cleanup.

@abcxff
abcxff force-pushed the stack/fix-sdks-drop-rivet-util-metrics-dep-from-protocol-crates-to-unlink-rocksdb-woolkmvt branch from 69621d8 to c245c3a Compare August 11, 2026 14:24
@abcxff
abcxff force-pushed the stack/fix-sdks-drop-rivet-util-metrics-dep-from-protocol-crates-to-unlink-rocksdb-woolkmvt branch from c245c3a to 686f265 Compare August 11, 2026 17:22
@abcxff
abcxff merged commit 686f265 into main Aug 11, 2026
10 of 11 checks passed
@abcxff
abcxff deleted the stack/fix-sdks-drop-rivet-util-metrics-dep-from-protocol-crates-to-unlink-rocksdb-woolkmvt branch August 11, 2026 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant