Skip to content

Better handling of multi-line logs (e.g., with backtrace) #311

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
Open
bradjones1 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Better handling of multi-line logs (e.g., with backtrace) #311

bradjones1 wants to merge 1 commit into from
+47 −23

Conversation

@bradjones1
Copy link
Contributor

@bradjones1 bradjones1 commented Jan 17, 2016

Hopefully this PR isn't too broad, but I've attempted to clean up some of the logging logic particularly when it comes to handling multi-line messages. Reading through the existing code, most messages destined for the log appear as $string, and a string is assumed, though the Logger's log() function includes an implode() if it's indeed passed an array.

I ship my logs off to an offsite log aggregation service, using rsyslog as an intermediary. Furthermore I like to package log messages as JSON, either in the application or with an rsyslog template. This PR is backwards-compatible with existing configurations but will result in only one "main" error message being logged, as opposed to the foreach() that was running which resulted in one log message per line. That can become a bit unwieldy with the backtrace.

If the logging service is syslog and the format is set to "json," the message is maintained as an array and is then JSON-encoded.

Also this downgrades the "Error report generated" message to a notice, since it is not itself the error, but simply a note about the logged error.

Includes some abstraction in Exception.php as the result of the removal of those foreach() loops.

@bradjones1
Better handling of multi-line logs. Includes some abstraction of logg…
4bd3fa1 
…ing code.

Introduces option for 'json' log format, in which case syslog will json_encode()
the log contents. Also demotes the message logging the error ID to a notice.
@bradjones1 bradjones1 mentioned this pull request Jan 30, 2016
Closed
@jaimeperez
Copy link
Member

jaimeperez commented Feb 23, 2016

Thanks for the PR @bradjones1!

Could you provide some example outputs for both the default, backwards-compatible logging, and the new format with JSON?

@jaimeperez
Copy link
Member

jaimeperez commented Apr 21, 2016

Hi again @bradjones1!

I've recently enhanced a bit the handling of logs, including those with backtraces. Backtraces are now logged as debug, so they can be ignored easily if you want.

Regarding the formatting in JSON, wouldn't that be possible if you provide a JSON-encoded string in the logging.format configuration option? In case it's not possible, or you need something more elaborate, I think it might make sense to have a separate log handler for this. That's another improvement I introduced recently. Now it's possible to use custom log handlers just by implementing the interface \SimpleSAML\Logger\LoggingHandlerInterface, making the class available for the autoloader, and referencing it in the logging.handler configuration option.

@tvdijen tvdijen mentioned this pull request Aug 4, 2017
Open
@tvdijen tvdijen force-pushed the master branch 3 times, most recently from 1c686ab to eb20457 Compare August 17, 2020 20:43
@tvdijen tvdijen force-pushed the master branch 2 times, most recently from 08ebb9c to 64fca25 Compare July 2, 2021 14:12
@tvdijen tvdijen force-pushed the master branch 2 times, most recently from 7a53fc8 to d73ae47 Compare September 26, 2021 13:03
@tvdijen tvdijen force-pushed the master branch 2 times, most recently from e5c0e21 to d5616df Compare January 9, 2022 11:00
@tvdijen tvdijen force-pushed the master branch 3 times, most recently from 2e6ab04 to 32f9acc Compare November 23, 2022 18:24
@tvdijen tvdijen force-pushed the master branch 6 times, most recently from 7e3ea19 to 2523634 Compare January 5, 2023 16:31
@tvdijen tvdijen force-pushed the master branch 6 times, most recently from 3b5f5ba to 96357ee Compare July 19, 2023 12:37
@tvdijen tvdijen force-pushed the master branch 5 times, most recently from 7587851 to d523b31 Compare August 2, 2023 11:58
@tvdijen tvdijen force-pushed the master branch 3 times, most recently from 8c90121 to d534e3b Compare September 5, 2023 08:09
@tvdijen tvdijen force-pushed the master branch 2 times, most recently from bc1c5c8 to d0a5974 Compare October 17, 2023 21:17
@tvdijen tvdijen force-pushed the master branch 3 times, most recently from ccb9b02 to 120a100 Compare December 1, 2023 14:34
@tvdijen tvdijen force-pushed the master branch 2 times, most recently from 6004a77 to 58bf8db Compare May 4, 2024 23:45
@tvdijen tvdijen force-pushed the master branch 2 times, most recently from 5c9fb2c to 0970efc Compare May 27, 2024 12:27
@tvdijen tvdijen force-pushed the master branch 2 times, most recently from c27831c to 71e49f4 Compare June 19, 2024 17:03
@tvdijen tvdijen force-pushed the master branch 3 times, most recently from c06a17a to a52c98d Compare August 20, 2025 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement low started

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bradjones1 @jaimeperez