Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging
Summary: Add ability to specify client IP and SIP account used in SIP2 logging
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords: release-notes-needed
Depends on: 15253
Blocks:
  Show dependency treegraph
 
Reported: 2020-05-12 11:13 UTC by Kyle M Hall
Modified: 2021-07-30 15:13 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
21.11.00


Attachments
Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging (3.74 KB, patch)
2020-05-12 11:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging (5.93 KB, patch)
2020-05-12 16:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging (6.21 KB, patch)
2020-05-12 16:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging (6.26 KB, patch)
2021-04-16 10:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging (6.39 KB, patch)
2021-07-23 12:24 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25464: Add ability to specify client IP and SIP account used in SIP2 logging (6.40 KB, patch)
2021-07-26 09:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25464: Adjust occurrence in debian/koha-core.postinst (932 bytes, patch)
2021-07-26 10:11 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2020-05-12 11:13:43 UTC
Bug 15253 originally had the ability to specify the incoming IP address used for a given log statement via SIP, as well as the SIP2 account that was in use at the time. This data is very helpful for debugging purposes, and should be brought back.
Comment 1 Kyle M Hall 2020-05-12 11:17:23 UTC
Created attachment 104759 [details] [review]
Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging

Bug 15253 originally had the ability to specify the incoming IP address used for
a given log statement via SIP, as well as the SIP2 account that was in use at the time.
This data is very helpful for debugging purposes, and should be brought back.

Test Plan:
1) Apply this patch
2) Update you SIP ConversionPattern to "[%d] [%p] %X{accountid}@%X{peeraddr}: %m %l %n"
3) Restart SIP
4) Use the SIP cli tester to make some SIP requests
5) View the SIP2 log, note the account id and client ip address show in the log!
Comment 2 Jonathan Druart 2020-05-12 11:53:24 UTC
Did not we want to not use Log::Log4Perl directly?

Bug 15253 comment 90.
Comment 3 Kyle M Hall 2020-05-12 12:37:35 UTC
(In reply to Jonathan Druart from comment #2)
> Did not we want to not use Log::Log4Perl directly?
> 
> Bug 15253 comment 90.

I think I misunderstood your question. I had thought you meant re-tooling all the code to remove C4::SIP::Logger and use Koha::Logger directly in all the code. Is that not what you meant?

In any case, I think what was pushed to master is good. What impact do you think that decision has related to this bug report?
Comment 4 Jonathan Druart 2020-05-12 13:16:41 UTC
I have the feeling that Log::Log4perl::MDC->put should have a wrapper method in Koha::Logger, instead of calling Log::Log4perl directly.
Comment 5 Kyle M Hall 2020-05-12 14:06:11 UTC
(In reply to Jonathan Druart from comment #4)
> I have the feeling that Log::Log4perl::MDC->put should have a wrapper method
> in Koha::Logger, instead of calling Log::Log4perl directly.

Ah, I think I understand what you were getting at now. I'll think about that and see if anything should be.
Comment 6 Kyle M Hall 2020-05-12 16:47:31 UTC
Created attachment 104794 [details] [review]
Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging

Bug 15253 originally had the ability to specify the incoming IP address used for
a given log statement via SIP, as well as the SIP2 account that was in use at the time.
This data is very helpful for debugging purposes, and should be brought back.

Test Plan:
1) Apply this patch
2) Update you SIP ConversionPattern to "[%d] [%p] %X{accountid}@%X{peeraddr}: %m %l %n"
3) Restart SIP
4) Use the SIP cli tester to make some SIP requests
5) View the SIP2 log, note the account id and client ip address show in the log!
Comment 7 Kyle M Hall 2020-05-12 16:55:06 UTC
Created attachment 104795 [details] [review]
Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging

Bug 15253 originally had the ability to specify the incoming IP address used for
a given log statement via SIP, as well as the SIP2 account that was in use at the time.
This data is very helpful for debugging purposes, and should be brought back.

Test Plan:
1) Apply this patch
2) Update you SIP ConversionPattern to "[%d] [%p] %X{accountid}@%X{peeraddr}: %m %l %n"
3) Restart SIP
4) Use the SIP cli tester to make some SIP requests
5) View the SIP2 log, note the account id and client ip address show in the log!
Comment 8 Martin Renvoize 2021-04-16 10:29:48 UTC
Created attachment 119706 [details] [review]
Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging

Bug 15253 originally had the ability to specify the incoming IP address used for
a given log statement via SIP, as well as the SIP2 account that was in use at the time.
This data is very helpful for debugging purposes, and should be brought back.

Test Plan:
1) Apply this patch
2) Update you SIP ConversionPattern to "[%d] [%p] %X{accountid}@%X{peeraddr}: %m %l %n"
3) Restart SIP
4) Use the SIP cli tester to make some SIP requests
5) View the SIP2 log, note the account id and client ip address show in the log!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2021-04-16 10:30:41 UTC
This works well and is a really useful addition to the SIP logs.
Comment 10 Nick Clemens 2021-07-23 12:24:56 UTC
Created attachment 123107 [details] [review]
Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging

Bug 15253 originally had the ability to specify the incoming IP address used for
a given log statement via SIP, as well as the SIP2 account that was in use at the time.
This data is very helpful for debugging purposes, and should be brought back.

Test Plan:
1) Apply this patch
2) Update you SIP ConversionPattern to "[%d] [%p] %X{accountid}@%X{peeraddr}: %m %l %n"
3) Restart SIP
4) Use the SIP cli tester to make some SIP requests
5) View the SIP2 log, note the account id and client ip address show in the log!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens 2021-07-23 12:25:48 UTC
Minor rebase

prove -v t/Logger.t - I get 1 failure, but I get it without these patches too, seems unrelated
Comment 12 Jonathan Druart 2021-07-26 09:50:26 UTC
Created attachment 123182 [details] [review]
Bug 25464: Add ability to specify client IP and SIP account used in SIP2 logging

Bug 15253 originally had the ability to specify the incoming IP address used for
a given log statement via SIP, as well as the SIP2 account that was in use at the time.
This data is very helpful for debugging purposes, and should be brought back.

Test Plan:
1) Apply this patch
2) Update you SIP ConversionPattern to "[%d] [%p] %X{accountid}@%X{peeraddr}: %m %l %n"
3) Restart SIP
4) Use the SIP cli tester to make some SIP requests
5) View the SIP2 log, note the account id and client ip address show in the log!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Jonathan Druart 2021-07-26 10:11:35 UTC
Created attachment 123183 [details] [review]
Bug 25464: Adjust occurrence in debian/koha-core.postinst
Comment 14 Jonathan Druart 2021-07-26 10:47:16 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 15 Kyle M Hall 2021-07-30 15:13:47 UTC
Enhancement not backported for 21.05