Attention is currently required from: neels.
iedemam has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/27081 )
Change subject: stats: new trackers for lchan life duration
......................................................................
Patch Set 19:
(12 comments)
Patchset:
PS18:
Hi Neels, […]
For now I'm using deciseconds
(not decaseconds as in my typo above). I'd like to have the underlying stat values be
milliseconds for clarity but if this isn't possible to have independent timer
frequency and increment values, it's not a deal-breaker.
Patchset:
PS19:
Hi all,
I think all feedback has now been addressed. I still want to do additional testing now
that we're really close so please do not merge, even if everything looks good.
Thanks,
-Michael
File include/osmocom/bsc/bts.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/e2f91611_256e6d9e
PS18, Line 61: BTS_CTR_CHAN_SDCCH_ACTIVE_MILLISECONDS_TOTAL,
(1)
Done
File include/osmocom/bsc/gsm_data.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/61943723_41ef4818
PS18, Line 890: /* Interval timing to capture duration per activation and cummulative
active time */
"cumulative"
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/15f55d51_92d8594f
PS18, Line 891: struct osmo_time_cc active_ms;
(1) […]
Done
File src/osmo-bsc/bts.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/134c40f4_93716e8e
PS18, Line 1038: "Cumulative number of milliseconds of SDCCH channel
activity" },
(1)
Done
File src/osmo-bsc/bts_trx_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/31ba6abc_315990ef
PS18, Line 576: VTY_NEWLINE);
(3) […]
This specific VTY output is the primary
motivation for this entire patch. I've re-added the osmo_time_cc_reset() so each lchan
activation can be tracked and output.
I started with option b but you argued against it so I refactored everything to only use a
single osmo_time_cc. I agree this is cleaner and now believe it matches my functional goal
and your implementation approach.
File src/osmo-bsc/bts_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/1959db04_7d21fd8b
PS18, Line 3844: vty_out(vty, "%s", VTY_NEWLINE);
(2) […]
Agreed. I've opted to show total
activations and then avg lifespan over past hour.
File src/osmo-bsc/lchan_fsm.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/d6bb8d10_2c515814
PS18, Line 208: struct rate_ctr *rate_ctr;
osmocom style requires that this local variable be
defined at the start of the scope, at line 203 ab […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/a09ec898_31d87a43
PS18, Line 224: if (lchan->active_ms.cfg.rate_ctr) {
(could just set the flag no matter if rate_ctr is set
or not. […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/56f4b207_a0e53313
PS18, Line 546: .active_ms = lchan->active_ms
please add a trailing comma. […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/e385d5ae_3588b8d7
PS18, Line 556: if (lchan->active_ms.cfg.rate_ctr) {
(could just set the flag no matter if rate_ctr is set
or not. […]
Done
--
To view, visit
https://gerrit.osmocom.org/c/osmo-bsc/+/27081
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I1b0670c47cb5e0b7776eda89d1e71545ba0e3347
Gerrit-Change-Number: 27081
Gerrit-PatchSet: 19
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 06 Apr 2022 12:59:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: iedemam <michael(a)kapsulate.com>
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment