Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

INT-2982 syslog Inbound Channel Adapter #780

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
Member

garyrussell commented Apr 8, 2013

Supports UDP/TCP syslog messages.

See docbook patch for more information.

Member

artembilan commented Apr 17, 2013

General comment: why not in INTEXT?

@ghillert ghillert commented on an outdated diff Apr 23, 2013

src/reference/docbook/whats-new.xml
@@ -50,6 +50,15 @@
For more information see <xref linkend="tcp-events"/>.
</para>
</section>
+ <section id="3.0-syslog">
+ <title>Syslog Support</title>
+ <para>
+ Building on the 2.2 <classname>SyslogToMapTransformer</classname> we now introduce
@ghillert

ghillert Apr 23, 2013

Member

Instead of now, maybe use Spring Integration 3.0.

@ghillert ghillert commented on an outdated diff Apr 23, 2013

src/reference/docbook/syslog.xml
+ Note the addition of the <code>protocol</code> attribute. This attribute can contain <code>udp</code>
+ or <code>tcp</code>; it defaults to <code>udp</code>.
+ </para>
+ <programlisting language="xml"><![CDATA[<int-syslog:inbound-channel-adapter id="udpSyslog"
+ channel="fromSyslog"
+ auto-startup="false"
+ phase="10000"
+ converter="converter"
+ send-timeout="1000"
+ error-channel="errors">
+ <int-syslog:udp-attributes port="1514" lookup-host="false" />
+</int-syslog:inbound-channel-adapter>]]></programlisting>
+ <para>
+ A <code>UDP</code> adapter that sends messages to channel <code>fromSyslog</code>. It also shows the
+ <interfacename>SmartLifecyle</interfacename> attributes <code>auto-startup</code> and
+ <code>phase</code>. It has a reference to a custom <interfacename>MessageConverter</interfacename>
@ghillert

ghillert Apr 23, 2013

Member

Maybe link to the JavaDoc of the MessageConverter (or add the full package of the MessageConverter) to avert ambiguity.

@ghillert ghillert commented on an outdated diff Apr 23, 2013

src/reference/docbook/syslog.xml
+<chapter xmlns="http://docbook.org/ns/docbook" version="5.0" xml:id="syslog"
+ xmlns:xlink="http://www.w3.org/1999/xlink">
+ <title>Syslog Support</title>
+
+ <section id="syslog-intro">
+ <title>Introduction</title>
+ <para>
+ Spring Integration 2.2 introduced the syslog transformer
+ <classname>SyslogToMapTransformer</classname>. This transformer, together with
+ a <code>UDP</code> or <code>TCP</code> inbound adapter could be used to receive
+ and analyze syslog records from other hosts. The transformer creates a message
+ payload containing a map of the elements from the syslog message.
+ </para>
+ <para>
+ Spring Integration 3.0 introduced convenient namespace support for configuring a
+ syslog inbound adapter in a single element.
@ghillert

ghillert Apr 23, 2013

Member

Maybe better Syslog Inbound Adapter?

@ghillert ghillert commented on the diff Apr 23, 2013

...yslog/inbound/SyslogReceivingChannelAdapterTests.java
+ Thread.sleep(1000);
+ byte[] buf = "<157>JUL 26 22:08:35 WEBERN TESTING[70729]: TEST SYSLOG MESSAGE".getBytes("UTF-8");
+ DatagramPacket packet = new DatagramPacket(buf, buf.length, new InetSocketAddress("localhost", 1514));
+ DatagramSocket socket = new DatagramSocket();
+ socket.send(packet);
+ socket.close();
+ Message<?> message = outputChannel.receive(10000);
+ assertNotNull(message);
+ assertEquals("WEBERN", message.getHeaders().get("syslog_HOST"));
+ }
+
+ @Test
+ public void testTcp() throws Exception {
+ SyslogReceivingChannelAdapterFactoryBean factory = new SyslogReceivingChannelAdapterFactoryBean(
+ SyslogReceivingChannelAdapterFactoryBean.Protocol.tcp);
+ factory.setPort(1514);
@ghillert

ghillert Apr 23, 2013

Member

maybe "detect" open port

@ghillert ghillert commented on the diff Apr 23, 2013

...yslog/inbound/SyslogReceivingChannelAdapterTests.java
+import org.springframework.integration.channel.QueueChannel;
+import org.springframework.integration.core.PollableChannel;
+import org.springframework.integration.syslog.config.SyslogReceivingChannelAdapterFactoryBean;
+
+/**
+ * @author Gary Russell
+ * @since 3.0
+ *
+ */
+public class SyslogReceivingChannelAdapterTests {
+
+ @Test
+ public void testUdp() throws Exception {
+ SyslogReceivingChannelAdapterFactoryBean factory = new SyslogReceivingChannelAdapterFactoryBean(
+ SyslogReceivingChannelAdapterFactoryBean.Protocol.udp);
+ factory.setPort(1514);
@ghillert

ghillert Apr 23, 2013

Member

Maybe use SocketUtils.findAvailableServerSocket()

@ghillert ghillert commented on the diff Apr 23, 2013

...gReceivingChannelAdapterParserTests-fail4-context.xml
+ xmlns:int-syslog="http://www.springframework.org/schema/integration/syslog"
+ xsi:schemaLocation="http://www.springframework.org/schema/integration/ip http://www.springframework.org/schema/integration/ip/spring-integration-ip.xsd
+ http://www.springframework.org/schema/integration/syslog http://www.springframework.org/schema/integration/syslog/spring-integration-syslog.xsd
+ http://www.springframework.org/schema/integration http://www.springframework.org/schema/integration/spring-integration.xsd
+ http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">
+
+ <int:channel id="foo">
+ <int:queue/>
+ </int:channel>
+
+ <int:channel id="errors">
+ <int:queue/>
+ </int:channel>
+
+ <int-syslog:inbound-channel-adapter id="fullBoatTcp"
+ port="1514"
@ghillert

ghillert Apr 23, 2013

Member

Maybe use SocketUtils.findAvailableServerSocket()

@ghillert ghillert commented on the diff Apr 23, 2013

...ation/syslog/config/spring-integration-syslog-3.0.xsd
+ <tool:annotation kind="ref">
+ <tool:expected-type type="org.springframework.integration.syslog.MessageConverter"/>
+ </tool:annotation>
+ </xsd:appinfo>
+ <xsd:documentation>
+ A converter used to map the UDP/TCP message to a Spring Integration message.
+ Default is DefaultSyslogMessageConverter.
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:attribute>
+ <xsd:attribute name="send-timeout" type="xsd:string" use="optional">
+ <xsd:annotation>
+ <xsd:documentation>
+ A timeout (milliseconds) when sending messages to 'channel' or 'error-channel'. Only
+ applies if the send might block - such as when sending to a bounded QueueChannel that
+ is currently full.
@ghillert

ghillert Apr 23, 2013

Member

Please specify also the default value if not set. Maybe you can also specify the type as:

        <xsd:simpleType>
            <xsd:union memberTypes="xsd:integer xsd:string" />
        </xsd:simpleType>

But that might be too nitt-picky for integer attributes, as STS is not providing any additional code completion here.

@ghillert ghillert and 1 other commented on an outdated diff Apr 23, 2013

...log/inbound/SyslogReceivingChannelAdapterSupport.java
+public abstract class SyslogReceivingChannelAdapterSupport extends MessageProducerSupport {
+
+ protected static final int DEFAULT_PORT = 514;
+
+ private volatile int port = DEFAULT_PORT;
+
+ protected final Log logger = LogFactory.getLog(this.getClass());
+
+ private volatile MessageConverter converter = new DefaultMessageConverter();
+
+ protected int getPort() {
+ return this.port;
+ }
+
+ public void setPort(int port) {
+ this.port = port;
@ghillert

ghillert Apr 23, 2013

Member

Should we do an assertion on permissible port numbers?

@garyrussell

garyrussell Apr 23, 2013

Member

I don't see why - you can configure the sending syslog to send them to any port.

@ghillert ghillert commented on an outdated diff Apr 23, 2013

...log/inbound/SyslogReceivingChannelAdapterSupport.java
+ *
+ * @author Gary Russell
+ * @since 3.0
+ *
+ */
+public abstract class SyslogReceivingChannelAdapterSupport extends MessageProducerSupport {
+
+ protected static final int DEFAULT_PORT = 514;
+
+ private volatile int port = DEFAULT_PORT;
+
+ protected final Log logger = LogFactory.getLog(this.getClass());
+
+ private volatile MessageConverter converter = new DefaultMessageConverter();
+
+ protected int getPort() {
@ghillert

ghillert Apr 23, 2013

Member

Javadoc missing

@ghillert ghillert commented on an outdated diff Apr 23, 2013

...log/inbound/SyslogReceivingChannelAdapterSupport.java
+ */
+public abstract class SyslogReceivingChannelAdapterSupport extends MessageProducerSupport {
+
+ protected static final int DEFAULT_PORT = 514;
+
+ private volatile int port = DEFAULT_PORT;
+
+ protected final Log logger = LogFactory.getLog(this.getClass());
+
+ private volatile MessageConverter converter = new DefaultMessageConverter();
+
+ protected int getPort() {
+ return this.port;
+ }
+
+ public void setPort(int port) {
@ghillert

ghillert Apr 23, 2013

Member

Java Doc missing

@garyrussell garyrussell INT-2982 syslog Inbound Channel Adapter
Supports UDP/TCP syslog messages.

See docbook patch for more information.
be994f4

@ghillert ghillert commented on an outdated diff Apr 23, 2013

.../config/SyslogReceivingChannelAdapterFactoryBean.java
+ return this.adapter == null ? SyslogReceivingChannelAdapterSupport.class :
+ this.adapter.getClass();
+ }
+
+ @Override
+ protected SyslogReceivingChannelAdapterSupport createInstance() throws Exception {
+ SyslogReceivingChannelAdapterSupport adapter;
+ if (this.protocol == Protocol.tcp) {
+ adapter = new TcpSyslogReceivingChannelAdapter();
+ if (this.connectionFactory != null) {
+ Assert.isNull(this.port, "Cannot specify both 'port' and 'connectionFactory'");
+ ((TcpSyslogReceivingChannelAdapter) adapter).setConnectionFactory(this.connectionFactory);
+ }
+ Assert.isNull(this.udpAdapter, "Cannot specifiy 'udp-attributes' when the protocol is 'tcp'");
+ }
+ else {
@ghillert

ghillert Apr 23, 2013

Member

Throw an Exception on else and check explicitly for the udp enum. Just in case we add a 3rd enum type, which should never ever happen...I just like being explicit :-)

@ghillert ghillert commented on an outdated diff Apr 23, 2013

.../config/SyslogReceivingChannelAdapterFactoryBean.java
+
+ private volatile int phase;
+
+ private volatile Long sendTimeout;
+
+ private volatile AbstractServerConnectionFactory connectionFactory;
+
+ private volatile UnicastReceivingChannelAdapter udpAdapter;
+
+ private volatile Integer port;
+
+ private volatile MessageConverter converter;
+
+ private volatile String beanName;
+
+ public SyslogReceivingChannelAdapterFactoryBean(Protocol protocol) {
@ghillert

ghillert Apr 23, 2013

Member

Maybe add a quick JavaDoc that depending on the Protocol either an instance of
TcpSyslogReceivingChannelAdapter or UdpSyslogReceivingChannelAdapter() is created.

@ghillert

ghillert Apr 23, 2013

Member

Maybe do a null check.

@ghillert ghillert commented on an outdated diff Apr 23, 2013

.../syslog/config/SyslogInboundChannelAdapterParser.java
+ * limitations under the License.
+ */
+package org.springframework.integration.syslog.config;
+
+import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.beans.factory.support.AbstractBeanDefinition;
+import org.springframework.beans.factory.support.BeanDefinitionBuilder;
+import org.springframework.beans.factory.xml.ParserContext;
+import org.springframework.integration.config.xml.IntegrationNamespaceUtils;
+import org.springframework.integration.ip.config.UdpInboundChannelAdapterParser;
+import org.springframework.util.StringUtils;
+import org.springframework.util.xml.DomUtils;
+import org.w3c.dom.Element;
+
+/**
+ * Parses &lt;int-syslog:inbound-channel-adapter/&gt;
@ghillert

ghillert Apr 23, 2013

Member

Maybe use {@code <int-syslog:inbound-channel-adapter>>} to make it more readable.

@ghillert ghillert commented on an outdated diff Apr 23, 2013

...ingframework/integration/syslog/MessageConverter.java
+ */
+package org.springframework.integration.syslog;
+
+import org.springframework.integration.Message;
+
+
+/**
+ * A converter to convert the raw message created by the underlying
+ * UDP/TCP endpoint to a specific form of Syslog message.
+ * @author Gary Russell
+ * @since 3.0
+ *
+ */
+public interface MessageConverter {
+
+ static String PREFIX = "syslog_";
@ghillert

ghillert Apr 23, 2013

Member

For consistency, shouldn't PREFIX go into a class SyslogHeaders? Also, I am not a fan of placing constants onto interfaces. At lease I would add some JavaDoc, explaining that PREFIX will prepend the Syslog Headers during message conversion.

@ghillert ghillert commented on an outdated diff Apr 23, 2013

...ework/integration/syslog/DefaultMessageConverter.java
+ * limitations under the License.
+ */
+package org.springframework.integration.syslog;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+
+import org.springframework.integration.Message;
+import org.springframework.integration.support.MessageBuilder;
+import org.springframework.integration.transformer.SyslogToMapTransformer;
+
+/**
+ * Default {@link MessageConverter}; delegates to a {@link SyslogToMapTransformer}
+ * to convert the payload to a map of values and also provides each field (FACILITY,
+ * SEVERITY, TIMESTAMP, HOSTNAME, TAG) as a message header (prefixed by 'syslog_').
@ghillert

ghillert Apr 23, 2013

Member

maybe JavaDoc link to MessageConverter#PREFIX

Member

garyrussell commented Apr 23, 2013

In reply to @artembilan , I don't really have a good answer for that except this is a pretty trivial module that shouldn't cause any issues with versioning/release cycles etc - it doesn't depend on any libraries so we are not going to hit any requirements to do a release because of a bug in a library,

Also, the SyslogToMapTransformer is already in core; so it just made more sense to me to make this part of the core distribution.

This is also one of the use cases for Spring XD.

@markfisher ?

Member

markfisher commented Apr 23, 2013

I agree we should go ahead and move this into core. It will be an increasingly popular module for ingestion of log data.

Member

garyrussell commented Apr 23, 2013

Rebased; pushed fixes for PR comments.

Member

ghillert commented Apr 24, 2013

LGTM - merging

@ghillert ghillert pushed a commit to ghillert/spring-integration that referenced this pull request Apr 24, 2013

Gunnar Hillert Merge pull request #780 from garyrussell/INT-2982
* garyrussell-INT-2982:
  INT-2982 Add Syslog Inbound Channel Adapter
e6753ef
Member

ghillert commented Apr 24, 2013

Merged.

Did minor fixes:

  • Removed the semicolon after @deprecated; in SyslogToMapTransformer - caused a warning in JDK7
  • Removed trailing white-space

@ghillert ghillert closed this Apr 24, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment