2 Jul
2024
2 Jul
'24
6:59 a.m.
<!-- Kamailio Pull Request Template -->
<!--
IMPORTANT:
- for detailed contributing guidelines, read:
https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md
- pull requests must be done to master branch, unless they are backports
of fixes from master branch to a stable branch
- backports to stable branches must be done with 'git cherry-pick -x
...'
- code is contributed under BSD for core and main components (tm, sl, auth, tls)
- code is contributed GPLv2 or a compatible license for the other components
- GPL code is contributed with OpenSSL licensing exception
-->
#### Pre-Submission Checklist
<!-- Go over all points below, and after creating the PR, tick all the checkboxes
that apply -->
<!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines
from above-->
<!-- If you're unsure about any of these, don't hesitate to ask on
sr-dev mailing list -->
- [x] Commit message has the format required by CONTRIBUTING guide
- [x] Commits are split per component (core, individual modules, libs, utils, ...)
- [x] Each component has a single commit (if not, squash them into one commit)
- [x] No commits to README files for modules (changes must be done to docbook files
in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change
- [ ] Small bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds new functionality)
- [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist:
<!-- Go over all points below, and after creating the PR, tick the checkboxes that
apply -->
- [ ] PR should be backported to stable branches
- [x] Tested changes locally
- [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description
Allow Kamailio modules to register their log function so that all Kamailio log messages
(coming from core and other modules) are logged uniformly in the format defined by the
custom log function.
To enable the custom log format feature, Kamailio must be compiled with the
`CUSTOM_LOG_FMT` flag. Then the module that wishes to modify the log format has to call
`km_custom_log_func_set` during the initialization to supply a custom log formatting
function (see `km_default_custom_log_func` as an example).
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/3902
-- Commit Summary --
* core: add support for custom log function
-- File Changes --
M src/core/dprint.c (19)
M src/core/dprint.h (47)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/3902.patch
https://github.com/kamailio/kamailio/pull/3902.diff
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3902
You are receiving this because you are subscribed to this thread.
Message ID: &lt;kamailio/kamailio/pull/3902(a)github.com&gt;
3 Jul
3 Jul
2:25 a.m.
@anmartan pushed 1 commit.
52277d9b74e6ba2973ecaa0163a49be54bdd5454 core: add support for custom log function
--
View it on GitHub:
https://github.com/kamailio/kamailio/pull/3902/files/411b3c9fa3157437a0973c…
You are receiving this because you are subscribed to this thread.
Message ID:
<kamailio/kamailio/pull/3902/before/411b3c9fa3157437a0973cf66ee47fc6096b4e21/after/52277d9b74e6ba2973ecaa0163a49be54bdd5454(a)github.com>
4 Jul
4 Jul
4:57 a.m.
Any reason that:
```
ksr_logdata_t __kld = {0};
```
I replaced with:
```
ksr_logdata_t __kld = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
```
Also, `asprintf()` allocates memory, it is freed somewhere?
Then, when this new mechanism is used and what is the benefit? Can you provide some
examples?
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3902#issuecomment-2208459573
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3902/c2208459573(a)github.com>
6:25 a.m.
@anmartan pushed 1 commit.
2ecd44410be96c441f137a0c02ffe47a51f20fd9 --amend
--
View it on GitHub:
https://github.com/kamailio/kamailio/pull/3902/files/52277d9b74e6ba2973ecaa…
You are receiving this because you are subscribed to this thread.
Message ID:
<kamailio/kamailio/pull/3902/before/52277d9b74e6ba2973ecaa0163a49be54bdd5454/after/2ecd44410be96c441f137a0c02ffe47a51f20fd9(a)github.com>
6:30 a.m.
Any reason that:
```
ksr_logdata_t __kld = {0};
```
I replaced with:
```
ksr_logdata_t __kld = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
```
I've reverted the change.
Also, `asprintf()` allocates memory, it is freed
somewhere?
It is now, at the end of the function.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3902#issuecomment-2208643702
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3902/c2208643702(a)github.com>
6:40 a.m.
Hi @miconda ,
thanks for your feedback!
Regarding `ksr_logdata_t`, most likely it got flagged by some static checker, but
we've re-tested the original version and couldn't pin-point which exact check/rule
triggered this.
Regarding the `asprintf` - thanks for spotting the memory leak! We've never detected
it using our memory checkers since we never actually use the default stub continuously -
it always gets replaces when our custom module is initialised.
Then, when this new mechanism is used and what is the
benefit? Can you provide some examples?
The benefits of being able to control the output format of the logs are mainly evident
when Kamailio is used in a deployment alongside other products/tools, all of which are
generating their logs using a common scheme. Thus, being able to control Kamailio log
output scheme helps to:
1. keep the log collection/processing pipeline manageable (since all logs flowing through
it share a common scheme)
2. reduce the developer cognitive load from having to switch to different log formats
during development stage (not dealing with production logs from log pipeline but with
direct Kamailio output)
Here's a sample output made possible with the help of this patch:
(scheme used: `<pname> <ver> <timestamp> <lvl> <mname>
<pid>:<file>:<line> <msg>`)
```
pname 1.35.15 2024-06-20T10:43:24.134886Z WARNING pname 13:libpname/core.cpp:85 Core
successfully created!
pname 1.35.15 2024-06-20T10:43:24.135058Z INFO tls 13:tls_mod.c:445 use OpenSSL
version: 30000070
pname 1.35.15 2024-06-20T10:43:24.135079Z INFO tls 13:tls_mod.c:447 With
ECDH-Support!
pname 1.35.15 2024-06-20T10:43:24.135096Z INFO tls 13:tls_mod.c:450 With Diffie
Hellman
pname 1.35.15 2024-06-20T10:43:24.135166Z WARNING tls 13:tls_init.c:980 openssl bug
#1491 (crash/mem leaks on low memory) workaround enabled (on low memory tls operations
will fail preemptively) with free memory thresholds 11534336 and 5767168 bytes
pname 1.35.15 2024-06-20T10:43:24.135203Z INFO core 13:core/cfg/cfg_ctx.c:589
tls.low_mem_threshold1 has been changed to 11534336
pname 1.35.15 2024-06-20T10:43:24.135223Z INFO core 13:core/cfg/cfg_ctx.c:589
tls.low_mem_threshold2 has been changed to 5767168
...
pname 1.35.15 2024-06-20T10:43:24.136142Z INFO tls 13:tls_mod.c:491 Loading SSL_CTX
in process_no=0 rank=-127 ksr_tls_threads_mode=0
pname 1.35.15 2024-06-20T10:43:24.136176Z INFO tls 13:tls_domain.c:353
TLSs<default>: tls_method=23
pname 1.35.15 2024-06-20T10:43:24.136200Z INFO tls 13:tls_domain.c:365
TLSs<default>: certificate='/usr/local/lib/pname/certs/serv.crt'
...
pname 1.35.15 2024-06-20T10:43:33.612026Z TRACE pname
16:libpname/log/trace_object.hpp:19 Create > binding::SipMsg()
pname 1.35.15 2024-06-20T10:43:33.612093Z TRACE pname
16:libpname/binding/sip_msg.cpp:645 Call > binding::SipMsg::getCallId()
pname 1.35.15 2024-06-20T10:43:33.612136Z TRACE pname 16:libpname/log/logger.cpp:100
Call > zroute::Logger::setTraceId("3811ef5eb36d7024f1f3e7bfaf70e572")
pname 1.35.15 2024-06-20T10:43:33.612194Z INFO pname
16:/usr/local/etc/pname/./route.cfg:11 TraceID[3811ef5eb36d7024f1f3e7bfaf70e572]
Processing "INVITE" request received from "sip:10.22.10.9:5060"
[Call-Id: "8t1b49///1-94(a)10.22.10.9"]
pname 1.35.15 2024-06-20T10:43:33.612218Z INFO debugger 16:debugger_api.c:351
TraceID[3811ef5eb36d7024f1f3e7bfaf70e572] request_route=[DEFAULT_ROUTE]
c=[/usr/local/etc/pname/./route.cfg] l=12 a=25 n=MYLOG_DBG
pname 1.35.15 2024-06-20T10:43:33.612244Z TRACE pname 16:libpname/log/logger.cpp:110
TraceID[3811ef5eb36d7024f1f3e7bfaf70e572] Call > zroute::Logger::isTraceIdSet()
pname 1.35.15 2024-06-20T10:43:33.613788Z DEBUG pname
16:/usr/local/etc/pname/././invite.cfg:2 TraceID[3811ef5eb36d7024f1f3e7bfaf70e572]
Received INVITE request, routing
pname 1.35.15 2024-06-20T10:43:33.612266Z DEBUG pname
16:/usr/local/etc/pname/./route.cfg:12 TraceID[3811ef5eb36d7024f1f3e7bfaf70e572] INVITE
sip:test@example.com SIP/2.0
```
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3902#issuecomment-2208661203
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3902/c2208661203(a)github.com>
11 Jul
11 Jul
4:41 a.m.
Hi @miconda, is there anything else that needs clarifying?
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3902#issuecomment-2222361751
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3902/c2222361751(a)github.com>
18 Jul
18 Jul
4:21 a.m.
I had no time for a proper review, it seems rather important change considering that
logging is critical component. Does it affect in any way the modules `log_custom` or
`log_systemd`?
Maybe other can review as well and make remarks on this change.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3902#issuecomment-2235915993
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3902/c2235915993(a)github.com>
4:44 a.m.
Does it affect in any way the modules log_custom or
log_systemd?
It does not. Mentioned modules rely on the `_km_log_func` (set via `km_log_func_set`)
while this patch uses `_km_custom_log_func` (set via `km_custom_log_func_set`) and makes
no changes to the `_km_log_func`. The only downside is that if the feature from this patch
is enabled using the `CUSTOM_LOG_FMT`, then default log macros will bypass `_km_log_func`
and will use `_km_custom_log_func` instead. In short - mentioned modules are currently not
compatible with this patch.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3902#issuecomment-2235959446
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3902/c2235959446(a)github.com>
29 Aug
29 Aug
10:54 p.m.
This PR is stale because it has been open 6 weeks with no activity. Remove stale label or
comment or this will be closed in 2 weeks.
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3902#issuecomment-2319752277
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3902/c2319752277(a)github.com>
30 Aug
30 Aug
2 a.m.
Does it affect in any way the modules log_custom or
log_systemd?
Hi @miconda , did the above answer provide sufficient details for you?
it seems rather important change considering that
logging is critical component
Perhaps you would us to make any additional changes to further isolate this feature to
reduce the chance of misuse or unintended performance degradation?
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3902#issuecomment-2320154108
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3902/c2320154108(a)github.com>
5:29 a.m.
I will look into it in more details soon, summer holidays kept me out of the office quite
a lot. It would be good to find a way to have a single framework that works for all
logging related modules, like `log_custom` or `log_systemd` (maybe these modules can be
adapted to the way proposed by this patch, if it offers more flexibility that the existing
one).
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/3902#issuecomment-2320640758
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/pull/3902/c2320640758(a)github.com>
3
days inactive
62
days old
11 comments
4 participants
participants (4)
-
Daniel-Constantin Mierla -
github-actions[bot]
-
Ivan Ribakov
-
Nirv.Ana