-
Notifications
You must be signed in to change notification settings - Fork 11
Added capability to redirect to stdout #11
base: master
Are you sure you want to change the base?
Conversation
|
Hard to see "Replaced standalone Dockerfile" does. Please improve the commit message with clear motivation for:
|
|
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, 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! |
|
@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 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 |
|
I re-factored the code to comply with the current debug line. 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. 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:
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. |
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.