-
-
Notifications
You must be signed in to change notification settings - Fork 112
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
Add Logentries handler. #33
base: master
Are you sure you want to change the base?
Conversation
Hey, this would be useful to me. Is there any chance of this being upstreamed soon? cc: @tj |
hmm weird I thought I commented on this one, added |
LGTM other than the potentially superfluous Close |
@tj @johnwchadwick - Just rebased - a review would be great. I imagine in long running situations the |
@johnwchadwick care to check it out? I don't use logentries |
Oh, I think the current code is probably sufficient. The le_go library does ensure the TCP socket is open and will attempt to reconnect if not, and if that fails it will return an error. That being said, I definitely think the |
yep, cool. I'll tweak a few things and get that in tomorrow. We're missing comments and the style is a bit different than the others (they all use |
This handler will report to LogEntries with a [TCP token](https://github.com/bsphere/le_go#usage).
return nil, err | ||
} | ||
|
||
return &LogHandler{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't 100% sure here if I could nix the previous guard and return &LogHandler{logger: logger}, err
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's best to return nil on error. (in other words, I think the guard is good.)
Please add comments to your content! |
Hmm. I'm running some tests with this in a close-to-production environment and I actually see some issues with the reliability. It seemed to arbitrarily disconnect silently and not reconnect. It's kind of strange given that Well, YMMV. I may try to roll a version that doesn't have external dependencies for my own usage, since the LE protocol is pretty braindead-simple anyways. |
And, now I've done that. My LogEntries handler is available here. Of course, there's no guarantee that my implementation of the LogEntries protocol is better than |
This handler will report to LogEntries with a TCP token.