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

Already on GitHub? Sign in to your account

udp socket bind adjustment. modification to collaboration with glossy. left in tcp4/unix support in connect. #5

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
7 participants

m4tty commented Apr 6, 2012

Fix udp bind to not include the targeted host and port (now it is assigned a random port number), this was failing because syslog is listening on the host/port (the host/port in which we wish to socket.send, but not bind). Fixed message for glossy.produce call which expects a string, using JSON.stringify to avoid [object Object] in syslogMsg and ultimately downstream log.

Fix udp bind to not include the targeted host and port (now it is ass…
…igned a random port number), this was failing because syslog is listening on the host/port (the host/port in which we wish to socket.send, but not bind). Fixed message for glossy.produce call which expects a string, using JSON.stringify to avoid [object Object] in syslogMsg and ultimately downstream log.

I think this is fine for BSD protocol. Considered using structuredData via glossy when type===rfc544, but this puts some burden on the client to understand the implications of RFC5424 and how it relates to the meta object and supplied message object.

you should submit a pull request for this... after you fix the indentation :)
So was readyState removed from socket in 0.6 ?

hehe... I just commented on the commit and now I realize you already did submit a pull request... funny how these comments get pulled into the pull request. I would still fix the indentation :)

m4tty commented Apr 18, 2012

I don't think the readyState property ever existed on dgram socket... udp or unix_dgram (although unix_dgram is no longer supported anyway). That said, the readyState property is on net socket (e.g. tcp4) which winston-syslog uses if tcp protocol is specified for the syslog transport.
The socket api differences between udp and tcp forces some weirdness within winston-syslog. Perhaps the node folks will bring them in line, or winston-syslog implementation can change to better support the differences... but I was trying to keep my changes to a minimum. i'd be tempted to drop support unix_dgram support in winston-syslog, and then have winston-syslog collaborate with a syslog-udp and syslog-tcp log(). anyway...

@m4tty m4tty closed this Apr 18, 2012

@m4tty m4tty reopened this Apr 18, 2012

m4tty commented Apr 18, 2012

Damn. clicked comment and close. might as well reopen... for the time being.

jazzzz commented May 15, 2012

I lost a bit of time trying to figure out why my local syslog server did not receive my logs!

message: msg
instead of message: JSON.stringify(data)
is cleaner.

tlo commented Dec 7, 2012

Hello,
We had trouble with the master version trying to send messages to our local syslog servers. It is working with the code of this PR. Will you ever considerate merging it ? Thanks.

@briktop briktop commented on the diff Oct 1, 2013

lib/winston-syslog.js
@@ -59,11 +64,11 @@ var Syslog = exports.Syslog = function (options) {
//
// Merge the default message options.
//
- this.localhost = options.localhost || 'localhost';
- this.type = options.type || 'BSD';
+ this.localhost = options.localhost || os.hostname();
+ this.type = options.type || 'BSD';//RFC5424';
@briktop

briktop Oct 1, 2013

This comment (//RFC5424';) is confusing, and is probably something you only intended to have here during debugging.

Contributor

julien51 commented Jan 10, 2014

Whoa. That's an old PR. Is that still a thing? Please rebase and send another PR! Thanks

@julien51 julien51 closed this Jan 10, 2014

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