feat(timestamp-stack): add routing timestamp instrumentation#30
Open
YuanYuYuan wants to merge 6 commits into
Open
feat(timestamp-stack): add routing timestamp instrumentation#30YuanYuYuan wants to merge 6 commits into
YuanYuYuan wants to merge 6 commits into
Conversation
- Cap TsStack wire decode at 64 entries to prevent OOM from crafted packets - Rename wire type TimestampStack -> WireTimestampStack to eliminate name collision with the public API type zenoh::timestamp_stack::TimestampStack, including all remaining import sites in the zenoh crate - Remove unwrap/expect in get_ts_stack_timestamp; return empty vec on serialization failure - Stamp ROUTE timestamp per-subscriber in fan-out so each gets its own dispatch time instead of sharing a pre-loop timestamp - Replace avoidable .clone() with mem::take() at receive-side extract sites - Replace expect() in push_ts_interception with debug_assert + graceful return - Add #[non_exhaustive] to InterceptionPoint - Add pub(crate) visibility comment on Runtime::state - Document timeout reply ext_ts_stack: None intentionally unset
…ultiple ROUTE hops
a764445 to
fc1e09c
Compare
… reply_err, multi-subscriber, publisher API, and callback context
…s Peer not Client
- Add `TimestampInstrumentation::new(send, route, receive)` direct constructor alongside the builder, for C/Python bindings that don't need the builder chain - Add per-publisher default instrumentation: `PublisherBuilder::timestamp_instrumentation()` stores a default on the `Publisher` struct; per-put override takes precedence - Add `TimestampStackRecord::as_timestamp()` to parse standard HLC bytes into `zenoh::time::Timestamp`, avoiding raw byte handling for non-custom records - Rename `GetTimestampCallback` → `SessionTimestampCallback` to clarify that the callback is session-scoped (registered at open time, not per-message) All 20 timestamp_stack integration tests pass (job 63 on beta-cuda).
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
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.
Summary
Adds opt-in timestamp instrumentation for measuring end-to-end message latency in Zenoh. Messages can carry a
TsStackwire extension that accumulatesInterceptionrecords at up to three points along a message's path: Send, Route, and Receive.The feature is entirely
#[cfg(feature = "unstable")]-gated and has zero overhead on uninstrumented messages.Key Changes
Wire protocol (
zenoh-protocol,zenoh-codec)TsStackTypeextension (ID0x7) added toPush,Request, andResponsemessagesWireTimestampStackcarriesconf_flags(which points are active) + an orderedVec<Interception>Public API (
zenohcrate,feature = "unstable")TimestampInstrumentationBuilder/TimestampInstrumentation— configure which points to recordInterceptionPoint(Send,Route,Receive) —#[non_exhaustive]for forward compatibilityTimestampStack/TimestampStackRecord— read timestamps off a receivedSample,ReplyError, orQueryTsStackContext/GetTimestampCallback— custom timestamp generation viaOpenBuilder::with_timestamp_callback.timestamp_instrumentation(Option<TimestampInstrumentation>)builder method on put / get / reply / publisherInstrumentation points
session.rs(resolve_put,resolve_get) andbuilders/reply.rsrouting/dispatcher/pubsub.rs(per-subscriber, inside fan-out loop) andqueries.rsWeakSession::send_push_consumeandadminspace.rsRobustness fixes
WireTimestampStackrenamed fromTimestampStackto eliminate name collision with the public API typeget_ts_stack_timestampremovesunwrap()/expect()— returns empty bytes on clock failure instead of panickingpush_ts_interceptionusesdebug_assert+ silent skip instead ofexpectfor unknown point IDsext_ts_stack: Noneon timeout replies is now documented (was aTODOcomment)Plumbing
WeakDynamicRuntime/WeakRuntimeweak-reference plumbing for runtime access in query stateReplyErrpublic API accessorlazy_hlcinRuntimeState— lazily-initialized HLC fallback when no user HLC is configured, replacing the previousSystemTime::now()approachTests
zenoh/tests/timestamp_stack.rs: no-instrumentation baseline, single-point (Send / Receive), all-points ordering, custom callback byte verification, callback context correctness, per-message stack independence, query/reply flows,is_customflag, route-only instrumentation,ReplyErrorpropagation, multiple-subscriber fan-out, and publisher API pathBreaking Changes
None. All new types and methods are behind
feature = "unstable".Known Limitations
QueryCleanuptimeout responses carryext_ts_stack: None— not instrumented (documented rationale in code)🏷️ Label-Based Checklist
Based on the labels applied to this PR, please complete these additional requirements:
Labels:
new feature🆕 New Feature Requirements
Since this PR adds a new feature:
feature = "unstable", zero overhead when inactivepush_ts_interceptionis a no-op whenext_ts_stackisNone)Consider: Can this feature be split into smaller, incremental PRs?
Instructions:
- [ ]to- [x])This checklist updates automatically when labels change, but preserves your checked boxes.