Bug 46626

Summary: Log4J SyslogAppender does not handle the TAG field
Product: Log4j - Now in Jira Reporter: Mathieu Rozieres <mathieu.rozieres>
Component: AppenderAssignee: log4j-dev <log4j-dev>
Severity: enhancement CC: mathieu.rozieres, onlynone
Priority: P2    
Version: unspecified   
Target Milestone: 1.2.18   
Hardware: All   
OS: All   
URL: https://issues.apache.org/bugzilla/show_bug.cgi?id=46626
Attachments: Inproved SyslogAppender
Patch for SyslogAppender.java and SyslogAppenderTest.java
Patch for SyslogAppender.java and SyslogAppenderTest.java

Description Mathieu Rozieres 2009-01-29 08:16:18 UTC
Created attachment 23192 [details]
Inproved SyslogAppender

I found today an issue reported that looks like the one i am facing : the difficulty to handle tag for syslog message routing with Log4j Syslog Appender. On this issue, it was advised to use ConversionPattern Layout to handle the tag prefixing the real message payload. 

I.E, this workaround only work for small log message as the SyslogAppender slice long message in < 1024 chunks. In order to prevent the tag to only appear on the 1st syslog message, i've diverted the facilityprinting options that limit the usage of tags to the facility names. To let the user define its own tag, i've added a TagLabel property to the appender, renamed FacilityPrinting to TagPrinting and not to be too code intrusive, i've had a little hack to suffix the SyslogAppender header string with it.

I hope it will help someone out there.



Rapid syslog protocol packet description: http://www.monitorware.com/Common/en/Articles/syslog-described.php
Comment 1 Steven Willis 2012-05-17 17:58:30 UTC
Created attachment 28801 [details]
Patch for SyslogAppender.java and SyslogAppenderTest.java

This patch against trunk adds the printing of a TAG in the header of a syslog message. Unlike the previous version attached to this issue, this change doesn't affect facilityPrinting and comes with test coverage.

If a tag is not set, then the behavior is identical to the existing SyslogAppender. If tag it set to non-null, then the tag will print out with the header.
Comment 2 Gary Gregory 2012-05-22 21:30:33 UTC
I wonder if some of the code in the patch is in the wrong place. 

In "4.1.3 MSG Part of a syslog Packet" from https://tools.ietf.org/html/rfc3164 I read:

"The MSG part has two fields known as the TAG field and the CONTENT field."

So adding the tag in the method getPacketHeader() does not feel right.

Comment 3 Gary Gregory 2012-05-22 21:35:06 UTC
Hm... I also read: 

"The TAG is a string of ABNF alphanumeric characters that MUST NOT exceed 32 characters."

This patch does not truncate the Java String.

Am I looking at the right spec?
Comment 4 Steven Willis 2012-05-24 16:19:34 UTC
Regarding the TAG being part of the HEADER or MSG. That RFC definitely says that the TAG is part of the MSG. However, there are some issues with that RFC and syslog messages in general. I would highly recommend reading this write up:


Notably: 'The syslog protocol has not been standardized until relatively recently. The first document "smelling" a bit like a standard is RFC 3164, which dates back to August 2001. The problem is that this document is no real standard. It has assigned "informational" status by the IETF which means it provides some hopefully useful information but does not demand anything. It is impossible to "comply" to an informational document.'

And in future RFCs covering syslog messages, for example: http://tools.ietf.org/html/rfc5424 the TAG (now called APP-NAME) field has been moved into the header. Because, frankly, it makes more sense there.

The problem is that when a stack trace is sent along with a log message, the SyslogAppender sends each line of the stack trace as a separate syslog message and only prepends the HEADER. It doesn't send the line through the layout formatter first. I was originally using the layout format to put my TAG at the beginning of each log message, but since the stack trace lines don't go through the formatter they weren't getting my TAG.

Also, with or without a TAG, the messages written by the updated class are still compliant with rfc3164.

And about the TAG not exceeding 32 alphanumeric characters. I can update the patch to deal with that. What would be the preferred behavior? I was thinking of dealing with it inside setTag(String): silently truncate to 32 characters, then throw an exception if any non-alphanum characters are found in the remaining string.
Comment 5 Steven Willis 2012-05-24 18:17:36 UTC
Created attachment 28831 [details]
Patch for SyslogAppender.java and SyslogAppenderTest.java

This patch includes checking for the length and content of the tag.
Comment 6 Gary Gregory 2012-05-24 19:20:08 UTC
Hi Steven, 

Thank you for the new patch.

If APP-NAME is the current term, shouldn't the ivar be called 'appName' instead of 'tag'?

Comment 7 Steven Willis 2012-05-25 14:43:13 UTC
(In reply to comment #6)
> Hi Steven, 
> Thank you for the new patch.
> If APP-NAME is the current term, shouldn't the ivar be called 'appName'
> instead of 'tag'?
> Gary

Well, this class is still very old-school, it really targets classic syslog behavior. And that field is only called APP-NAME in newer syslog standards which are pretty different from the classic one. So I think it still makes sense to call it tag, since in the classic format, it's called tag.

I suppose the exact spot in the code where the tag is added to the message could be moved from getPacketHeader() to anywhere the message is constructed in order to make it more semantically correct. Though I think that complicates the code quite a bit as there will be several more places where a change is necessary and the output will be the same.
Comment 8 Gary Gregory 2012-05-25 19:00:31 UTC
Patch applied with some changes: Use a constant instead of magic numbers. Better Javadocs. Thank you Steven.
Committed revision 1342772.