[kamailio/kamailio] Sipcapture: When inserting a reply, this module would insert the SIP … (#876)
1 Dec
2016
1 Dec
'16
2:23 p.m.
…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
--
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/876
1 Dec
1 Dec
2:26 p.m.
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.
--
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/876#issuecomment-264186008
2:36 p.m.
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.
--
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/876#issuecomment-264188366
2:51 p.m.
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 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/876#issuecomment-264192515
3:12 p.m.
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!
--
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/876#issuecomment-264198234
2 Dec
2 Dec
11:03 a.m.
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!
--
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/876#issuecomment-264429830
11:04 a.m.
Closed #876.
--
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/876#event-879833214
2720
days inactive
2721
days old
6 comments
3 participants
participants (3)
-
Alexandr Dubovikov -
Daniel-Constantin Mierla
-
David Villasmil