[CXF-6423] PrettyLoggingFilter tests and improvements

[skjolber]

See task at

https://issues.apache.org/jira/browse/CXF-6423

[cschneider]

The new code changes the behaviour for truncated messages. In the current code the result is always well formed xml. The new code for example sometimes omits closing tags. This is not correct.

[skjolber]

So for logging, why would it be logical to try to repair trunkated (or invalid xml)?

The primary goal must be to log something which correctly represents the payload content. And does that whether the content is good or bad xml, trunkated or not.

Consider this valid xml string:

<a><b><c></c></b></a>

if it is trunkated to

<a><b>

then the current approach would give you

<a><b></b></a>

as the closing tags are added. This is not only a string which is longer than the limit, it might give the impression that there is no c element. This is clearly a good way to create misunderstandings.

Another use-case: This one shows that inserting an 'content trunkated' message and then adding end tags is not a good idea either

Consider this invalid xml string:

<a><b><c></c></a>

if it is trunkated to

<a><b><c></c>

then the current approach would give you

<a><b><c></c></b></a>

which is valid xml. And so tracking the problem by looking at the logged xml would be quit difficult. So if we did

<a><b><c></c>[content trunkated]</b></a>

we would still have the same problem.

Certainly I can add some more test cases and test documentation, if you prefer.

[cschneider]

Honestly I am not sure. I think the reason for creating well formed xml was to enable processing with xml tools like xslt. I will try to clarify if it is needed.

[skjolber]

So think of it this way: The information value of the log statements should not change whether pretty-printing is enabled or not.

For log processors which require well-formed xml, a more advanced filter should be used - removing subtrees / truncating long CDATA or text nodes. A truncate filter is not sufficient.

[cschneider]

I think ideally we should have two different switches. One for the pretty printing and one for reparing the xml. There is value to both options. It makes sense to provide well formed xml for tools that need it but it can also be important to truncate at exactly the correct place.

[skjolber]

I see there were some changes done back in March which improved performance but altered the behavior.

I see now that what is really necessary is to do the same thing in this module as the core module. That means pretty-printing first then applying limit to truncate afterwards.

I'll update the pull request.

[cschneider]

Yes. That makes sense we also need a switch to enabled well formed xml though. You do not have to do it all of course.

[tomitribe-dev]

Build triggered. sha1 is original commit.