search

hmalphettes / log4js-elasticsearch

log4js appender for node that targets elasticsearch. Viewable with kibana.
21 stars 7 forks source link

support for AWS elasticsearch service #6

Closed issacg closed 8 years ago

issacg commented 8 years ago

Hey,

I wanted to say I love the module! My use-case is to use AWS's managed ElasticSearch service, which requires signing POST requests. I didn't think that lo4js-elasticsearch should necessarily support AWS out-of-the-box (it's a bit out of scope, and includes dependencies that many users may not need/want) so I wrote log4js-elasticsearch-aws which monkey-patches execRequest.

I'd appreciate a second pair of eyes to look - if there's something that you plan on changing here, I'd appreciate knowing so that I can ensure that the modules play nicely together.

Thanks again for writing this!

hmalphettes commented 8 years ago

Thanks for the compliments! I am glad it is useful. I am interested in the AWS support and I concur with you that it is a good thing to develop it else where: thanks for doing it. Feel free to send us the link or submit a PR to update this README.md and point at what you are doing.

hmalphettes commented 8 years ago

@issacg Feel free to modify the readme further.

I had a very quick look at your extension and I think it does a fine job.

It really no big deal for me but maybe we could both adopt the same license?

If number of dependencies was a big deal we could implement it solely based on a module that only purpose is to support aws signing version 4. There are a handful of them and they have no dependencies: https://www.npmjs.com/package/aws-sign2 https://www.npmjs.com/package/aws4

issacg commented 8 years ago

Hey @hmalphettes sorry for the delay.

If changing the license is super-important for you, I'll consider it; as an oldish-school member of the ASF community, I tend to stick to the Apache-2 license unless I have a good reason not to.

Regarding the dependencies, I don't have issues with requiring aws-sdk's massive dependency chain (certainly not in a package explicitly aimed at AWS users); if someone is using AWS ElasticSearch, chances are good that they're using the big aws-sdk for other stuff anyway.

Thanks for the doc-patch!

FYI, I'm investigating another issue with SIGINT on windows platform not flushing properly - I'll open another issue when I have concrete understanding of what's not working.

Thanks again for the great work!

hmalphettes commented 8 years ago

@issacg understood for the Apache License. After all that is the license used by Elasticsearch too. Thanks to you!

issacg commented 8 years ago

FYI My issue with the shutdown handlers was an uncaught exception, but while digging, I came across #7.