You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by skjolber <gi...@git.apache.org> on 2015/05/27 19:43:00 UTC
[GitHub] cxf pull request: [CXF-6423] PrettyLoggingFilter tests and improve...
GitHub user skjolber opened a pull request:
https://github.com/apache/cxf/pull/75
[CXF-6423] PrettyLoggingFilter tests and improvements
See task at
https://issues.apache.org/jira/browse/CXF-6423
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/skjolber/cxf master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cxf/pull/75.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #75
----
commit 14f456d60d55cdaf413ef0669ff6897660b5cab4
Author: Thomas Skjølberg <th...@gmail.com>
Date: 2015-05-27T16:46:53Z
Logging Feature pretty-printing
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cxf pull request: [CXF-6423] PrettyLoggingFilter tests and improve...
Posted by skjolber <gi...@git.apache.org>.
Github user skjolber commented on the pull request:
https://github.com/apache/cxf/pull/75#issuecomment-107853240
I see there were some changes done back in March which improved performance but altered the behavior.
I seen now that what is really necessary is to do the same thing in this module as the core module. That means pretty-printing first than applying limit to truncate afterwards.
I'll update the pull request.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cxf pull request: [CXF-6423] PrettyLoggingFilter tests and improve...
Posted by tomitribe-dev <gi...@git.apache.org>.
Github user tomitribe-dev commented on the pull request:
https://github.com/apache/cxf/pull/75#issuecomment-218430618
Build triggered. sha1 is original commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cxf pull request: [CXF-6423] PrettyLoggingFilter tests and improve...
Posted by cschneider <gi...@git.apache.org>.
Github user cschneider commented on the pull request:
https://github.com/apache/cxf/pull/75#issuecomment-107856090
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cxf pull request: [CXF-6423] PrettyLoggingFilter tests and improve...
Posted by cschneider <gi...@git.apache.org>.
Github user cschneider commented on the pull request:
https://github.com/apache/cxf/pull/75#issuecomment-107007358
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cxf pull request: [CXF-6423] PrettyLoggingFilter tests and improve...
Posted by skjolber <gi...@git.apache.org>.
Github user skjolber commented on the pull request:
https://github.com/apache/cxf/pull/75#issuecomment-107067157
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:
```xml
<a><b><c></c></b></a>
```
if it is trunkated to
```xml
<a><b>
```
then the current approach would give you
```xml
<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:
```xml
<a><b><c></c></a>
```
if it is trunkated to
```xml
<a><b><c></c>
```
then the current approach would give you
```xml
<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
```xml
<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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cxf pull request: [CXF-6423] PrettyLoggingFilter tests and improve...
Posted by cschneider <gi...@git.apache.org>.
Github user cschneider commented on the pull request:
https://github.com/apache/cxf/pull/75#issuecomment-107085737
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cxf pull request: [CXF-6423] PrettyLoggingFilter tests and improve...
Posted by cschneider <gi...@git.apache.org>.
Github user cschneider commented on the pull request:
https://github.com/apache/cxf/pull/75#issuecomment-107528593
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cxf pull request: [CXF-6423] PrettyLoggingFilter tests and improve...
Posted by skjolber <gi...@git.apache.org>.
Github user skjolber commented on the pull request:
https://github.com/apache/cxf/pull/75#issuecomment-107164240
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cxf pull request #75: [CXF-6423] PrettyLoggingFilter tests and improvements
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cxf/pull/75
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---