Skip to content

Add Logs_syslog_unix.unix_reporter and Logs_syslog_lwt.unix_reporter#5

Merged
hannesm merged 3 commits intohannesm:masterfrom
dra27:unix-sockets
Sep 17, 2018
Merged

Add Logs_syslog_unix.unix_reporter and Logs_syslog_lwt.unix_reporter#5
hannesm merged 3 commits intohannesm:masterfrom
dra27:unix-sockets

Conversation

@dra27
Copy link
Contributor

@dra27 dra27 commented Aug 11, 2018

This PR adds the ability to connect to /dev/log and post messages to the local syslog daemon.

This is related to a PR in the syslog-message library, and I'd expect the second commit to be blown away (and the opam file updated) before merging.

Code for restoring connections factored out of tcp_reporter and a Unix
domain sockets version added in unix_reporter.
@dra27
Copy link
Contributor Author

dra27 commented Aug 11, 2018

There seems to be something up with Travis - I think the failing jobs just need restarting.

@dra27 dra27 mentioned this pull request Aug 11, 2018
fails, the log message is reported to standard error, and attempts are made
to re-establish the connection. A syslog message is truncated to [truncate]
bytes and is framed according to the given [framing]. The default for
[truncate] is [65536] is [framing] is not provided and [0] otherwise. If
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: second is in line 36 should be if

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, ta!

@hannesm
Copy link
Owner

hannesm commented Aug 13, 2018

this looks good to me, but it diverges the unix and lwt backend. could I bother you to adjust the lwt backend in a similar fashion? once syslog-message is re-released, I'm happy to merge the first commit here

Code for restoring connections factored out of tcp_reporter and a Unix
domain sockets version added in unix_reporter.
@dra27 dra27 changed the title Add Logs_syslog_unix.unix_reporter Add Logs_syslog_unix.unix_reporter and Logs_syslog_lwt.unix_reporter Aug 14, 2018
@dra27
Copy link
Contributor Author

dra27 commented Aug 14, 2018

A very good point - I've added an Lwt version, although I haven't (yet) and I have now tested it. The required change in syslog-message is now merged, so I've altered the temporary commit to be a pin-depends rather than a code-hack (not that that makes it any less unmergeworthy!)

@dra27
Copy link
Contributor Author

dra27 commented Aug 14, 2018

Hmm, broken something on the Mirage build...

@hannesm
Copy link
Owner

hannesm commented Aug 15, 2018

thanks, now that I merged #7, would you be so kind to rebase this PR? a pin to syslog-message in travis.yml is fine to be merged into this repository (and I'm sure @verbosemode will release syslog-message once verbosemode/syslog-message#18 is sorted) :)

@dra27 dra27 force-pushed the unix-sockets branch 2 times, most recently from 1bbc887 to 65bf80e Compare August 15, 2018 10:04
| Error e -> Lwt.return (Error e)
| Ok () ->
let transmit =
if st = Unix.SOCK_DGRAM then
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(sorry, was away from computer for a week, now slowly catching up on the PRs here -- thanks for them!)
I'm curious about the reasoning behind this conditional. Lwt_unix.send calls send(), which does in either case return the number of transmitted bytes. why is a partial write to a unix domain socket not handled here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is the same thing in the normal unix version too. My (not necessarily correct) understanding is that a partial write would need the same kind of handling as in the UDP reporter (where the return from sendto is also ignored). The Unix domain socket is opened in datagram mode by default (with handling for if you happen to be on an either very old or weird system which does syslog in stream mode on /dev/log). Without framing, you can't write anything further on a partial write, since the other hand has no idea when the packet ends. So my understanding is that because we're connection-oriented, like TCP, means send is required and not sendto but, like UDP, there's not a lot we can do about a partial write.

Since the UDP case doesn't handle a partial write, I didn't put one here - perhaps it should be that both should do some error handling 🙂

.travis.yml Outdated
env:
global:
- PACKAGE="logs-syslog"
- PINS="syslog-message:git+https://github.com/verbosemode/syslog-message.git"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's actually sufficient to have PINS="syslog-message" :)

@hannesm hannesm merged commit 6ced677 into hannesm:master Sep 17, 2018
@hannesm
Copy link
Owner

hannesm commented Sep 17, 2018

thanks and sorry for the delay

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants