Skip to content

Commit

Permalink
fix: Drop Rails dependency for ActiveSupport Instrumentation
Browse files Browse the repository at this point in the history
This makes it possible to use this instrumentation in non-Rails applications that leverage AS::Notifications.

In addition to that it mitigates a race condition where a subscription may be registered by a separate thread before obtaining a lock on the notifications object.

See open-telemetry#218
  • Loading branch information
arielvalentin committed Jan 14, 2023
1 parent 5aabcf2 commit ca76448
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 19 deletions.
19 changes: 6 additions & 13 deletions instrumentation/active_support/Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,16 @@
#
# SPDX-License-Identifier: Apache-2.0

# Rails 5.2 is incompatible with Ruby 3.
if RUBY_VERSION < '3'
appraise 'rails-5.2' do
gem 'rails', '~> 5.2.0'
end
end

appraise 'rails-6.0' do
gem 'rails', '~> 6.0.0'
appraise 'activesupport-6.0' do
gem 'activesupport', '~> 6.0.0'
end

appraise 'rails-6.1' do
gem 'rails', '~> 6.1.0'
appraise 'activesupport-6.1' do
gem 'activesupport', '~> 6.1.0'
end

if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.7.0')
appraise 'rails-7.0' do
gem 'rails', '~> 7.0.0'
appraise 'activesupport-7.0' do
gem 'activesupport', '~> 7.0.0'
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@ module OpenTelemetry
module Instrumentation
# rubocop:disable Style/Documentation
module ActiveSupport
NOTIFIER_MAJOR_VERSION_6 = 6

# The SpanSubscriber is a special ActiveSupport::Notification subscription
# handler which turns notifications into generic spans, taking care to handle
# context appropriately.

# A very hacky way to make sure that OpenTelemetry::Instrumentation::ActiveSupport::SpanSubscriber
# gets invoked first
# Rails 6+ https://github.com/rails/rails/blob/0f0ec9908e25af36df2d937dc431f626a4102b3d/activesupport/lib/active_support/notifications/fanout.rb#L51
#
def self.subscribe(
tracer,
Expand All @@ -29,13 +32,25 @@ def self.subscribe(
)

subscriber_object = ::ActiveSupport::Notifications.subscribe(pattern, subscriber)
active_support_major_version = ::ActiveSupport.version.canonical_segments.first

::ActiveSupport::Notifications.notifier.synchronize do
if ::Rails::VERSION::MAJOR >= 6
s = ::ActiveSupport::Notifications.notifier.instance_variable_get(:@string_subscribers)[pattern].pop
::ActiveSupport::Notifications.notifier.instance_variable_get(:@string_subscribers)[pattern].unshift(s)
if active_support_major_version >= NOTIFIER_MAJOR_VERSION_6
subscribers = ::ActiveSupport::Notifications.notifier.instance_variable_get(:@string_subscribers)[pattern]
else
subscribers = ::ActiveSupport::Notifications.notifier.instance_variable_get(:@subscribers)
end

if subscribers.nil?
OpenTelemetry.handle_error(
message: "Unable to move OTEL ActiveSupport Notifications subscriber to the front of the notifications list which may cause incomplete traces." +
"Please report an issue here: " +
"https://github.com/open-telemetry/opentelemetry-ruby-contrib/issues/new?labels=bug&template=bug_report.md&title=ActiveSupport%20Notifications%20subscribers%20list%20is%20nil"
)
else
s = ::ActiveSupport::Notifications.notifier.instance_variable_get(:@subscribers).pop
::ActiveSupport::Notifications.notifier.instance_variable_get(:@subscribers).unshift(s)
subscribers.unshift(
subscribers.delete(subscriber_object)
)
end
end
subscriber_object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Gem::Specification.new do |spec|
spec.add_dependency 'opentelemetry-api', '~> 1.0'
spec.add_dependency 'opentelemetry-instrumentation-base', '~> 0.21.0'

spec.add_development_dependency 'activesupport'
spec.add_development_dependency 'appraisal', '~> 2.2.0'
spec.add_development_dependency 'bundler', '~> 2.4'
spec.add_development_dependency 'minitest', '~> 5.0'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

require 'test_helper'

describe OpenTelemetry::Instrumentation::ActiveSupport::SpanSubscriber do
describe 'OpenTelemetry::Instrumentation::ActiveSupport::SpanSubscriber' do
let(:instrumentation) { OpenTelemetry::Instrumentation::ActiveSupport::Instrumentation.instance }
let(:tracer) { instrumentation.tracer }
let(:exporter) { EXPORTER }
Expand Down
3 changes: 3 additions & 0 deletions instrumentation/active_support/test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
require 'bundler/setup'
Bundler.require(:default, :development, :test)

require 'active_support'
require 'active_support/core_ext'
require 'opentelemetry-instrumentation-active_support'

require 'minitest/autorun'
require 'webmock/minitest'


# global opentelemetry-sdk setup:
EXPORTER = OpenTelemetry::SDK::Trace::Export::InMemorySpanExporter.new
span_processor = OpenTelemetry::SDK::Trace::Export::SimpleSpanProcessor.new(EXPORTER)
Expand Down

0 comments on commit ca76448

Please sign in to comment.