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

Add simple Logstash adapter #64

Closed
wants to merge 6 commits into from
Closed

Add simple Logstash adapter #64

wants to merge 6 commits into from

Conversation

maxekman
Copy link
Contributor

This adds a simpler version of the Logstash UDP adapter created in github.com/gliderlabs/logspout.

@maxekman maxekman changed the title Add simple Logstash module Add simple Logstash adapter Mar 26, 2015
func init() {
router.AdapterFactories.Register(NewLogstashAdapter, "logstash")

hostname, _ = os.Hostname()
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, it's a global. Maybe set this up as a field of the adapter object and do this in its constructor.

@maxekman
Copy link
Contributor Author

@progrium Updated with more Docker fields, similar to the fields in https://github.com/digital-wonderland/docker-logstash-forwarder.

@progrium
Copy link
Contributor

Why do you think JSON+UDP should be the canonical way logspout should send logs to logstash?

@maxekman
Copy link
Contributor Author

I think UPD should be the default because it's faster for streaming logs, and there can be a lot from some docker containers. For more info: https://github.com/dwbutler/logstash-logger#what-type-of-logger-should-i-use

@progrium
Copy link
Contributor

Maybe it should just be called a JSON adapter then?

@progrium
Copy link
Contributor

Does this satisfy your use?
#65

@maxekman
Copy link
Contributor Author

I think your JSON adapter is a pretty cool implementation, but I would like to have both! My implementation is focused on performance as this is more important in our use case.

@progrium
Copy link
Contributor

I recommend putting this adapter in its own repo and building logspout
against it. Others can use it if they want but you can continue to maintain
it.

On Friday, March 27, 2015, Max Persson notifications@github.com wrote:

I think your JSON adapter is a pretty cool implementation, but I would
like to have both! My implementation is focused on performance as this is
more important in our use case.


Reply to this email directly or view it on GitHub
#64 (comment).

Jeff Lindsay
http://progrium.com

@maxekman
Copy link
Contributor Author

Sure, are there any guidelines on how to do that?

@progrium
Copy link
Contributor

The repo name should be prefixed with logspout- and have a descriptive name, so I imagine this would be logspout-logstash-json on your github account. The go package can still be called logstash or anything, it's not relevant. You can put the package anywhere in there, but I recommend at the root. This way, you can just add a line to modules.go imports:

    _ "github.com/maxpersson/logspout-logstash-json"

Depending on how often you might change it and to what degree, I recommend versioning/tagging with gopkg.in conventions. This way people can use that for pinning if they wish.

@maxekman
Copy link
Contributor Author

Will this be added to gliderlabs/logspout/modules.go or to my own fork?

@progrium
Copy link
Contributor

Anybody's fork. We'll list it as a third-party module in the readme.

@maxekman
Copy link
Contributor Author

What I meant was when using logspout; will I be able to run your container with my module referenced in it, or would I create and push my own container from my own fork using my adapter?

@progrium
Copy link
Contributor

For now you need your own container fork. Eventually you can dynamically load modules at container boot.

@maxekman
Copy link
Contributor Author

Ok, thanks for the info.

@progrium
Copy link
Contributor

This might make it easier for you: https://github.com/gliderlabs/logspout/pull/66/files

@maxekman
Copy link
Contributor Author

I created a custom module instead, available here https://github.com/looplab/logspout-logstash. Closing this PR.

@maxekman maxekman closed this Mar 31, 2015
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