Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple problems in consumer.Syslogd #185

Open
ppar opened this issue Jul 23, 2017 · 2 comments
Open

Multiple problems in consumer.Syslogd #185

ppar opened this issue Jul 23, 2017 · 2 comments

Comments

@ppar
Copy link
Contributor

ppar commented Jul 23, 2017

  • If the configuration specifies a UNIX domain socket, and this socket already exists in the filesystem, consumer.Syslogd will fail with [2017-07-24 00:26:19 CEST] ERROR Failed to open unix:///tmp/gol.sock PluginID=SyslogdConsumer PluginType=consumer.Syslogd instead of simply reusing the existing socket

  • In the above situation, despite the "Error" during startup, Gollum doesn't terminate.

  • When shutting down, consumer.Syslogd doesn't remove this socket from the filesystem

  • Specifying a UDP or TCP transport for a protocol that only supports the other causes a "Warning" instead of a fatal error and termination.

  • When listening on a UNIX socket and running with Format: "RFC5424" or "RFC6587", transmitting a message with logger -u /tmp/gol.sock dadwqewqe123213123 [1] produces only an empty string in the Gollum message. Test results of all formats and transports:

# RFC3164 UNIX  OK
# RFC3164 UDP   OK
# RFC3164 TCP   N/A
#
# RFC5424 UNIX  error - empty msg
# RFC5424 UDP   OK
# RFC5424 TCP   N/A
#
# RFC6587 UNIX  error - empty msg
# RFC6587 UDP   OK
# RFC6587 TCP   OK

[1] Debian 9 x86_64, logger from bsdutils 1:2.28.2-1

  • consumer.Syslogd doesn't provide syslogd-specific fields (facility, prio, etc) as Gollum metadata fields
@arnecls
Copy link
Contributor

arnecls commented Jul 24, 2017

  1. "simply reusing" is not really possible as the socket can be owned by another process. A possible solution for this case is implemented in consumer.Socket.
  2. Can be solved by pushing the error instead of just reporting it, but again, see 1
  3. That's a bug
  4. Interesting point. Until now we tried to fix the problem if possible (e.g. by forcing the correct protocol). We could change this to "don't startup" now as config errors will always stop gollum from starting, but this has to be consequently fixed in all other plugins, too.
  5. Needs further investigation but this might not be a problem of the underlying library or the logger tool (which I personally don't think, but syslog clients seem to be broken quite often)
  6. We did not touch syslogd after adding metadata, but this is a good idea.

@arnecls arnecls added this to the v0.5.x milestone Jul 24, 2017
@ppar
Copy link
Contributor Author

ppar commented Jul 24, 2017

"simply reusing" is not really possible as the socket can be owned by another process.

Apparently it's the server.ListenUnixgram() call that fails if the socket already exists, whether's it's in use or not - so the consistent behavior here would be to just fail hard and terminate startup if this happens.

Same for the .ListenTCP() and .ListenUDP() calls in Consume(), it doesn't look like their failures abort startup either.

Removing the socket at shutdown like consumer.Socket seems to do solves the idempotency problem anyway.

We could change this to "don't startup" now as config errors will always stop gollum from starting, but this has to be consequently fixed in all other plugins, too.

Agreed.

@arnecls arnecls modified the milestones: v0.6.0, v0.5.x Aug 2, 2017
@arnecls arnecls added this to Backlog in v0.6.0 Aug 2, 2017
@arnecls arnecls removed this from the v0.6.0 milestone Jul 19, 2019
@arnecls arnecls removed this from Backlog in v0.6.0 Jul 19, 2019
@arnecls arnecls added this to Todo in v0.6.x via automation Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v0.6.x
  
Todo
Development

No branches or pull requests

2 participants