Skip to content

Commit

Permalink
fix: ActiveSupport constant conflict in Active Model Serializers in…
Browse files Browse the repository at this point in the history
…strumentation (#1072)

When both `ActiveSupport` and `ActiveModelSerializers` instrumentations are loaded,
constant `ActiveSupport::Notifications` inside `ActiveModelSerializers` can be incorrectly
resolved to `OpenTelemetry::Instrumentation::ActiveSupport`.

Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>
  • Loading branch information
flexoid and fbogsany authored Jan 5, 2022
1 parent da0b8f8 commit 46020cc
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def require_dependencies
end

def register_event_handler
ActiveSupport::Notifications.subscribe(event_name) do |_name, start, finish, _id, payload|
::ActiveSupport::Notifications.subscribe(event_name) do |_name, start, finish, _id, payload|
EventHandler.handle(start, finish, payload)
end
end
Expand Down
2 changes: 2 additions & 0 deletions instrumentation/all/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ gem 'opentelemetry-instrumentation-sinatra', path: '../sinatra'
gem 'opentelemetry-instrumentation-trilogy', path: '../trilogy'

group :test do
gem 'active_model_serializers'
gem 'activesupport'
gem 'opentelemetry-sdk', path: '../../sdk'
gem 'opentelemetry-semantic_conventions', path: '../../semantic_conventions'
end
23 changes: 23 additions & 0 deletions instrumentation/all/test/opentelemetry/instrumentation/all_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

# Copyright The OpenTelemetry Authors
#
# SPDX-License-Identifier: Apache-2.0

require_relative '../../test_helper'

require 'active_support/all'
require 'active_model_serializers'

# require Instrumentation so .install method is found:
require_relative '../../../lib/opentelemetry/instrumentation/all'

describe OpenTelemetry::Instrumentation::All do
describe 'ActiveModelSerializers instrumentation' do
it 'does not conflict with ActiveSupport instrumentation' do
# Expected behavior is that the exception is not raised.
OpenTelemetry::Instrumentation::ActiveSupport::Instrumentation.instance.install
OpenTelemetry::Instrumentation::ActiveModelSerializers::Instrumentation.instance.install
end
end
end
1 change: 0 additions & 1 deletion instrumentation/all/test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,3 @@
# SPDX-License-Identifier: Apache-2.0

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

0 comments on commit 46020cc

Please sign in to comment.