[
https://issues.apache.org/jira/browse/PDFBOX-5695?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17790939#comment-17790939
]
ASF subversion and git services commented on PDFBOX-5695:
---------------------------------------------------------
Commit 1914203 from le...@apache.org in branch 'pdfbox/trunk'
[ https://svn.apache.org/r1914203 ]
PDFBOX-5695: remove no longer needed rules
> Improve PDFBox Logging
> ----------------------
>
> Key: PDFBOX-5695
> URL: https://issues.apache.org/jira/browse/PDFBOX-5695
> Project: PDFBox
> Issue Type: Improvement
> Affects Versions: 4.0.0
> Reporter: Axel Howind
> Assignee: Andreas Lehmkühler
> Priority: Major
> Labels: logging, patch
> Attachments:
> Fixed_DebugLogAppender_not_being_initialised_and_runtime_exceptions_in_LogDialog.patch,
> Optimize_debug_logging_across_multiple_files.patch,
> add_logging_imports_to_benchmark_classes.patch,
> fix_log_statement_with_wrong_parameter_count.patch,
> image-2023-11-25-16-22-45-143.png,
> replace_commons-logging_by_log4j_(benchmark).patch,
> replace_commons-logging_with_log4j.patch,
> replace_commons-logging_with_log4j2_(cleanup_POM_files).patch,
> replace_commons-logging_with_log4j_(examples).patch,
> replace_commons-logging_with_log4j_(pdfbox).patch,
> replace_commons-logging_with_log4j_(pdfbox-debugger).patch,
> replace_commons-logging_with_log4j_(pdfbox-io).patch,
> replace_commons-logging_with_log4j_(pdfbox-tools).patch,
> replace_commons-logging_with_log4j_(xmpbox).patch,
> update_log4j_to_2_21_1.patch, update_log4j_to_2_22_0.patch,
> use_Property_for_log4j_version_replace_commons_logging_by_log4j-api_(fontbox).patch
>
>
> I know this has been an issue before
> (https://issues.apache.org/jira/browse/PDFBOX-693), but since then a lot of
> time has passed. I propose to switch Logging to either SLF4J (my preference)
> or Log4J.
> Why? Currently, PDFBox uses commons logging. That library seems not to be
> actively maintained anymore, the last release was in 2014 with the comment
> „the main purpose of the 1.2 release is to drop support for Java 1.1“. PDFBox
> 4 requires at least Java 11. The Java language has evolved a lot over the
> last few years, and so have to logging frameworks.
> *Maintainabilty and Performance*
> The main point that speaks against commons logging IMHO is that you have to
> make compromises between readability and performance. Let’s look at an
> example (taken from COSParser.java):
>
> {code:java}
> if (offsetOrObjstmObNr != null)
> {
> LOG.debug("Set missing offset " + offsetOrObjstmObNr + " for object " +
> objKey);
> document.getXrefTable().put(objKey, offsetOrObjstmObNr);
> }
> {code}
> Each time the if-statement's body is entered, the logging message is
> constructed. This involves:
> - converting offsetOrObjstmObNr to a String
> - converting objKey to a String which in turn does two more conversions of
> integer and long values to a String, creates a new String by joining results
> of former conversions
> - creating a new String consisting of the results of the both operations
> above and some static text
> A lot of temporary objects are created and all of this happens regardless of
> whether logging is necessary or not.
> Both SLF4J and Log4J (and even JUL logging) support a message format syntax
> where arguments to logging calls are only evaluated when needed:
>
> {code:java}
> if (offsetOrObjstmObNr != null)
> {
> LOG.debug("Set missing offset {} for object {}", offsetOrObjstmObNr,
> objKey);
> document.getXrefTable().put(objKey, offsetOrObjstmObNr);
> }
> {code}
> Using common s logging, you can guard the logging statement:
>
> {code:java}
> if (offsetOrObjstmObNr != null)
> {
> if (LOG.isDebugEnabled())
> {
> LOG.debug("Set missing offset " + offsetOrObjstmObNr + " for object "
> + objKey);
> }
> document.getXrefTable().put(objKey, offsetOrObjstmObNr);
> }
> {code}
> This works, but makes the code less readable and harder to maintain because
> the level has to be changed in two places instead of one.
> *Flexibility*
> Another thing is that both SLF4J and Log4J offer you a facade that you can
> bridge to nearly every logging implementation in widespread use nowadays
> whereas commons logging is a full logging implementation that most of us do
> not need (there are ways to make commons logging work with other backends,
> but they are more of a workaround than a clean solution).
> *JPMS support*
> And of course, both SLF4J and Log4J offer first class support for the module
> system.
> So what are the PDFBox maintainer's thoughts on this? I think PDFBox 4 offers
> an opportunity to switch to a new Logging facade/framework, and if there's a
> concensus, I volunteer to contribute a patch to make the transition.
> *Support Lambda Logging*
> In some cases, even using a message format ist not enough, for example there
> are instances in the code where a `byte[]` value is logged as a String. Since
> the String conversion has to be done explicitly, it would be done before the
> actual logging method is entered. This can be solved using lambdas by passing
> `() -> new String(value)` instead (syntax may vary).
> *Alternatives*
> The alternatives (sorted according to my own preference) are:
> - use SLF4J as logging facade
> - use Log4J2 as logging facade
> - use JUL logging (comes with a cost when used with other logging backends)
> - stay with commons logging and add guards in the code where they are not
> yet present
> - stay with commons logging and try to convince the maintainers (if there
> still are any) to add a message format support that is comparable to what the
> other three and then switch to the new version of commons logging once the
> functionality is there (and yes, if there's a chance this gets approved by
> the maintainers I'd see if I can contribute the necessary changes to commons
> logging provided the idea gets support over there)
> Cheers,
> Axel
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
