stream_exporter
stream_exporter copied to clipboard
Added capability to redirect to stdout
Added capability for redirecting to stdout, giving the capability of who ever uses Syslog to collect multiple logs, be able to use docker logs to view the logs received from other hosts.
Hard to see "Replaced standalone Dockerfile" does. Please improve the commit message with clear motivation for:
- why that is necessary ?
- what feature (value) you add or what bug you fix. ?
Lot of whitespace changes, which might upset the maintainer. Please roll back whitespace changes to keep the patch as clean as possible.
Improve the commit message of "Added capability to redirect to stdout" as well.
HI @x-wf, Thanks for the PR! Just so I understand the use-case, this is mainly for debugging what data is arriving? There is a debug statement in the general loop that might cover this already?
For a decent sized installation, I would expect this to cause significant overhead, so would be good to understand when you would find it useful.
@blenessy Thanks for taking a look, appreciate help with reviews! While I technically agree with most of your points, they come across a bit too harsh to me, which I think doesn't help the discussion. Please consider this going forward.
@carlpett You're right, but theoretically the debug in the main loop is for debugging purpose, and this PR is with intention of a functionality and not debugging, thus allowing it as input.syslog.redirectstdout parameter. But you're right anyways
@carlpett Apologies for seeming offensive, I realised that I'm commenting the Public PR too late. I was under the impression that I was reviewing our internal PR (@x-wf and me are colleagues).
@carlpett the fundamental user story is:
As a devops engineer I want to be able to route the logs from Docker container Foo to the stream_exporer Docker container so that I can extract prometheus metrics.
Enabling the syslog log-driver for container Foo allows this. However only one log-driver can be enabled on docker CE, which means that docker logs will no longer output anything for container Foo. The patch in question aims to address this problem by echoing the logs to the stream_explorer stdout/stderr so that these are not lost forever.
I re-factored the code to comply with the current debug line.
@carlpett
The scenario is quite simple here. Imagine you are given 10 different containers, and one of them performs as Syslog Server container for the rest of them.
Problem: You will not be able to view the logs individually of each container with docker logs command.
So, at the moment stream_exporter collects the logs and creates prometheus metrics, but the logs are not accessible.
For this, the Syslog Server container should output them to stdout, making all those logs accessible through the docker logs command.
Furthermore, I made changes to accept input.redirectstdout as a parameter, to make the logs output to stdout.
Question: Why not just use the debug functionality to output those logs?
Because, this is mainly to act as a functionality and not debugging purpose. Using the debug mode would imply that, even if we have more log.Debugf lines through out the code, we would also get them too. Which is not what we want.
Hope that got it cleared
Sorry, been busy a few days, so this has had to be on the wait list.
@blenessy Thanks for the context, then I understand your interactions better :)
@x-wf Right, then I understand the usage. There are "common" deployment methods I've seen and heard about for this:
- Either to direct docker to the host's syslog server and then have that split off the relevant logs to stream_exporter
- Don't use the docker daemon syslog mode, but use whatever centralized log shipping tool you have to read the json log files and both send to logstash (or similar) and syslog.
Would either of these work for you? The reason I'm wary of this direction of seeing the stream_exporter as a full-fledged syslog server is that there is a lot of functionality you'd want in a syslog server that'd we'd have to reimplement here, and the reliability expectations of a syslog server might not be easy to meet in this code.
Thoughts? I don't want to dismiss this right away, but I want the decision to add this to be well considered.
@carlpett No worry man (for the delay).
Either way the discussion goes, @x-wf will make improve this patch some more - so please hold off merging.
We are not looking for a fully featured syslog server either. In fact the plan is to enable the json logging driver for the stream_exporter Docker container, which has nice retention configurability. However, that can only be leveraged if we can get the stream_exporter to print logs to stdout/stderr :).
Please not that we are not changing the default case for your existing users, this is a new feature that must be enabled explicitly.
BTW, we are in the DLT (think blockchain) bussiness, and this will be used by the nodes on our network. Streaming logs to a central server does not go well with decentralisation, which is why we want to do some on the processing (filtering) on node.
Refactored the code.