Re: [sr-dev] [kamailio/kamailio] Siptrace (#1912)

henningw commented on this pull request. Thank you also from my side for the contribution. I obviously only did a quick look to the code, given this complexity. I found a few places where it would be good to have a look. <para> The SIPtrace module offer a possibility to store incoming and outgoing SIP messages in a database and/or duplicate to the capturing server (using <acronym>HEP</acronym>, - the Homer encapsulation protocol, or plain SIP mode) + the Homer encapsulation protocol, or plain SIP mode). Since version 5.4 new levels of tracing + are available. Transactions and dialogs can be traced. Trace flag is now useless. "flag is useless" - probably should be changed after the re-introducing as well. Stateless forwarded messages (forward()) are not logged if you set the flag, use sip_trace() inside <emphasis>onsend_route</emphasis> block. </para> + <para> + If dialog level tracing is used internally generated BYE transaction(in + case of internal timeouts) won't be traced. At the moment kamailio offers + no posibility to trace those messages. Daniel commented in the main comment thread about the sip parsing, that it is actually not that expensive anymore /* register callbacks to TM */ if(load_tm_api(&tmb) != 0) { LM_WARN("can't load tm api. Will not install tm callbacks.\n"); - } else if(tmb.register_tmcb(0, 0, TMCB_REQUEST_IN, trace_onreq_in, 0, 0) - <= 0) { - LM_ERR("can't register trace_onreq_in\n"); - return -1; + } + + if (load_dlg_api(&dlgb) < 0) { + LM_WARN("can't load dlg api. Will not install dialog callbacks.\n"); Below you log on ERROR level if the dialog tracing is not available, makes probably sense to log here with ERROR as well. - * Send sip trace with destination and correlation id + * TODO TODO TODO: Is this still a TODO or was already added? sto.stat = siptrace_rpl; #endif - sip_trace_store(&sto, NULL, NULL); - return; + if (info->uriState == STRACE_RAW_URI) { + LM_BUG("uriState must be either UNUSED or PARSED here! must be a bug! Message won't be traced!\n"); + abort(); Calling abort() will kill Kamailio, it is not possible to recover here? - pkg_free(*param); - /* free temporary proxy*/ - if(p) { - free_proxy(p); /* frees only p content, not p itself */ - pkg_free(p); + trace_type = parse_siptrace_flag(&sflags); + if (trace_type == SIPTRACE_NONE) { + LM_ERR("Failed to parse trace type!\n"); + return -1; + } + + *param = pkg_malloc(sizeof(trace_type)); pkg_malloc can fail, check for success. - if(((char *)(*param))[0] == '\0') { - // Empty URI, use the URI set at module level (dup_uri) - if(dup_uri) { - uri = *dup_uri; - } else { - LM_ERR("Missing duplicate URI\n"); - return -1; - } - } else { - duri = (char *)*param; + for (idx = 0; idx < sflags->len; idx++) { This looks that there a a lot of whitespaces, can you check? Maybe github shows it different. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1912#pullrequestreview-223338198