Attention is currently required from: iedemam, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27081 ) Change subject: WIP: New stats for lchan life duration. ...................................................................... Patch Set 4: (5 comments) File src/osmo-bsc/bts_trx_vty.c: https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/7ecf102a_5a8ad6b4 PS4, Line 575: vty_out(vty, " Activated %llu ms ago%s", duration_ms, VTY_NEWLINE); Probably more interesting to print it in VTY as seconds? Doesn't look like we can glance millisecond updates in the VTY anyway. File src/osmo-bsc/bts_vty.c: https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/0fd3e91a_42ca9366 PS4, Line 3830: vty_out(vty, " Channel Activations : %"PRIu64" TCH", activations_tch); I'd rather put it: Channel Activations: TCH (avg lifespan 3s) SDCCH (avg lifespan 5s) See the line formatting, and also use seconds instead of milliseconds. "Average" can probably be shortened to "avg". File src/osmo-bsc/lchan_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/2f4a8537_0e73101b PS4, Line 1812: long long duration_ms = gsm_lchan_active_duration_ms(lchan); Shouldn't this be an unsigned long long everywhere? Maybe simply use time_t? https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/0722481b_de0b326b PS4, Line 1814: if (lchan->type == GSM_LCHAN_TCH_F || lchan->type == GSM_LCHAN_TCH_H) { I'd go for a switch statement here, but not critical. https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/f9e99713_697ad89f PS4, Line 1816: rate_ctr_add(rate_ctr_group_get_ctr(bts_ctrs, BTS_CTR_LCHAN_TCH_TOTAL_ACTIVE_MILLISECONDS), duration_ms); Does it really makes sense to use this kind of value as a rate_ctr? Just asking. It's difficult to interpet it I think. -- 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: 4 Gerrit-Owner: iedemam <michael(a)kapsulate.com> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: iedemam <michael(a)kapsulate.com> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 08 Feb 2022 11:23:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment