Skip to content

Conversation

@xuan-cao-swi
Copy link
Contributor

Description

metrics-sdk test case fails due to a race condition in the meter_provider initialization, which causes the runs the after_hook after forking test to fail.

  1) Error:
OpenTelemetry::SDK::Metrics::ForkHooks#test_0001_runs the after_hook after forking:
NoMethodError: undefined method 'metric_readers' for an instance of OpenTelemetry::Internal::ProxyMeterProvider
    lib/opentelemetry/sdk/metrics/fork_hooks.rb:21:in 'OpenTelemetry::SDK::Metrics::ForkHooks.after_fork'
    lib/opentelemetry/sdk/metrics/fork_hooks.rb:29:in 'block in OpenTelemetry::SDK::Metrics::ForkHooks#_fork'
    <internal:kernel>:91:in 'Kernel#tap'
    lib/opentelemetry/sdk/metrics/fork_hooks.rb:28:in 'OpenTelemetry::SDK::Metrics::ForkHooks#_fork'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:15:in 'Kernel#fork'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:15:in 'block in fork_with_fork_hooks'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:45:in 'with_pipe'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:14:in 'fork_with_fork_hooks'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:58:in 'block (3 levels) in <top (required)>'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:45:in 'with_pipe'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:52:in 'block (2 levels) in <top (required)>'

207 runs, 16653 assertions, 0 failures, 1 errors, 0 skips
........

Finished in 41.416792s, 4.9980 runs/s, 402.0833 assertions/s.

  1) Error:
OpenTelemetry::SDK::Metrics::ForkHooks#test_0001_runs the after_hook after forking:
Timeout::Error: execution expired
    /opt/hostedtoolcache/Ruby/3.4.1/x64/lib/ruby/3.4.0/timeout.rb:40:in 'Timeout::Error.handle_timeout'
    /opt/hostedtoolcache/Ruby/3.4.1/x64/lib/ruby/3.4.0/timeout.rb:194:in 'Timeout.timeout'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:26:in 'block in fork_with_fork_hooks'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:45:in 'with_pipe'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:14:in 'fork_with_fork_hooks'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:58:in 'block (3 levels) in <top (required)>'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:45:in 'with_pipe'
    test/opentelemetry/sdk/metrics/fork_hooks_test.rb:52:in 'block (2 levels) in <top (required)>'

207 runs, 16653 assertions, 0 failures, 1 errors, 0 skips

This fix aims to stub the meter_provider with an initialized one. However, we may need to consider improving the method self.after_fork to make it more robust.

def self.after_fork
  meter_provider = ::OpenTelemetry.meter_provider
  return unless meter_provider.respond_to?(:metric_readers)
  
  meter_provider.metric_readers.each do |reader|
     reader.after_fork if reader.respond_to?(:after_fork)
  end
end

Copy link
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

Thank you! 🎉

@xuan-cao-swi
Copy link
Contributor Author

@chrisholmes

I just want to make sure this doesn't break the original intention of test case (e.g. if avoid race condition should be tested in this test case)

Copy link
Contributor

@chrisholmes chrisholmes left a comment

Choose a reason for hiding this comment

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

@xuan-cao-swi I think we're good here. The tests aim is to demonstrate our ability to apply an after_fork hook rather than the behaviour of what gets called when we do.

@kaylareopelle kaylareopelle merged commit 65cff52 into open-telemetry:main Oct 31, 2025
65 checks passed
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.

3 participants