Hi Daniel, thanks for having a look. There is already a callback for negative ACK, my problem was that I was using t_check_trans (tm api) to check if the ACK was a negative ACK, and t_check_trans was calling this callback internally. So, if after the sip_trace() call in the script we had a t_check_trans(), the callback itself was called again, thus giving a duplicated message. I've solved using instead t_lookup_request. I've pushed another commit on the branch and I'm going to open the PR.
Cheers, Federico On Sun, Apr 5, 2020 at 9:18 AM Daniel-Constantin Mierla <mico...@gmail.com> wrote: > Hello, > > I think it is fine to merge those three commits to master as they solve > some of the reported issue. There is also some work planned to be done for > a global enable/disable tracing to database, so it makes sense to have all > the code in master for combined testing. > > Regarding the negative ACK, maybe getting the invite transaction and > seeing if it is a failed transaction is an option. Or simply adding in tm > module a callback for negative ACK. > > Cheers, > Daniel > On 03.04.20 13:48, Federico Cabiddu wrote: > > Hi all, > following the feedback I just pushed a branch ( > https://github.com/kamailio/kamailio/tree/grumvalski/siptrace_flag_fixes), > which tries to address the issues discussed. > I've tried to split the commits so that each issue is handled separately. > With the first commit ( > https://github.com/kamailio/kamailio/commit/b64b3f03a9c6b69587ca360465f091f873f7274b) > I fixed the incoming ACK for negative replies tracing: as discussed it > makes no sense to check in the callback if tracing is enabled or not. > The second commit ( > https://github.com/kamailio/kamailio/commit/e28f464457eea47cc606c73cbfe4b30fcc8b542a) > refactors the e2e CANCEL handling. With the previous implementation the > incoming CANCEL captured would have the ANYADDR set as destination address. > This commit also allows to have exactly the same behavior between > transaction tracing (sip_trace_mode("t")) and legacy tracing (setflag + > sip_trace()) when tracing a specific INVITE. > With the third ( > https://github.com/kamailio/kamailio/commit/080c6e07708f1964498a43e70c9b6240b5bdebcd) > I've tried as much as possible to restore the legacy behavior when tracing > all the requests without having duplicated captures for CANCEL and ACK for > negative replies. I could achieve this for the CANCEL checking if the > INVITE it refers to is already being traced (meaning that the CANCEL will > be captured by the callback) but I couldn't for the ACK. I couldn't find a > way to check if the ACK is for a negative reply (and thus it belongs to a > transaction), without having the tm callbacks for ACK run, since both > t_check and t_check_trans tm calls run the E2ECANCEL_IN callbacks. > I've tried different scenarios in both capturing modes (transaction and > flag+trace): > 1) Successful call (INVITE-200-ACK) > 2) Error replied > 3) Canceled call > 4) locally generated CANCEL (timeout) > All looks good (except for the ACK issue) in both modes. > I would like to have the developers' feedback before opening a PR, there > could be other scenarios/use cases I'm not considering here. > Thank you all. > > Cheers, > > Federico > > > > On Wed, Apr 1, 2020 at 2:45 PM Federico Cabiddu < > federico.cabi...@gmail.com> wrote: > >> Hi, >> >>> OK, indeed, the previous behavior should be preserved in this case. Is >>> sip_trace() without params now doing transaction mode capturing? >>> >> Yes and no. Transaction mode is activated but actual behavior is not >> exactly the same (see case 3) vs case 1)). >> >> Cheers, >> >> Federico >> > -- > Daniel-Constantin Mierla -- www.asipto.comwww.twitter.com/miconda -- > www.linkedin.com/in/miconda > >
_______________________________________________ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev