Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SIP plugin: add logger callback function for processing SIP message traces in a single log callback for Sofia >= 1.13 #3386

Merged
merged 4 commits into from
Jul 1, 2024
Merged

SIP plugin: add logger callback function for processing SIP message traces in a single log callback for Sofia >= 1.13 #3386

merged 4 commits into from
Jul 1, 2024

Conversation

ycherniavskyi
Copy link
Contributor

Fixes #3365.

@lminiero
Copy link
Member

lminiero commented Jun 6, 2024

Thanks for reviving this! I'll review in the next few days and get back to you 👍
Have you tested this with both 1.12 and 1.13?

@ycherniavskyi
Copy link
Contributor Author

I've specifically tested on version 1.13. And in current implementation logic for version 1.12 was not changed at all.

lminiero
lminiero reviewed Jun 14, 2024
View reviewed changes
Copy link
Member

@lminiero lminiero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've added some notes inline. Revisiting the logger, I'm starting to wonder whether it's safe to use the static call_id as we do here: are we sure the logger is always invoked by the same thread? Have you checked this as part of your tests?

src/plugins/janus_sip.c Outdated Show resolved Hide resolved
src/plugins/janus_sip.c Outdated Show resolved Hide resolved
src/plugins/janus_sip.c Show resolved Hide resolved
src/plugins/janus_sip.c Outdated Show resolved Hide resolved
@lminiero
Copy link
Member

@ycherniavskyi were you waiting for more feedback from me, or have all notes been addressed in the commits that followed?

@ycherniavskyi
Copy link
Contributor Author

@lminiero I've addressed all your comments except for the one about dropping OPTIONS, which we decided to leave (assuming I understood you correctly). Therefore, from my perspective, this PR is ready to merge! 😊

@lminiero
Copy link
Member

lminiero commented Jul 1, 2024

Thanks, merging then! I'll also backport this to 0.x 👍

@lminiero lminiero merged commit 15edae8 into meetecho:master Jul 1, 2024
8 checks passed
lminiero pushed a commit that referenced this pull request Jul 1, 2024
@ycherniavskyi @lminiero
SIP plugin: add logger callback function for processing SIP message t…
50b9150 
…races in a single log callback for Sofia >= 1.13 (#3386)
@ycherniavskyi ycherniavskyi deleted the siptrace_single_log_callback branch July 3, 2024 08:42
@lminiero lminiero mentioned this pull request Sep 4, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lminiero lminiero lminiero left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[1.x] Increased event handlers SIP logging buffer size one more time or make it configurable
2 participants
@ycherniavskyi @lminiero