IPv6-safe connection metrics + opt-in per-connection traffic + IPv4/IPv6 totals#313
Open
ABHIRAMSHIBU wants to merge 4 commits into
Open
IPv6-safe connection metrics + opt-in per-connection traffic + IPv4/IPv6 totals#313ABHIRAMSHIBU wants to merge 4 commits into
ABHIRAMSHIBU wants to merge 4 commits into
Conversation
Module-level helpers added in earlier commits broke the convention followed by every other *_ds.py file (class on the first non-import line). Move them as @staticmethod onto IPConnectionDatasource — the sibling Stats/Traffic datasources call them via the same cross-class pattern already used for BaseDSProcessor.trimmed_records. Also drops the unused _count_connection_records wrapper. No behavior change. 8/8 focused tests pass; 215/215 full suite passes.
CollectorHandler.collect() returned with no yield when a scrape landed
inside minimal_collect_interval, so every mktxp_* series went missing for
that request while up{} stayed 1. Concurrent scrapers, or a single
Prometheus scrape overlapping a slow collection (e.g. connection_traffic
= True), surfaced as phantom 'down' gaps in dashboards.
Materialise each successful collection, cache it under a Lock, and yield
from the cache on within-MCI calls. Empty collections don't clobber the
cache. ProbeCollectorHandler unchanged — /probe semantics intentionally
raise on deferral.
Verified on a live RouterOS target: 12 back-to-back 1s-cadence scrapes
all returned ~1.1k mktxp_* lines (mix of fresh ~0.5–4.2s collects and
~8ms cache replays). Previously 4 of 5 scrapes returned only the 1.8KB
process-metric stub.
2/2 new tests pass; 211/211 full suite passes.
Owner
|
@ABHIRAMSHIBU thanks for the PR, it looks good overall. a few comments:
|
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.
Changes
IPv6-safe connection parsing — both
/ip/firewall/connection/and/ipv6/firewall/connection/are now polled and merged. The previoussrc-address.split(':')[0]truncated IPv6 addresses at the first colon, producing nonsense labels. Replaced with proper[host]:port/host:portnormalization that handles both families.New per-family totals (always on, backward compatible):
mktxp_ip_connections_total— combined (unchanged behavior)mktxp_ipv4_connections_total— IPv4-onlymktxp_ipv6_connections_total— IPv6-onlyInvariant:
ip_total == ipv4_total + ipv6_total.New opt-in per-connection traffic metrics (
connection_traffic = Falseby default):mktxp_connection_upload_bytes(fromorig-bytes)mktxp_connection_download_bytes(fromrepl-bytes)mktxp_connection_total_bytes