sensu-plugins / sensu-plugins-elasticsearch

This plugin provides native ElasticSearch instrumentation for monitoring and metrics collection, including: service health and metrics for cluster, node, and more.
http://sensu-plugins.io
MIT License
32 stars 77 forks source link

es conflict with check-es-query-count.rb #48

Open kaushiksriram100 opened 8 years ago

kaushiksriram100 commented 8 years ago

When I attempt to run the check-es-query-count.rb I end up getting this error:

[root@server tmp]# /opt/sensu/embedded/bin/ruby /opt/sensu/embedded/lib/ruby/gems/2.2.0/gems/sensu-plugins-elasticsearch-1.0.0/bin/check-es-query-count.rb -h Usage: /opt/sensu/embedded/lib/ruby/gems/2.2.0/gems/sensu-plugins-elasticsearch-1.0.0/bin/check-es-query-count.rb (options) /opt/sensu/embedded/lib/ruby/2.2.0/rubygems/specification.rb:2112:in raise_if_conflicts': Unable to activate sensu-plugins-elasticsearch-1.0.0, because elasticsearch-1.1.0 conflicts with elasticsearch (~> 1.0.14) (Gem::ConflictError) from /opt/sensu/embedded/lib/ruby/2.2.0/rubygems/specification.rb:1280:inactivate' from /opt/sensu/embedded/lib/ruby/2.2.0/rubygems.rb:198:in rescue in try_activate' from /opt/sensu/embedded/lib/ruby/2.2.0/rubygems.rb:195:intry_activate' from /opt/sensu/embedded/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb:126:in rescue in require' from /opt/sensu/embedded/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb:39:inrequire' from /opt/sensu/embedded/lib/ruby/gems/2.2.0/gems/sensu-plugins-elasticsearch-1.0.0/bin/check-es-query-count.rb:40:in `

'

majormoses commented 7 years ago

we have pinned it here: https://github.com/sensu-plugins/sensu-plugins-elasticsearch/blob/master/sensu-plugins-elasticsearch.gemspec#L37 we can either bump it there or you can remove that version. Could you put together a pr to bump the version?

majormoses commented 7 years ago

@eheydrick our dependency on 1.x is quite old, I am tempted to say we should try to bump it all the way to 5.x if possible if not we should go to at least 2.x

eheydrick commented 7 years ago

As long as it works on all the versions we support - 1.x, 2.x, and 5.x, I'm cool with moving it to a newer version.

majormoses commented 7 years ago

@eheydrick Understood and hope we can maintain support for all 3, however we might want to drop 1.x if it causes issues as its now technically EOL.

https://www.elastic.co/support/eol

Elasticsearch   EOL Date    Maintained Until
1.7.x           2017-01-16  5.0.0
2.4.x           2018-02-28  6.0.0
5.4.x           2019-11-04  5.5.0
majormoses commented 7 years ago

@kaushiksriram100 would you be willing to try bumping the dependencies and testing?

kaushiksriram100 commented 7 years ago

@majormoses : Sure I can try, am a beginner to ruby gems so would need some pointers.

What do you think about this dependency for aws-es-transport? Looks like that is dependent on elasticsearch >= 1.0.10, ~> 1.0 https://rubygems.org/gems/aws-es-transport

I am getting this error after bumping up es gem version in the gemspec (to s.add_runtime_dependency 'elasticsearch', ['>= 2.0.0', '~> 2.0.0'])

L-SNVJ0NCFFT-M:sensu-plugins-elasticsearch skaush1$ gem install ./sensu-plugins-elasticsearch-1.3.1.gem ERROR: While executing gem ... (Gem::DependencyResolutionError) conflicting dependencies elasticsearch (>= 1.0.10, ~> 1.0) and elasticsearch (>= 2.0.0, ~> 2.0.0) Activated elasticsearch-2.0.0 which does not match conflicting dependency (>= 1.0.10, ~> 1.0)

Conflicting dependency chains: sensu-plugins-elasticsearch (= 1.3.1), 1.3.1 activated, depends on elasticsearch (>= 2.0.0, ~> 2.0.0), 2.0.0 activated

versus: sensu-plugins-elasticsearch (= 1.3.1), 1.3.1 activated, depends on aws-es-transport (~> 0.1), 0.1.1 activated, depends on elasticsearch (>= 1.0.10, ~> 1.0)

L-SNVJ0NCFFT-M:sensu-plugins-elasticsearch skaush1$

majormoses commented 7 years ago

More than happy to help.

s.add_runtime_dependency 'elasticsearch', ['>= 2.0.0', '~> 2.0.0'] means greater than or equal to 2 and grater than 2 but less than 3 so that is essentially the same as ~> 2 notice the lack of digits after the major. In our case to support all three we would need to open it up to '>= 1.0.10' to satisfy the constraints of any ES 1.x, 2.x, and 5.x as well as match the version constraint of aws-es-transport. This may baloon fast and we might need to make backwards incompatible changes but lets see if this works for you.

kaushiksriram100 commented 7 years ago

@majormoses got it. Is this reasonable? https://github.com/kaushiksriram100/sensu-plugins-elasticsearch/blob/master/sensu-plugins-elasticsearch.gemspec#L37

However, I get a warning when I build the gem because there is no upper bound version -

L-SNVJ0NCFFT-M:sensu-plugins-elasticsearch skaush1$ gem build sensu-plugins-elasticsearch.gemspec WARNING: open-ended dependency on elasticsearch (>= 1.0.10) is not recommended if elasticsearch is semantically versioned, use: add_runtime_dependency 'elasticsearch', '~> 1.0', '>= 1.0.10' WARNING: See http://guides.rubygems.org/specification-reference/ for help Successfully built RubyGem Name: sensu-plugins-elasticsearch Version: 1.3.1 File: sensu-plugins-elasticsearch-1.3.1.gem

Another question:

s.add_runtime_dependency 'aws-sdk', ['>= 2.1.14', '< 2.5', '~> 2.1']

Why do we need that '~> 2.1' ? The previous two conditions anyway restrict gem between 2.1.14 and 2.5. Just checking

majormoses commented 7 years ago

I dont think we do... @eheydrick am I missing something?

majormoses commented 7 years ago

I am guessing that when < 2.5 was added no one removed the ~> . 2.1

kaushiksriram100 commented 7 years ago

@majormoses : one last question for this thread: What determines which version of ES should get installed. For example when I run the gem I see

Fetching: elasticsearch-1.1.2.gem (100%) Successfully installed elasticsearch-1.1.2

What determines that es-1.1.2 (this specific version) has to be installed?

majormoses commented 7 years ago

I need to honestly play with this as I am not used to ever trying to support multiple major versions of a gem and usually support 1 major at a time. My guess prior to seeing your output would be that it will install the highest that satisfies constraints but that does not appear to be the case.

majormoses commented 7 years ago

I will try to get to playing with this tonight.

eheydrick commented 7 years ago

@majormoses, the aws-sdk '~> 2.1' bit looks extraneous and can be removed.

majormoses commented 7 years ago

@eheydrick that would make this break all the checks that rely on the aws-transport gem which I do think we need to do something about anyways as it's old and does not seem to be maintained.

https://github.com/sensu-plugins/sensu-plugins-elasticsearch/blob/e8779562e9bdc4709d27c0b7070fa849501e2bc7/CHANGELOG.md#L40

Here are the related issues:

eheydrick commented 7 years ago

@kaushiksriram100 I see you're executing the check from the gems directory. What happens if you execute it using the binstub e.g. /opt/sensu/embedded/bin/check-es-query-count.rb? That should select the correct dependency. Can you paste the output of /opt/sensu/embedded/bin/gem list |grep elasticsearch. Wondering if there's something up with the gems installed in the embedded ruby.

eheydrick commented 7 years ago

@majormoses the aws transport requirement is just < 2.5 from what I can see. ['>= 2.1.14', '< 2.5'] should cover that.

majormoses commented 7 years ago

yes that was what I was saying here: https://github.com/sensu-plugins/sensu-plugins-elasticsearch/issues/48#issuecomment-300576773 My following comment when you said drop the it I somehow did not connect the dots that you only meant that version.