From 5f7808d6903f42ba1dbf323471e1b724a0b789c4 Mon Sep 17 00:00:00 2001 From: Katherine Oelsner <49968061+octokatherine@users.noreply.github.com> Date: Wed, 25 Oct 2023 21:22:59 +0000 Subject: [PATCH 1/5] wrap call to depcrecated private API behind version conditional --- .../active_support/span_subscriber.rb | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb b/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb index 72e8f04a9..f241e6673 100644 --- a/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb +++ b/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb @@ -30,19 +30,21 @@ def self.subscribe( subscriber_object = ::ActiveSupport::Notifications.subscribe(pattern, subscriber) - ::ActiveSupport::Notifications.notifier.synchronize do - subscribers = ::ActiveSupport::Notifications.notifier.instance_variable_get(:@string_subscribers)[pattern] - - 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 - subscribers.unshift( - subscribers.delete(subscriber_object) - ) + if Rails.version < Gem::Version.new('7.2') + ::ActiveSupport::Notifications.notifier.synchronize do + subscribers = ::ActiveSupport::Notifications.notifier.instance_variable_get(:@string_subscribers)[pattern] + + 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 + subscribers.unshift( + subscribers.delete(subscriber_object) + ) + end end end subscriber_object From 089d112b0d2dc3e6956674464859d6d1ed27aa34 Mon Sep 17 00:00:00 2001 From: Katherine Oelsner <49968061+octokatherine@users.noreply.github.com> Date: Wed, 25 Oct 2023 21:42:03 +0000 Subject: [PATCH 2/5] convert Rails version string to Gem::Version --- .../instrumentation/active_support/span_subscriber.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb b/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb index f241e6673..8d3cbfc0f 100644 --- a/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb +++ b/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb @@ -30,7 +30,7 @@ def self.subscribe( subscriber_object = ::ActiveSupport::Notifications.subscribe(pattern, subscriber) - if Rails.version < Gem::Version.new('7.2') + if Gem::Version.new(Rails.version) < Gem::Version.new('7.2') ::ActiveSupport::Notifications.notifier.synchronize do subscribers = ::ActiveSupport::Notifications.notifier.instance_variable_get(:@string_subscribers)[pattern] From f6303714c6e5baaff66cdba0f4c2f32a1cfc226d Mon Sep 17 00:00:00 2001 From: Katherine Oelsner <49968061+octokatherine@users.noreply.github.com> Date: Thu, 26 Oct 2023 17:29:07 +0000 Subject: [PATCH 3/5] fix module access? --- .../instrumentation/active_support/span_subscriber.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb b/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb index 8d3cbfc0f..c2a376ba3 100644 --- a/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb +++ b/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb @@ -30,7 +30,7 @@ def self.subscribe( subscriber_object = ::ActiveSupport::Notifications.subscribe(pattern, subscriber) - if Gem::Version.new(Rails.version) < Gem::Version.new('7.2') + if Gem::Version.new(::Rails.version) < Gem::Version.new('7.2') ::ActiveSupport::Notifications.notifier.synchronize do subscribers = ::ActiveSupport::Notifications.notifier.instance_variable_get(:@string_subscribers)[pattern] From bb699be8a482726ecd95701745b696c63c173f36 Mon Sep 17 00:00:00 2001 From: Katherine Oelsner <49968061+octokatherine@users.noreply.github.com> Date: Sun, 29 Oct 2023 17:37:17 +0000 Subject: [PATCH 4/5] check for synchronize method instead of Rails version --- .../instrumentation/active_support/span_subscriber.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb b/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb index c2a376ba3..5fd8c5cc2 100644 --- a/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb +++ b/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb @@ -30,7 +30,7 @@ def self.subscribe( subscriber_object = ::ActiveSupport::Notifications.subscribe(pattern, subscriber) - if Gem::Version.new(::Rails.version) < Gem::Version.new('7.2') + if ::ActiveSupport::Notifications.notifier.respond_to?(:synchronize) ::ActiveSupport::Notifications.notifier.synchronize do subscribers = ::ActiveSupport::Notifications.notifier.instance_variable_get(:@string_subscribers)[pattern] From 8c2c2e7c7fed8b733be0c947528689e08ada3b98 Mon Sep 17 00:00:00 2001 From: Katherine Oelsner <49968061+octokatherine@users.noreply.github.com> Date: Tue, 31 Oct 2023 17:11:09 +0000 Subject: [PATCH 5/5] add comment --- .../instrumentation/active_support/span_subscriber.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb b/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb index 5fd8c5cc2..7c1bcb0a9 100644 --- a/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb +++ b/instrumentation/active_support/lib/opentelemetry/instrumentation/active_support/span_subscriber.rb @@ -30,6 +30,8 @@ def self.subscribe( subscriber_object = ::ActiveSupport::Notifications.subscribe(pattern, subscriber) + # this can be removed once we drop support for Rails < 7.2 + # see https://github.com/open-telemetry/opentelemetry-ruby-contrib/pull/707 for more context if ::ActiveSupport::Notifications.notifier.respond_to?(:synchronize) ::ActiveSupport::Notifications.notifier.synchronize do subscribers = ::ActiveSupport::Notifications.notifier.instance_variable_get(:@string_subscribers)[pattern]