[
https://issues.apache.org/jira/browse/PDFBOX-5695?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17780606#comment-17780606
]
Axel Howind commented on PDFBOX-5695:
-------------------------------------
[~lehmi] Thank you.
I will look into the POM files if there are any redundancies. I will eventually
add a final patch after doing a clean check out of trunk and going over
everything related to logging once more.
As for the different scopes: Since log4j-api is just an API, we need to supply
a logging implementation when we run tests or programs, that's why there are
two entries that ar both needed (Maven does not allow to specify multiple
scopes in a single dependency declaration).
I tried to write up something for the release notes to clarify some points
about the logging changes. More information can be found on the Log4J pages
like the
[FAQ|[https://logging.apache.org/log4j/2.x/faq.html|https://logging.apache.org/log4j/2.x/faq.html#which_jars_log4j-to-slf4j]].
*PDFBox now uses the Log4J2-API for logging*
We use log4j-api (the logging API) as a transient dependency. It supplies
everything needed to compile the code. It does not however pull in a logging
implementation.
Downstream *library* projects should not need to make any changes except maybe
adding in their release notes that a library they use now logs using log4j-api.
Library projects should not make any assumptions on the logging implementation
the library users chose. Adding any transient dependency on a wrapper or bridge
to route logging calls just makes life harder for everyone else.
Downstream *application* projects can then chose whatever logging
implementation they use by including either:
* For application projects already using {*}log4j2{*}: No action is needed.
log4j2 is directly compatible with log4j-api. Applications that use log4j2 as
their logging implementation should already have the required dependencies
(log4j-api and log4j-core).
* For application projects using *Logback* or any other SLF4J logging
implementation: If the downstream projects want to use Logback to handle the
log4j-api logs outputted by your library, they would need to include the
log4j-to-slf4j bridge:
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-to-slf4j</artifactId>
<version>2.XX.X</version> <!-- put the log4j version here -->
</dependency>
> 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: Optimize_debug_logging_across_multiple_files.patch,
> fix_log_statement_with_wrong_parameter_count.patch,
> replace_commons-logging_with_log4j.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,
> 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: [email protected]
For additional commands, e-mail: [email protected]
