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

Reverse the require order so that the VERSION is available to the exporter #1458

Merged
merged 3 commits into from
Jun 1, 2023

Conversation

jmorrell
Copy link

@jmorrell jmorrell commented Jun 1, 2023

Potentially fixes #1457

rake test is failing for me on main

opentelemetry-ruby/exporter/otlp git/main
❯ rake test
/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/google-protobuf-3.23.2-arm64-darwin/lib/google/protobuf/any_pb.rb:15: warning: assigned but unused variable - e
Coverage report generated for Unit Tests to /Users/jmorrell/workspace/opentelemetry-ruby/exporter/otlp/coverage. 161 / 353 LOC (45.61%) covered.
Stopped processing SimpleCov as a previous error not related to SimpleCov has been detected
/Users/jmorrell/workspace/opentelemetry-ruby/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb:38:in `<class:Exporter>': uninitialized constant OpenTelemetry::Exporter::OTLP::VERSION (NameError)

        DEFAULT_USER_AGENT = "OTel-OTLP-Exporter-Ruby/#{OpenTelemetry::Exporter::OTLP::VERSION} Ruby/#{RUBY_VERSION} (#{RUBY_PLATFORM}; #{RUBY_ENGINE}/#{RUBY_ENGINE_VERSION})"
                                                                                     ^^^^^^^^^
	from /Users/jmorrell/workspace/opentelemetry-ruby/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb:24:in `<module:OTLP>'
	from /Users/jmorrell/workspace/opentelemetry-ruby/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb:22:in `<module:Exporter>'
	from /Users/jmorrell/workspace/opentelemetry-ruby/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb:21:in `<module:OpenTelemetry>'
	from /Users/jmorrell/workspace/opentelemetry-ruby/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb:20:in `<top (required)>'
	from <internal:/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:88:in `require'
	from <internal:/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:88:in `require'
	from /Users/jmorrell/workspace/opentelemetry-ruby/exporter/otlp/lib/opentelemetry/exporter/otlp.rb:7:in `<top (required)>'
	from <internal:/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:88:in `require'
	from <internal:/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:88:in `require'
	from /Users/jmorrell/workspace/opentelemetry-ruby/exporter/otlp/test/test_helper.rb:13:in `<top (required)>'
	from <internal:/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:88:in `require'
	from <internal:/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:88:in `require'
	from /Users/jmorrell/workspace/opentelemetry-ruby/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb:6:in `<top (required)>'
	from <internal:/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:88:in `require'
	from <internal:/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:88:in `require'
	from /Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:21:in `block in <main>'
	from /Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6:in `select'
	from /Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6:in `<main>'
rake aborted!
Command failed with status (1)

Tasks: TOP => test
(See full trace by running task with --trace)

After swapping the statements:

❯ rake test
/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/google-protobuf-3.23.2-arm64-darwin/lib/google/protobuf/any_pb.rb:15: warning: assigned but unused variable - e
/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/google-protobuf-3.23.2-arm64-darwin/lib/google/protobuf/wrappers_pb.rb:15: warning: assigned but unused variable - e
/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/google-protobuf-3.23.2-arm64-darwin/lib/google/protobuf/duration_pb.rb:15: warning: assigned but unused variable - e
/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/google-protobuf-3.23.2-arm64-darwin/lib/google/protobuf/field_mask_pb.rb:15: warning: assigned but unused variable - e
/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/google-protobuf-3.23.2-arm64-darwin/lib/google/protobuf/struct_pb.rb:15: warning: assigned but unused variable - e
/Users/jmorrell/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/google-protobuf-3.23.2-arm64-darwin/lib/google/protobuf/timestamp_pb.rb:15: warning: assigned but unused variable - e
Run options: --seed 42950

# Running:

....................S...................

Finished in 0.132498s, 301.8913 runs/s, 913.2213 assertions/s.
40 runs, 121 assertions, 0 failures, 0 errors, 1 skips

You have skipped tests. Run with --verbose for details.
Coverage report generated for Unit Tests to /Users/jmorrell/workspace/opentelemetry-ruby/exporter/otlp/coverage. 330 / 357 LOC (92.44%) covered.

Copy link
Contributor

@robertlaurin robertlaurin left a comment

Choose a reason for hiding this comment

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

This looks like the right fix, thanks for the PR.

@robertlaurin robertlaurin self-assigned this Jun 1, 2023
Copy link
Contributor

@robertlaurin robertlaurin left a comment

Choose a reason for hiding this comment

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

Thanks for the quick report and fix on this one.

@robertlaurin robertlaurin merged commit e03e627 into open-telemetry:main Jun 1, 2023
@jmorrell jmorrell deleted the fix-1457 branch June 1, 2023 21:38
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.

Requiring OTLP exporter breaks as of 0.24.1
2 participants