Add write_syslog output plugin #3019

sradco · 2018-12-10T08:03:06Z

This patch adds an output plugin to send metrics in CEE-enhanced Syslog message format by TCP .
The syslog message includes the metrics data in human readable, structured data or json format.

The plugin is based on the write_tsdb and the write_graphite plugins.

It allows adding additional metadata.

I plan to add to it the option to send the data also as unix socket.

An example of the record after it was received by Rsyslog:

Dec 20 12:11:22 sradco.tlv.csb collectd [time=1545300683][collectd memory.if_dropped.tx=445906944 plugin="memory" plugin_instance="" type_instance="buffered" type="memory" interval=10][hostname="sradco.tlv.csb"][ovirt engine="test" vm="test2"] memory.if_dropped.tx=445906944

Dec 20 12:11:22 sradco.tlv.csb collectd {"time":1545300683, "collectd":{ "memory":{ "if_dropped.tx":445906944 }, "plugin":"memory", "plugin_instance":"", "type_instance":"buffered", "type":"memory", "interval":10 }, "hostname":"sradco.tlv.csb" , "ovirt": {"engine":"test","vm":"test2"}}

Where "ovirt": {"engine":"test","vm":"test2"} is metadata.
Configuration for this output plugin:

LoadPlugin write_rsyslog

<Plugin write_rsyslog>
   <Node "example">
     Host "localhost"
     Port "44514"
     Prefix "collectd"
     AlwaysAppendDS false
     MessageFormat "human"
     HostTags [tag1="value"][tag2="value2"]
   </Node>
 </Plugin>

ChangeLog: Write Syslog plugin: "write_syslog" plugin writes values lists as syslog messages.

Signed-off-by: Shirly Radco sradco@redhat.com

faxm0dem · 2018-12-12T12:28:47Z

is there anything specific to rsyslog in your plugin? if not, write_tcp_json or similar would make more sense for a name.

rgolangh · 2018-12-31T19:47:48Z

@faxm0dem If I plan to add the option to output to unixsocket with the same json format. Should I add a different plugin? or should I use this one and give it a more general name(that is the reason I called it write_rsyslog)?

perhaps just "write_json" for a name?

sradco · 2018-12-31T19:49:42Z

@faxm0dem If I plan to add the option to output to unixsocket with the same json format. Should I add a different plugin? or should I use this one and give it a more general name(that is the reason I called it write_rsyslog)?

perhaps just "write_json" for a name?
The format sent is of syslog message. The metric itself is in the msg part in json format.
I plan to have this for rsyslog with tcp/udp/unixsocket. @rgolangh

rgolangh · 2019-01-02T14:25:21Z

Taking back my comment about naming, it is pretty rsyslog specific as far as I understand.

@faxm0dem general question - I see that across all plugins, every invocation with pthread_mutex_lock is ignoring the error that may return. Why is that?

sradco · 2019-01-08T13:45:38Z

Hi, @octo, @tokkee, @pyr I would really appreciate your review for this patch.
I believe this an important plugin that send the metrics in syslog format like log messages.
The msg contant includes the metrics in json format that is very easy to parse and also allows adding any metadata the user needs in a very simple way.

faxm0dem · 2019-01-10T13:40:40Z

First of all sorry for not answering your requests earlier.
Could you please explain what your plugin does specifically with regards to rsyslog? Would it also work for syslog? syslog-ng? To me it looks like it's simply a TCP (or UDP) output that writes JSON formatted output.

sradco · 2019-01-10T13:58:33Z

First of all sorry for not answering your requests earlier.
Could you please explain what your plugin does specifically with regards to rsyslog? Would it also work for syslog? syslog-ng? To me it looks like it's simply a TCP (or UDP) output that writes JSON formatted output.

@faxm0dem Thank you for your review. The plugin send the metrics in CEE-enhanced Syslog message format. The syslog message part includes the metrics data in a json format.
This allows a very simple way to get the metrics into Rsyslog and parse the "msg" part of the syslog messages, using the Json parser.

Rsyslog expects syslog messages. Having a simple Json or key-value format does is not handled well in Rsyslog.
I verified the patch and it works really well. This is a very efficient way for us to ship the metrics to elasticsearch, using the rsyslog omelasticsearch output plugin, for example.

I used the following for structuring the data before the "msg":
https://tools.ietf.org/html/rfc5424

This is an example of a similar way we are using the messages that we get from Collectd:
https://www.rsyslog.com/tag/cee-enhanced/

I believe this should work for syslog and syslog-ng as well. But I did not test.

faxm0dem · 2019-01-11T09:37:20Z

I can confirm syslog-ng can parse JSON and filter using the marker option.

I therefore still think the plugin in its current form shouldn't be called write_rsyslog, but rather write_tcp or write_tcp_json.
Moreover, I believe (without reading the code in detail) that the code:

would benefit from deduplication with write_graphite, which it is based on
could be improved later by adding the possibility to share the formatters of write_http
could be further made more generic by using either tcp or udp transport

So IMHO in its current form it should be renamed to remove the rsyslog specifics, and for the rest I'm kindly asking for a second opinion :-)

sradco · 2019-01-13T08:08:15Z

I can confirm syslog-ng can parse JSON and filter using the marker option.

@faxm0dem First I really appreciate your review. Thank you.

I need to emphasis that Only the msg part of the syslog message is Json.
There is no input plugin that accept Json records in rsyslog. Only syslog messages.
There is a specific syslog message format before the msg that must be used it order for rsyslog to treat the message coming from Collectd correctly. This is what makes this message specific to rsyslog. I did not test with syslog-ng and syslog but I guess it should work the same there.
https://tools.ietf.org/html/rfc5424#section-6

The first example here https://asylum.madhouse-project.org/blog/2013/07/29/json-howto/ for processing plain json records is not available in rsyslog. But having syslog message with the json as the "message" part of the syslog message will work for both rsyslog and syslog-ng.

It needs a HEADER, PRI(combination of Facility and Severity), HOSTNAME, APP-NAME(collectd), PROCID(empty for now), MSGID, STRUCTURED-DATA(no need for now so left empty), MSG. These are the minimal values required for rsyslog to treat the message coming from Collectd as a syslog message.

I therefore still think the plugin in its current form shouldn't be called write_rsyslog, but rather write_tcp or write_tcp_json.

Simple json will not work for the above. Rsyslog/syslog-ng/syslog expect a syslog message format.
I have tested this with Rsyslog.

Moreover, I believe (without reading the code in detail) that the code:

would benefit from deduplication with write_graphite, which it is based on

could be improved later by adding the possibility to share the formatters of write_http

could be further made more generic by using either tcp or udp transport

If we have a generic tcp output with different messages to different outputs it can potentially work, but the code will be very complicated. The way that we build the message for rsyslog is not like for tsdb or graphite. Why do each of them have a different plugin? They both ship with tcp.
Like TSDB expects certain message structure and graphite expects a certain metric name(concatenated with dots), so does Rsyslog.

So IMHO in its current form it should be renamed to remove the rsyslog specifics, and for the rest I'm kindly asking for a second opinion :-)

faxm0dem · 2019-01-15T13:13:49Z

Thanks for clarifying. I agree on your analysis with respect to factorizing code.
I guess we should make a new pull request that would factor out the common code between all those plugins doing TCP.

Now, that it's clear this plugin targets the syslog format, why bother using JSON as the MSG part? You could use the STRUCTURED-DATA to store collectd plugin, type, instances and metric. Then any RFC5424 capable software would decode the parts automatically. Moreover, we could still use the MSG part to add the data in human-readable (aka unixsock) form:

mockup

<13>1 2019-01-15T14:06:51+01:00 node42.example.com collectd 21525 - [collectd plugin="load"][collectd type="load"][collectd type_instance="relative"][collectd ds_name="shortterm"][collectd metric="6.5e-02"] load/load-relative shortterm=6.500000e-02

In this case the plugin's name should be write_syslog or write_ietf or write_rfc5424

sradco · 2019-01-15T13:50:18Z

Thanks for clarifying. I agree on your analysis with respect to factorizing code.
I guess we should make a new pull request that would factor out the common code between all those plugins doing TCP.

Now, that it's clear this plugin targets the syslog format, why bother using JSON as the MSG part? You could use the STRUCTURED-DATA to store collectd plugin, type, instances and metric. Then any RFC5424 capable software would decode the parts automatically. Moreover, we could still use the MSG part to add the data in human-readable (aka unixsock) form:

mockup
<13>1 2019-01-15T14:06:51+01:00 node42.example.com collectd 21525 - [collectd plugin="load"][collectd type="load"][collectd type_instance="relative"][collectd ds_name="shortterm"][collectd metric="6.5e-02"] load/load-relative shortterm=6.500000e-02
In this case the plugin's name should be write_syslog or write_ietf or write_rfc5424

Json allows nesting and other abilities that structured data does not and for us it really simplified sending the data to elasticsearch.

But, I can definitely add a variable that user can choose between json and structured data formats.
About the name, Perhaps write_syslog would be exceptable, since I think its clearer. We can state the RFC5424 in the description.

What do you think @faxm0dem ?

Thanks again for your comments and review.

faxm0dem · 2019-01-17T11:10:34Z

write_syslog sounds right, and it would be awesome if you could implement the structured data bit, thanks!

I guess three options would make it flexible:

MessageFormat = "human"|"json" (human by default)
EnableStructuredData = true|false (true by default)
StructuredDataPrefix = "collectd" would affect both RFC5424 SDATA and the JSON object

FWIW when using syslog-ng for sending data to ES, in my example above format-json will create structured JSON automatically.

sradco · 2019-01-17T12:27:31Z

write_syslog sounds right, and it would be awesome if you could implement the structured data bit, thanks!

I guess three options would make it flexible:

MessageFormat = "human"|"json" (human by default)

EnableStructuredData = true|false (true by default)

Can you please explain what is he difference between the MessageFormat = "human" and EnableStructuredData= true ?
Why do we need the EnableStructuredData ?

Thanks!

StructuredDataPrefix = "collectd" would affect both RFC5424 SDATA and the JSON object

FWIW when using syslog-ng for sending data to ES, in my example above format-json will create structured JSON automatically.

I don't believe it is possible in Rsyslog...

sradco · 2019-01-20T17:50:48Z

@faxm0dem Please review my patch.
I update the name to write_rsyslog.
Added:

MessageFormat = "human"|"json" (human by default)
StructuredDataPrefix = "collectd" -> I already have a "Prefix" variable, and it is used for both structured data and Json.

Not sure why we need the flag:
EnableStructuredData = true|false (true by default)

sradco · 2019-01-20T17:54:47Z

I run contrib/format.sh src/write_syslog.c but there are no changes...

faxm0dem · 2019-01-20T21:04:01Z

my idea is that the user could choose to disable RFC5425 SDATA

sradco · 2019-01-20T21:36:13Z

I'll be happy to implement but I don't understand hat do you mean by disabling RFC5425 SDATA? How will the message look like?
Can you please share an example? @faxm0dem

rgolangh · 2019-01-21T11:28:07Z

On Sun, 20 Jan 2019 at 23:36, Shirly Radco ***@***.***> wrote: I'll be happy to implement but I don't understand hat do you mean by disabling RFC5425 SDATA? How will the message look like? Can you please share an example? @faxm0dem <https://github.com/faxm0dem> —

Why do we want to implement and support that? this is clearly out of our scope, and we don't have enough time to cover this case. Shirly implemented the syslog format, this is our MVP. Unless you say this is a no-go, can we please continue with that? You are receiving this because you were mentioned.

…

Reply to this email directly, view it on GitHub <#3019 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEwcHOFsyspOsiuqud3cQtRKa7fUhzjzks5vFOFQgaJpZM4ZK1_f> .

sradco · 2019-01-22T12:51:08Z

@faxm0dem Hi Fabien, Can we agree that this can later be added for other use cases if needed and approve this PR for now?

src/collectd.conf.pod

rubenk · 2019-03-05T09:14:38Z

src/collectd.conf.pod

+It implements the basic syslog protocol, extends it with
+content-based filtering, rich filtering capabilities,
+flexible configuration options and adds features such as using TCP for transport.
+The plugin can connect to a I<Syslog> daemon that will ingest metrics, transform and


Perhaps give an example of such a daemon. Which daemons support this input? rsyslog, syslog-ng?

I can provide a simple syslog-ng config

src/write_syslog.c

rubenk · 2019-03-05T09:27:59Z

I did another pass, and saw that you missed a few of my earlier comments.
Reviewing this is really hard if all the issues are dismissed because they also exists in other plugins.

I'd rather you spend an hour refactoring this plugin before it hits the tree, there should never be a rush to get things in.
Since @octo promised you it's fine to clean up afterwards, I'll leave the decision to merge up to him, but please address the few new comments I left.

sradco · 2019-03-07T13:46:40Z

@rubenk @faxm0dem Hi, I made the fixes for the issues you have raised. Please review.
If you can help me with syslog-ng base configuration it will be great. Thanks.

This patch adds an output plugin to send metrics as CEE-enhanced Syslog log messages by TCP . The syslog message includes the metrics data in human readable structured data format and in json format. It allows adding additional metedata. This plugin is based on the write_tsdb plugin. Signed-off-by: Shirly Radco <sradco@redhat.com>

faxm0dem · 2019-03-09T21:29:42Z

sure thing I'll do that on Monday

faxm0dem · 2019-03-15T10:42:16Z

Sorry for the delay. Here's an example source config for syslog-ng.
It configures collectd-write_syslog in human/rfc5424/SDATA mode.

collectd config

MessageFormat human

syslog-ng config


destination d_collectd {
    file(
      "/var/log/collectd.log"
      template("$(format-json -s rfc5424 -s all-nv-pairs)\n")
    );
  };

source s_collectd {
   network(
      ip("127.0.0.1")
      port(44514)
      transport(tcp)
      flags(syslog-protocol)
    );
}

log {
  source(s_collectd);
  destination(d_collectd);
};

sradco · 2019-03-21T13:08:15Z

To add this to the src/collectd.conf.pod ? @faxm0dem

faxm0dem · 2019-03-22T09:31:49Z

@rubenk what do you think?

sradco · 2019-03-25T10:44:50Z

Hi, Please update me about the place to add the example.
I don't believe I saw these kind of examples in the collectd documentation before.
If there is, I'll be happy if you can point me to it for reference.
@faxm0dem @rubenk @octo

sradco · 2019-04-03T09:23:27Z

Ping @faxm0dem @rubenk @octo

sradco · 2019-05-05T11:01:43Z

Ping @faxm0dem @rubenk @octo

mrunge · 2019-05-06T08:07:15Z

Ruben, I believe, Shirly did the changes you requested? Is there anything else missing?

faxm0dem · 2019-05-07T07:37:46Z

LGTM

the following can be added to the wiki: syslog-ng and rsyslog examples (see above comment)
the json model isn't perfect from a normalizing perspective, but users will be able to change it in the future (see next point), or they can use the SDATA alternative right away
some code deduplication will be needed at some point, with all the tcp/udp write plugins (graphite, tsdb, etc.) but this can be done in separate PR as stated by @octo

So again, LGTM as-is and can be merged for now as far as I'm humbly concerned :)

sradco force-pushed the master branch from d44883f to 6d0dae6 Compare December 10, 2018 08:17

sradco force-pushed the master branch 2 times, most recently from a127ef2 to 03f7463 Compare December 20, 2018 10:14

sradco force-pushed the master branch 2 times, most recently from 4bcf823 to a1aa19d Compare January 6, 2019 14:02

sradco force-pushed the master branch from a1aa19d to 4eca404 Compare January 9, 2019 12:27

sradco force-pushed the master branch from 4eca404 to a867990 Compare January 20, 2019 17:26

sradco changed the title ~~Add write_rsyslog output plugin~~ Add write_ssyslog output plugin Jan 20, 2019

sradco changed the title ~~Add write_ssyslog output plugin~~ Add write_syslog output plugin Jan 20, 2019

sradco force-pushed the master branch 2 times, most recently from 1d2a65b to 9f12b91 Compare January 20, 2019 17:48