[kamailio/kamailio] Sipcapture: When inserting a reply, this module would insert the SIP … (#876)
…Status as the reply method. I've patched sipcapture.c and sipcapture.h to insert the actual SIP Status into a 'status' field on replies, and the Method extracted from the CSEQ into the 'method' field. This means when using this version of the module, you're tables must include a 'status' varchar(5) NULL field. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/876
-- Commit Summary --
* Sipcapture: When inserting a reply, this module would insert the SIP Status as the reply method. I've patched sipcapture.c and sipcapture.h to insert the actual SIP Status into a 'status' field on replies, and the Method extracted from the CSEQ into the 'method' field. This means when using this version of the module, you're tables must include a 'status' varchar(5) NULL field.
-- File Changes --
M modules/sipcapture/sipcapture.c (83) M modules/sipcapture/sipcapture.h (1) M modules/sipcapture/sql/create_sipcapture_postgress.sql (3) M modules/sipcapture/sql/schema_data.sql (3)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/876.patch https://github.com/kamailio/kamailio/pull/876.diff
Please format the commit log messages according with the guidelines from:
* https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md
You should be able to amend previous commit log message in your branch and repush.
it can be approved only if it can be activated by param i.e.: modparam("sipcapture", "version_schema", 5);
anyway, I don't see why we should do this way and not just add a cseq_method field ? This will avoid a lot potential issues and give you same possibility for searches.
The way it works now, on a reply the SIP Status is inserted into the "method" field, this isn't very nice as, if you're working with the table, you'd need to actually look into the cseq and break it into Sequence and Method to be able to do anything useful. Then you'd need to check if the "method" field is an actual method or a status code. The patch makes it much more readable:
``` +---------------------+------------------+--------+--------+-------------------------------+--------------------------------------+ | date | micro_ts | method | status | reply_reason | ruri | +---------------------+------------------+--------+--------+-------------------------------+--------------------------------------+ | 2016-12-01 02:18:31 | 1480558711188146 | INVITE | | | sip:12345@1.2.3.4 | | 2016-12-01 02:18:31 | 1480558711188584 | INVITE | 100 | Trying | | | 2016-12-01 02:18:31 | 1480558711189884 | INVITE | 407 | Proxy Authentication Required | | | 2016-12-01 02:18:31 | 1480558711391719 | ACK | | | sip:12345@1.2.3.4 | | 2016-12-01 02:18:31 | 1480558711391871 | INVITE | | | sip:12345@1.2.3.4 | | 2016-12-01 02:18:31 | 1480558711392194 | INVITE | 100 | Trying | | | 2016-12-01 02:18:31 | 1480558711411499 | INVITE | 403 | Forbidden | | | 2016-12-01 02:18:31 | 1480558711796623 | ACK | | | sip:12345@1.2.3.4 |
``` Which is easier to read than the current way:
``` +---------------------+------------------+--------+-------------------------------+--------------------------------------+ | date | micro_ts | method | reply_reason | ruri | +---------------------+------------------+--------+-------------------------------+--------------------------------------+ | 2016-12-01 02:18:31 | 1480558711188146 | INVITE | | sip:12345@1.2.3.4 | | 2016-12-01 02:18:31 | 1480558711188584 | 100 | Trying | | | 2016-12-01 02:18:31 | 1480558711189884 | 407 | Proxy Authentication Required | | | 2016-12-01 02:18:31 | 1480558711391719 | ACK | | sip:12345@1.2.3.4 | | 2016-12-01 02:18:31 | 1480558711391871 | INVITE | | sip:12345@1.2.3.4 | | 2016-12-01 02:18:31 | 1480558711392194 | 100 | Trying | | | 2016-12-01 02:18:31 | 1480558711411499 | 403 | Forbidden | | | 2016-12-01 02:18:31 | 1480558711796623 | ACK | | sip:12345@1.2.3.4 |
```
Also, I'm doing a PR to homer as well to take into account this change, which will show in the "sip search" as a "Status" column.
you are talking about "view", that can be easy changed in UI and API. but I try to make a pointer on back compatibility with already existing setups. In this case more effective way is to add a cseq_method field and add mod param for this feature.
also 5 cents for the cseq's method. What sense write it into DB as a separate field ? Do search ONLY on this field ? Unlikely.... because normally you always do search on a "reply code" + cseq method (i.e. method="200" AND cseq like "% INVITE") and the "reply code" (method) is already indexed and wildcard will be effective just like a filter. Sorry, but just want to see real arguments.
If the case only for "nice view": it's just a small trigger for the result grid. Not need to change a lot components for this.
Thank you!
I've been looking into Homer's source code, and you are correct. It's easier doing it on the resultCtrl.js. In any case, this has been a good exercise for me in terms of working with Kamailio's source code, as I'm just a beginner.
Thanks!
Closed #876.
-
Alexandr Dubovikov -
Daniel-Constantin Mierla
-
David Villasmil