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