feat(yang-push): separate transport session info from subscription identity - #29
Draft
rodonile wants to merge 7 commits into
Draft
feat(yang-push): separate transport session info from subscription identity#29rodonile wants to merge 7 commits into
rodonile wants to merge 7 commits into
Conversation
rodonile
requested review from
riccardo-negri and
ustorbeck
and removed request for
riccardo-negri and
ustorbeck
August 6, 2026 07:22
rodonile
marked this pull request as draft
August 6, 2026 08:17
rodonile
force-pushed
the
sub-info-refactor
branch
2 times, most recently
from
August 6, 2026 09:57
3c3a787 to
3e4b508
Compare
- Bump yang-rs dependency to yang-rs-5 (libyang 5.4.9) and update Cargo.lock/Cargo.toml accordingly - Refactor validation actor internals for the new yang-rs API - Update example YANG modules (ietf-interfaces, ietf-yang-push-revision, ietf-system-capabilities) and add ietf-distributed-notif, ietf-yp-observation - Regenerate yang-lib.xml / yang-lib-full.xml to match updated modules
- Add counters/gauges for received, decoded, dropped, cache lookups, buffered/buffer drained, yang context load status, validated, skipped, sent, pending, cached peers/subscriptions - Record metrics at the relevant points in the validation pipeline
Ensure a CachedSubscription entry exists and schema_fetch_pending is set before sending a cache lookup request, for both LookupBySubscriptionInfo and LookupBySubscriptionId. Previously the flag was only set after buffer_packet ran, which can return early (and skip entry creation) when the per-subscription/per-peer buffer is full. That left process_cache_response unable to find the subscription later, and let duplicate packets keep re-triggering lookups to the cache actor. Also: - Log the send error on the LookupBySubscriptionId path the same way as LookupBySubscriptionInfo, instead of silently discarding it. - Add CachedSubscription::new() to remove the duplicated struct literal across the four construction sites. - Note a TODO: CachedPeerSubscriptions entries are never evicted by TTL/idleness, only replaced when SubscriptionStarted info changes.
- Collapse the wrapped <namespace> values back onto one line in yang-lib.xml and yang-lib-full.xml (ietf-notification-capabilities, ietf-subscribed-notifications, ietf-subscribed-notif-receivers, and ietf-yang-push-telemetry-message) to be on the safe side with yang libraries implementations - Update the schema_fetch_pending doc comment in validation/mod.rs
Replace the DROP_REASON_*, SKIP_REASON_*, and CACHE_LOOKUP_BY_* string constants with DropReason, SkipReason, and CacheLookupBy enums deriving Display, VariantNames, and IntoStaticStr. Counter descriptions are now generated from VariantNames instead of a hand-maintained list, and KeyValue tags use IntoStaticStr's zero-allocation &'static str conversion, avoiding per-packet String allocations on this hot path. Also merges the duplicate "reason" key into one REASON_KEY constant.
Split the (collector, interface, peer) tuple into two concepts: SessionInfo (transport context: collector, interface, full peer SocketAddr) and SubscriptionInfo (subscription identity, now keyed only on peer_ip). This fixes a latent cache-key bug where a device reconnecting with a new ephemeral source port was treated as a different subscription. Thread SessionInfo alongside SubscriptionInfo through the cache actor, YangLibraryFetcher trait, and validation/enrichment actors. Replace the validation->enrichment tuples with named structs ValidatedNotification/EnrichedNotification. Add diagnostic logging around YANG module resolution in fetch_from_device_by_id.
rodonile
force-pushed
the
sub-info-refactor
branch
from
August 6, 2026 12:05
3e4b508 to
7174e65
Compare
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
2 participants
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.
This PR depends on #23
Summary
SubscriptionInfoused to carry both subscription identity/content fields(
target,models,content_id, etc.) and transport fields(
collector,interface,peer). This PR extracts the transport fieldsinto a new
SessionInfostruct, leavingSubscriptionInfowith onlypeer_ipfor identitification purposes.Why
SubscriptionInfowas keyed on the full peerSocketAddr(incl. ephemeralsource port), so a device reconnecting with a new port was treated as a
different subscription. Keying on
peer_ipfixes this;SessionInfokeepstransport details available where still needed (logging/enrichment).
Changes
SessionInfotoudp-notif-service;UdpNotifRequestwraps it(keeps backward-compatible getters).
SubscriptionInfodropscollector/interface/port, keepspeer_ip.SessionInfothroughCacheLookupCommand,YangLibraryFetcher,validation and enrichment actors.
ValidatedNotification/EnrichedNotificationstructs.SessionInfonow reaching the enrichment actor,TelemetryMessageMetadata'scollection-address/collection-port(the collector's own address/port) arepopulated for the first time — previously always
None.fetch_from_device_by_id.subscriptions-info.jsonfixtures and all tests/call-sites.