<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Hello,</p>
    <p>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.</p>
    <p>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.</p>
    <p>Cheers,<br>
      Daniel<br>
    </p>
    <div class="moz-cite-prefix">On 03.04.20 13:48, Federico Cabiddu
      wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAFOaF_gdNZ7iq9m9r6gTAqtUyeCSeAsmLUz52XRWVxVa+s8mtA@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr">Hi all,
        <div>following the feedback I just pushed a branch (<a
href="https://github.com/kamailio/kamailio/tree/grumvalski/siptrace_flag_fixes"
            moz-do-not-send="true">https://github.com/kamailio/kamailio/tree/grumvalski/siptrace_flag_fixes</a>),
          which tries to address the issues discussed.</div>
        <div>I've tried to split the commits so that each issue is
          handled separately.</div>
        <div>With the first commit (<a
href="https://github.com/kamailio/kamailio/commit/b64b3f03a9c6b69587ca360465f091f873f7274b"
            moz-do-not-send="true">https://github.com/kamailio/kamailio/commit/b64b3f03a9c6b69587ca360465f091f873f7274b</a>)
          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.</div>
        <div>The second commit (<a
href="https://github.com/kamailio/kamailio/commit/e28f464457eea47cc606c73cbfe4b30fcc8b542a"
            moz-do-not-send="true">https://github.com/kamailio/kamailio/commit/e28f464457eea47cc606c73cbfe4b30fcc8b542a</a>)
          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.</div>
        <div>With the third (<a
href="https://github.com/kamailio/kamailio/commit/080c6e07708f1964498a43e70c9b6240b5bdebcd"
            moz-do-not-send="true">https://github.com/kamailio/kamailio/commit/080c6e07708f1964498a43e70c9b6240b5bdebcd</a>)
          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<span
            style="color:rgb(24,178,24);font-family:monospace"> </span>callbacks.</div>
        <div>I've tried different scenarios in both capturing modes
          (transaction and flag+trace):</div>
        <div>1) Successful call (INVITE-200-ACK)</div>
        <div>2) Error replied</div>
        <div>3) Canceled call</div>
        <div>4) locally generated CANCEL (timeout)</div>
        <div>All looks good (except for the ACK issue) in both modes.</div>
        <div>I would like to have the developers' feedback before
          opening a PR, there could be other scenarios/use cases I'm not
          considering here.</div>
        <div>Thank you all.</div>
        <div><br>
        </div>
        <div>Cheers,</div>
        <div><br>
        </div>
        <div>Federico</div>
        <div><br>
        </div>
        <div><br>
        </div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr" class="gmail_attr">On Wed, Apr 1, 2020 at 2:45 PM
          Federico Cabiddu <<a
            href="mailto:federico.cabiddu@gmail.com"
            moz-do-not-send="true">federico.cabiddu@gmail.com</a>>
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px
          0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
          <div dir="ltr">
            <div dir="ltr">Hi,</div>
            <div class="gmail_quote">
              <blockquote class="gmail_quote" style="margin:0px 0px 0px
                0.8ex;border-left:1px solid
                rgb(204,204,204);padding-left:1ex">
                <div>
                  <p>OK, indeed, the previous behavior should be
                    preserved in this case. Is sip_trace() without
                    params now doing transaction mode capturing?</p>
                </div>
              </blockquote>
              <div>Yes and no. Transaction mode is activated but actual
                behavior is not exactly the same (see case 3) vs case
                1)). </div>
              <div><br>
              </div>
              <div>Cheers,</div>
              <div><br>
              </div>
              <div>Federico</div>
            </div>
          </div>
        </blockquote>
      </div>
    </blockquote>
    <pre class="moz-signature" cols="72">-- 
Daniel-Constantin Mierla -- <a class="moz-txt-link-abbreviated" href="http://www.asipto.com">www.asipto.com</a>
<a class="moz-txt-link-abbreviated" href="http://www.twitter.com/miconda">www.twitter.com/miconda</a> -- <a class="moz-txt-link-abbreviated" href="http://www.linkedin.com/in/miconda">www.linkedin.com/in/miconda</a></pre>
  </body>
</html>