-
Notifications
You must be signed in to change notification settings - Fork 77
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
Support for AWS Signed Requests #64
Comments
Hey @tmonk42 I think it would be useful to have AWS transport support across all the checks. Best if it could be added in a backwards compatible way. |
Cool, backwards compatibility will certainly be the trick here. Any pointers on how to test/validate that? |
You can start up a local elasticsearch instance and point the checks at it. |
Word, had a short hiccup with getting ES5 in Docker, but I'm setup now to do testing against ES 0.90.latest, 1.7.6, 2.4.3, and 5.1.1. I'll also be testing against ES 1.5 and 2.3 in AWS to validate the AWS transport/signed requests option. |
Cool. You can skip 0.90 - we don't support it anymore. |
Cool. I believe we're going to need updated aws-es-transport in order for my refactor to work, see #65. I just ran through testing my rewrite of check-es-heap.rb and it looks solid. See WIP here: https://github.com/tmonk42/sensu-plugins-elasticsearch/commits/more_aws_transport ./test.rb DEBUG es_ver: ["1.7.6"] DEBUG es_ver: ["2.4.3"] DEBUG es_ver: ["5.1.1"] DEBUG es_ver: ["1.5.2"] DEBUG es_ver: ["2.3.2"] |
I've got this working for about a half dozen plugins we're actively using, however, it only works with my forked copy of aws-es-transport. Until this PR is merged and a new version of that gem is released, I'm afraid it will provide a very bad user experience. |
@eheydrick So the PR I have against aws-es-transport has been sitting there idle since mid-January. When I checked on rubygems.org, the only dependancy on that gem is sensu-plugins-elasticsearch: https://rubygems.org/gems/aws-es-transport/reverse_dependencies I'm wondering if maybe we ought to pull that code into this project directly so we can get these updates rolling... |
Or fork the project and release a new gem. |
Looks like aws-es-transport got my PR merged, now we need a version bump and publish to rubygems for that project, then it should be safe to add my work to this project :) |
@tmonk42 if at all possible I would like to wait as I have another pr cdunn/aws-es-transport#5 open against that as its dependencies are super restrictive and either they need to be loosened or we are gonna have to remove support for it all together. |
Due to lack of responsiveness I am setting a deadline for this. By Feb, 01, 2018. If we have not had those dependencies met by then either by merging or forking, renaming, and publishing another gem we will need to remove any code that depends on that gem. |
Ops life just brought me back to this gem and issue. 🤣 |
I'm working on setting up monitoring of AWS ElasticSearch, and for a variety of reasons, it would be great if I could use AWS Signed Requests in many of the ES plugins. Support for this already exists in some of the plugins in this repo. For examples of checks that work, see check-es-cluster-health.rb, check-es-query-count.rb for examples that support "--transport AWS"). I'd love to be able to this transport for several other checks, ie: check-es-heap.rb, check-es-circuit-breakers.rb.
This involves a pretty major rewrite of the plugin, and I ran through it as proof of concept on check-es-heap.rb. I'm curious if work along these lines would be accepted in this repo, or if I should target this as eventually submitting a PR to the sensu-plugins-aws repo instead.
Cheers!
The text was updated successfully, but these errors were encountered: