fix: RubyGems Fallback #1161

arielvalentin · 2022-03-25T22:17:49Z

The GitHub Monolith bypasses RubyGems when loading dependencies, which results in a nil variable being returned by Gem.loaded_specs.

This change fallsback to using Gem::Version.new(<VERSION>) where possible.

See #947

Related open-telemetry/opentelemetry-ruby-contrib#373

arielvalentin · 2022-03-29T17:01:05Z

Summarizing our conversation on 2022-03-29 we prefer to rely on gem specific constants over Gem.loaded_specs

instrumentation/base/lib/opentelemetry/instrumentation/base.rb

robbkidd · 2022-03-29T17:38:19Z

instrumentation/delayed_job/lib/opentelemetry/instrumentation/delayed_job/instrumentation.rb

@@ -27,6 +27,8 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base
        private

        def gem_version
+          # Version is hardcoded in the gemspec
+          # https://github.com/collectiveidea/delayed_job/blob/master/delayed_job.gemspec#L16
          Gem.loaded_specs['delayed_job'].version


Did we want to rub some try (&.) on this? Or leave as-is for an exception instead of a nil?

Or maybe …

Suggested change

Gem.loaded_specs['delayed_job'].version

Gem.loaded_specs.fetch('delayed_job').version

… for a different exception? KeyError (key not found: "delayed_job") as opposed to NoMethodError (undefined method 'version' for nil:NilClass)

(I am indeed trying to make fetch happen.)

Or even …

Suggested change

Gem.loaded_specs['delayed_job'].version

Gem

.loaded_specs

.fetch('delayed_job') { raise KeyError.new("'delayed_job' not

found among Gem.loaded_specs") }

.version

… for KeyError ('delayed_job' not found among Gem.loaded_specs).

🤔 Maybe that's too much fetch?

That would trigger the error_handler, is that how we want to control the flow?

https://github.com/open-telemetry/opentelemetry-ruby/blob/main/instrumentation/base/lib/opentelemetry/instrumentation/registry.rb#L84

I think that's how flow is controlled with the existing implementation.

Gem.loaded_specs['delayed_job'].version

If it can't find 'delayed_job' in Gem.loaded_specs, aNoMethodError (undefined method 'version' for nil:NilClass) exception would be thrown (right?) which I think is less clear about what the true error is.

Update: yep.

irb(main):001:0> Gem.loaded_specs['no_such_gem'].version Traceback (most recent call last): … blah blah blah NoMethodError (undefined method `version' for nil:NilClass) irb(main):002:0> Gem.loaded_specs.fetch('no_such_gem'){raise KeyError.new("'no_such_gem' not found among Gem.loaded_specs")}.version Traceback (most recent call last): … blah blah blah KeyError ('no_such_gem' not found among Gem.loaded_specs)

arielvalentin · 2022-03-30T13:20:18Z

Got a failing test for AWS v2.11 after removing mocks/stubs in my last commit.

@NathanielRN does the SDK team have a Timeline for EoL since v2 is deprecated?

arielvalentin · 2022-03-30T13:31:54Z

NVM @NathanielRN I read STDERR

Version 2 of the Ruby SDK will enter maintenance mode as of November 20, 2020. To continue receiving service updates and new features, please upgrade to Version 3. More information can be found here: https://aws.amazon.com/blogs/developer/deprecation-schedule-for-aws-sdk-for-ruby-v2/

arielvalentin · 2022-03-30T19:59:44Z

@YanivD I am going to drop support for AWS SDK 2.0 since it is no longer under maintenance since 2021-11. I will open a separate PR.

Please let me know if you have any concerns.

arielvalentin · 2022-03-30T21:41:02Z

cc: #1166

mwear

LGTM

The GitHub Monolith bypasses RubyGems when loading dependencies, which results in a `nil` variable being returned by `Gem.loaded_specs`. This change fallsback to using `Gem::Version.new(<VERSION>)` where possible. See open-telemetry#947 Related https://github.com/open-telemetry/opentelemetry-ruby/issues/988#issuecomment-1018676853

Prior to this commit developers had to either run tests from within a container, or independently run services from their host machine. Developers may now use docker-compose for all services and run tests from their host machine.

arielvalentin · 2022-04-02T14:31:27Z

Two unexpected errors in the test suites suite

Ruby 2.5 https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true

The Gemfile's dependencies are satisfied
[4972](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4972)
opentelemetry-instrumentation-rdkafka: test
[4973](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4973)
Your RubyGems version (2.7.6.3) has a bug that prevents `required_ruby_version` from working for Bundler. Any scripts that use `gem install bundler` will break as soon as Bundler drops support for your Ruby version. Please upgrade RubyGems to avoid future breakage and silence this warning by running `gem update --system 3.2.3`
[4974](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4974)
>> BUNDLE_GEMFILE=/home/runner/work/opentelemetry-ruby/opentelemetry-ruby/instrumentation/rdkafka/gemfiles/rdkafka_0.10.0.gemfile bundle exec rake test
[4975](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4975)
Your RubyGems version (2.7.6.3) has a bug that prevents `required_ruby_version` from working for Bundler. Any scripts that use `gem install bundler` will break as soon as Bundler drops support for your Ruby version. Please upgrade RubyGems to avoid future breakage and silence this warning by running `gem update --system 3.2.3`
[4976](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4976)
Run options: --seed 8271
[4977](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4977)

[4978](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4978)
# Running:
[4979](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4979)

[4980](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4980)
......
[4981](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4981)

[4982](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4982)
Finished in 11.175682s, 0.5369 runs/s, 3.1318 assertions/s.
[4983](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4983)

[4984](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4984)
6 runs, 35 assertions, 0 failures, 0 errors, 0 skips
[4985](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4985)
Segmentation fault (core dumped)
[4986](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4986)
rake aborted!
[4987](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4987)
Command failed with status (139)
[4988](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4988)
/opt/hostedtoolcache/Ruby/2.5.9/x64/bin/bundle:23:in `load'
[4989](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4989)
/opt/hostedtoolcache/Ruby/2.5.9/x64/bin/bundle:23:in `<main>'
[4990](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4990)
Tasks: TOP => test
[4991](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998961?check_suite_focus=true#step:6:4991)
(See full trace by running task with --trace)

Ruby 2.6 https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true

# Running:
[994](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:994)

[995](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:995)
.......................................F..
[996](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:996)

[997](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:997)
Finished in 0.044692s, 939.7698 runs/s, 2080.9188 assertions/s.
[998](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:998)

[999](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:999)
  1) Failure:
[1000](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1000)
OpenTelemetry::Instrumentation::ActiveJob::Patches::ActiveJobCallbacks::attributes::messaging.active_job.executions#test_0002_tracks correctly for jobs that do retry [/home/runner/work/opentelemetry-ruby/opentelemetry-ruby/instrumentation/active_job/test/instrumentation/active_job/patches/active_job_callbacks_test.rb:218]:
[1001](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1001)
Expected: 2
[1002](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1002)
  Actual: 1
[1003](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1003)

[1004](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1004)
42 runs, 93 assertions, 1 failures, 0 errors, 0 skips
[1005](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1005)
rake aborted!
[1006](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1006)
Command failed with status (1)
[1007](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1007)
/opt/hostedtoolcache/Ruby/2.6.9/x64/bin/bundle:23:in `load'
[1008](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1008)
/opt/hostedtoolcache/Ruby/2.6.9/x64/bin/bundle:23:in `<main>'
[1009](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1009)
Tasks: TOP => test
[1010](https://github.com/open-telemetry/opentelemetry-ruby/runs/5799998978?check_suite_focus=true#step:6:1010)
(See full trace by running task with --trace)

arielvalentin · 2022-04-08T17:59:20Z

@open-telemetry/ruby-maintainers thoughts?

ericmustin · 2022-04-08T18:09:23Z

@arielvalentin the tests appear to have magically fixed themselves, fwiw.

arielvalentin self-assigned this Mar 25, 2022

arielvalentin changed the title ~~fix: Bypass RubyGems loaded_specs~~ fix: RubyGems Fallback Mar 28, 2022

arielvalentin force-pushed the use-gem-versions branch 3 times, most recently from 5cb7fc6 to 0ed52f7 Compare March 28, 2022 14:46

arielvalentin marked this pull request as ready for review March 28, 2022 15:55

arielvalentin requested review from fbogsany, mwear, robertlaurin, dazuma, ericmustin and ahayworth as code owners March 28, 2022 15:55

arielvalentin marked this pull request as draft March 29, 2022 16:55

robbkidd reviewed Mar 29, 2022

View reviewed changes

instrumentation/base/lib/opentelemetry/instrumentation/base.rb Show resolved Hide resolved

robbkidd reviewed Mar 29, 2022

View reviewed changes

arielvalentin force-pushed the use-gem-versions branch from 0fbb507 to 28c7cbb Compare March 30, 2022 00:30

arielvalentin marked this pull request as ready for review March 30, 2022 00:36

arielvalentin requested a review from robbkidd March 30, 2022 00:36

arielvalentin marked this pull request as draft March 30, 2022 13:18

arielvalentin mentioned this pull request Mar 31, 2022

chore: AWS SDK min version 3.0 #1166

Closed

arielvalentin force-pushed the use-gem-versions branch 2 times, most recently from 258198d to 689d7db Compare April 1, 2022 15:13

arielvalentin marked this pull request as ready for review April 1, 2022 16:28

mwear approved these changes Apr 1, 2022

View reviewed changes

chore: Allow test to run from host machine

4fb6b66

Prior to this commit developers had to either run tests from within a container, or independently run services from their host machine. Developers may now use docker-compose for all services and run tests from their host machine.

arielvalentin force-pushed the use-gem-versions branch from d31b2ed to 4fb6b66 Compare April 2, 2022 14:00

ericmustin approved these changes Apr 8, 2022

View reviewed changes

robertlaurin approved these changes Apr 12, 2022

View reviewed changes

Merge branch 'main' into use-gem-versions

fa52b20

robertlaurin merged commit 3b03ff7 into open-telemetry:main Apr 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: RubyGems Fallback #1161

fix: RubyGems Fallback #1161

arielvalentin commented Mar 25, 2022 •

edited

Loading

arielvalentin commented Mar 29, 2022

robbkidd Mar 29, 2022 •

edited

Loading

robbkidd Mar 29, 2022

arielvalentin Mar 29, 2022

robbkidd Mar 30, 2022 •

edited

Loading

arielvalentin commented Mar 30, 2022

arielvalentin commented Mar 30, 2022

arielvalentin commented Mar 30, 2022

arielvalentin commented Mar 30, 2022

mwear left a comment

arielvalentin commented Apr 2, 2022

arielvalentin commented Apr 8, 2022

ericmustin commented Apr 8, 2022

	Gem.loaded_specs['delayed_job'].version
	Gem.loaded_specs.fetch('delayed_job').version

fix: RubyGems Fallback #1161

fix: RubyGems Fallback #1161

Conversation

arielvalentin commented Mar 25, 2022 • edited Loading

arielvalentin commented Mar 29, 2022

robbkidd Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

robbkidd Mar 29, 2022

Choose a reason for hiding this comment

arielvalentin Mar 29, 2022

Choose a reason for hiding this comment

robbkidd Mar 30, 2022 • edited Loading

Choose a reason for hiding this comment

arielvalentin commented Mar 30, 2022

arielvalentin commented Mar 30, 2022

arielvalentin commented Mar 30, 2022

arielvalentin commented Mar 30, 2022

mwear left a comment

Choose a reason for hiding this comment

arielvalentin commented Apr 2, 2022

arielvalentin commented Apr 8, 2022

ericmustin commented Apr 8, 2022

arielvalentin commented Mar 25, 2022 •

edited

Loading

robbkidd Mar 29, 2022 •

edited

Loading

robbkidd Mar 30, 2022 •

edited

Loading