<p> Attention is currently required from: Sean Bright, Joshua Colp, Alexei Gradinari. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/18824">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18824?tab=comments">Patch Set #3:</a> </p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">1. I think it would be better to have AND condition for different filters: hosts and methods.<br>If I set both filters on hosts and methods I suppose that both filters are in effect. In your case I have to guess which one is in effect.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Yeah, I think your intuition is right, even though it's not the way that the logger works currently. I'll do this in the next patch set.</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">2. The command "method" set filter on one or more methods in your code.<br>To be consistent with host filter and the name of command (singular) it should set filter only on one method. In this case you should compare the whole string instead of substring.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">So if I wanted to look at only INVITEs and REGISTERs, you're saying I would have to run pjsip set logger method add, run twice, as opposed to adding a filter for both at once?</p><p style="white-space: pre-wrap; word-wrap: break-word;">I think that's probably fine, yeah.</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">3. The name and brief comment of function pjsip_log_test_addr don't reflect what this function does now.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Right, I guess I'll update to pjsip_log_test_filter_match or something like that.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18824?tab=comments">Patch Set #3:</a> </p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">If anyone actually needs this level of filtering, use sngrep instead. It's the correct tool for this job<br>At some point, sngrep may be needed, but it shouldn't be required for such a basic thing. pjsip set logger on as is is pretty useless on any decently busy system, with all the noise from registers, notifys, etc. Just like filtering to a specific host, if we're trying to debug an INVITE, for example, it should be very simple to do that in the pjsip logger. That was in fact the motivation for this. It makes debugging way easier. Firing up sngrep is overkill.</p></blockquote><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">If for some reason this actually needs to be included in Asterisk: SIP methods can be anything, not just the list of ones that have been enumerated here. Put the methods that the user is interested in into a vector of strings.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">When you say "SIP methods can be anything", are you saying we should allow filtering on *anything*, like "foobar" requests? I'm not sure I understand the point of allowing filtering requests beyond those which PJSIP knows about, or why anyone would want to do that. It also means we would have to do direct string comparisons instead which would drastically worsen performance for no obvious benefit. If a request method is outside of those enumerated, Asterisk can't do anything with it, and that probably *is* better suited for either sngrep or a full blown "log everything" like is done now, at that point.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18824">change 18824</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/18824"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Idd03bd9b466b40e4bca7769437d52ac13a957cf9 </div>
<div style="display:none"> Gerrit-Change-Number: 18824 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Reviewer: Alexei Gradinari <alex2grad@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-Attention: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-Attention: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Alexei Gradinari <alex2grad@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 19 Aug 2022 12:37:37 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Comment-In-Reply-To: Alexei Gradinari <alex2grad@gmail.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>