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 66f1f96a2..8a98e3efb 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 @@ -65,16 +65,17 @@ class SpanSubscriber # rubocop:disable Metrics/ParameterLists def initialize(name:, tracer:, notification_payload_transform: nil, disallowed_notification_payload_keys: nil, kind: nil, span_name_formatter: nil) - @span_name = safe_span_name_for(span_name_formatter, name).dup.freeze + @name = name @tracer = tracer @notification_payload_transform = notification_payload_transform @disallowed_notification_payload_keys = Array(disallowed_notification_payload_keys) @kind = kind || :internal + @span_name_formatter = span_name_formatter end # rubocop:enable Metrics/ParameterLists def start(name, id, payload) - span = @tracer.start_span(@span_name, kind: @kind) + span = @tracer.start_span(span_name(name).dup.freeze, kind: @kind) token = OpenTelemetry::Context.attach( OpenTelemetry::Trace.context_with_span(span) ) @@ -137,11 +138,20 @@ def sanitized_value(value) end end + def span_name(name) + case @name + when Regexp + safe_span_name_for(name) + else + @span_name ||= safe_span_name_for(@name) + end + end + # Helper method to try an shield the span name formatter from errors # # It wraps the user supplied formatter in a rescue block and returns the original name if a StandardError is raised by the formatter - def safe_span_name_for(span_name_formatter, name) - span_name_formatter&.call(name) || name + def safe_span_name_for(name) + @span_name_formatter&.call(name) || name rescue StandardError => e OpenTelemetry.handle_error(exception: e, message: 'Error calling span_name_formatter. Using default span name.') name diff --git a/instrumentation/active_support/test/opentelemetry/instrumentation/active_support/span_subscriber_test.rb b/instrumentation/active_support/test/opentelemetry/instrumentation/active_support/span_subscriber_test.rb index f42141afb..0e2c75269 100644 --- a/instrumentation/active_support/test/opentelemetry/instrumentation/active_support/span_subscriber_test.rb +++ b/instrumentation/active_support/test/opentelemetry/instrumentation/active_support/span_subscriber_test.rb @@ -205,123 +205,226 @@ def finish(name, id, payload) end describe 'instrument' do - before do - ActiveSupport::Notifications.unsubscribe(notification_name) + after do + ActiveSupport::Notifications.notifier.all_listeners_for(notification_name).each do |listener| + ActiveSupport::Notifications.unsubscribe(listener) + end end - it 'does not trace an event by default' do - ActiveSupport::Notifications.subscribe(notification_name) do - # pass + describe 'when subscribing to events directly' do + it 'does not trace an event by default' do + ActiveSupport::Notifications.subscribe(notification_name) do + # pass + end + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + _(last_span).must_be_nil end - ActiveSupport::Notifications.instrument(notification_name, extra: 'context') - _(last_span).must_be_nil - end - it 'traces an event when a span subscriber is used' do - OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name) - ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + it 'traces an event when a span subscriber is used' do + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name) + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') - _(last_span).wont_be_nil - _(last_span.name).must_equal(notification_name) - _(last_span.attributes['extra']).must_equal('context') - _(last_span.kind).must_equal(:internal) - end + _(last_span).wont_be_nil + _(last_span.name).must_equal(notification_name) + _(last_span.attributes['extra']).must_equal('context') + _(last_span.kind).must_equal(:internal) + end - describe 'when using a custom span name formatter' do - describe 'when using the LEGACY_NAME_FORMATTER' do - let(:span_name_formatter) { OpenTelemetry::Instrumentation::ActiveSupport::LEGACY_NAME_FORMATTER } - it 'uses the user supplied formatter' do - OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, span_name_formatter: span_name_formatter) - ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + describe 'when using a custom span name formatter' do + describe 'when using the LEGACY_NAME_FORMATTER' do + let(:span_name_formatter) { OpenTelemetry::Instrumentation::ActiveSupport::LEGACY_NAME_FORMATTER } + it 'uses the user supplied formatter' do + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, span_name_formatter: span_name_formatter) + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + + _(last_span).wont_be_nil + _(last_span.name).must_equal('foo bar') + _(last_span.attributes['extra']).must_equal('context') + end + end - _(last_span).wont_be_nil - _(last_span.name).must_equal('foo bar') - _(last_span.attributes['extra']).must_equal('context') + describe 'when using a custom formatter' do + let(:span_name_formatter) { ->(name) { "custom.#{name}" } } + + it 'uses the user supplied formatter' do + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, span_name_formatter: span_name_formatter) + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + + _(last_span).wont_be_nil + _(last_span.name).must_equal('custom.bar.foo') + _(last_span.attributes['extra']).must_equal('context') + end end - end - describe 'when using a custom formatter' do - let(:span_name_formatter) { ->(name) { "custom.#{name}" } } + describe 'when using a invalid formatter' do + it 'defaults to the notification name' do + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, span_name_formatter: ->(_) {}) + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') - it 'uses the user supplied formatter' do - OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, span_name_formatter: span_name_formatter) - ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + _(last_span).wont_be_nil + _(last_span.name).must_equal(notification_name) + _(last_span.attributes['extra']).must_equal('context') + end + end + + describe 'when using a unstable formatter' do + it 'defaults to the notification name' do + allow(OpenTelemetry).to receive(:handle_error).with(exception: RuntimeError, message: String) + + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, span_name_formatter: ->(_) { raise 'boom' }) + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') - _(last_span).wont_be_nil - _(last_span.name).must_equal('custom.bar.foo') - _(last_span.attributes['extra']).must_equal('context') + _(last_span).wont_be_nil + _(last_span.name).must_equal(notification_name) + _(last_span.attributes['extra']).must_equal('context') + end end end - describe 'when using a invalid formatter' do - it 'defaults to the notification name' do - OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, span_name_formatter: ->(_) {}) + it 'finishes spans even when block subscribers blow up' do + ActiveSupport::Notifications.subscribe(notification_name) { raise 'boom' } + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name) + + expect do ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + end.must_raise RuntimeError - _(last_span).wont_be_nil - _(last_span.name).must_equal(notification_name) - _(last_span.attributes['extra']).must_equal('context') - end + _(last_span).wont_be_nil + _(last_span.name).must_equal(notification_name) + _(last_span.attributes['extra']).must_equal('context') end - describe 'when using a unstable formatter' do - it 'defaults to the notification name' do - allow(OpenTelemetry).to receive(:handle_error).with(exception: RuntimeError, message: String) + it 'finishes spans even when complex subscribers blow up' do + ActiveSupport::Notifications.subscribe(notification_name, CrashingEndSubscriber.new) + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name) - OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, span_name_formatter: ->(_) { raise 'boom' }) + expect do ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + end.must_raise RuntimeError - _(last_span).wont_be_nil - _(last_span.name).must_equal(notification_name) - _(last_span.attributes['extra']).must_equal('context') - end + _(last_span).wont_be_nil + _(last_span.name).must_equal(notification_name) + _(last_span.attributes['extra']).must_equal('context') end - end - it 'finishes spans even when block subscribers blow up' do - ActiveSupport::Notifications.subscribe(notification_name) { raise 'boom' } - OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name) + it 'supports unsubscribe' do + obj = OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name) + ActiveSupport::Notifications.unsubscribe(obj) - expect do ActiveSupport::Notifications.instrument(notification_name, extra: 'context') - end.must_raise RuntimeError - _(last_span).wont_be_nil - _(last_span.name).must_equal(notification_name) - _(last_span.attributes['extra']).must_equal('context') + _(obj.class).must_equal(ActiveSupport::Notifications::Fanout::Subscribers::Evented) + _(last_span).must_be_nil + end + + it 'supports setting the span kind' do + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, 'bar.foo', nil, [], kind: :client) + ActiveSupport::Notifications.instrument('bar.foo', extra: 'context') + + _(last_span).wont_be_nil + _(last_span.name).must_equal('bar.foo') + _(last_span.attributes['extra']).must_equal('context') + _(last_span.kind).must_equal(:client) + end end - it 'finishes spans even when complex subscribers blow up' do - ActiveSupport::Notifications.subscribe(notification_name, CrashingEndSubscriber.new) - OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name) + describe 'when subscribing to events matching a regular expression' do + let(:notification_pattern) { /.*\.foo/ } - expect do + it 'does not trace an event by default' do + ActiveSupport::Notifications.subscribe(notification_pattern) do + # pass + end ActiveSupport::Notifications.instrument(notification_name, extra: 'context') - end.must_raise RuntimeError + _(last_span).must_be_nil + end - _(last_span).wont_be_nil - _(last_span.name).must_equal(notification_name) - _(last_span.attributes['extra']).must_equal('context') - end + it 'traces an event when a span subscriber is used' do + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_pattern) + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') - it 'supports unsubscribe' do - obj = OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name) - ActiveSupport::Notifications.unsubscribe(obj) + _(last_span).wont_be_nil + _(last_span.name).must_equal(notification_name) + _(last_span.attributes['extra']).must_equal('context') + _(last_span.kind).must_equal(:internal) + end - ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + describe 'when using a custom span name formatter' do + describe 'when using the LEGACY_NAME_FORMATTER' do + let(:span_name_formatter) { OpenTelemetry::Instrumentation::ActiveSupport::LEGACY_NAME_FORMATTER } + it 'uses the user supplied formatter' do + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_pattern, nil, nil, span_name_formatter: span_name_formatter) + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + + _(last_span).wont_be_nil + _(last_span.name).must_equal('foo bar') + _(last_span.attributes['extra']).must_equal('context') + end + end - _(obj.class).must_equal(ActiveSupport::Notifications::Fanout::Subscribers::Evented) - _(last_span).must_be_nil - end + describe 'when using a custom formatter' do + let(:span_name_formatter) { ->(name) { "custom.#{name}" } } - it 'supports setting the span kind' do - OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, 'bar.foo', nil, [], kind: :client) - ActiveSupport::Notifications.instrument('bar.foo', extra: 'context') + it 'uses the user supplied formatter' do + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_pattern, nil, nil, span_name_formatter: span_name_formatter) + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') - _(last_span).wont_be_nil - _(last_span.name).must_equal('bar.foo') - _(last_span.attributes['extra']).must_equal('context') - _(last_span.kind).must_equal(:client) + _(last_span).wont_be_nil + _(last_span.name).must_equal('custom.bar.foo') + _(last_span.attributes['extra']).must_equal('context') + end + end + + describe 'when using a invalid formatter' do + it 'defaults to the notification name' do + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_pattern, nil, nil, span_name_formatter: ->(_) {}) + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + + _(last_span).wont_be_nil + _(last_span.name).must_equal(notification_name) + _(last_span.attributes['extra']).must_equal('context') + end + end + + describe 'when using a unstable formatter' do + it 'defaults to the notification name' do + allow(OpenTelemetry).to receive(:handle_error).with(exception: RuntimeError, message: String) + + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_pattern, nil, nil, span_name_formatter: ->(_) { raise 'boom' }) + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + + _(last_span).wont_be_nil + _(last_span.name).must_equal(notification_name) + _(last_span.attributes['extra']).must_equal('context') + end + end + end + + it 'finishes spans even when block subscribers blow up' do + ActiveSupport::Notifications.subscribe(notification_pattern) { raise 'boom' } + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_pattern) + + expect do + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + end.must_raise RuntimeError + + _(last_span).wont_be_nil + _(last_span.name).must_equal(notification_name) + _(last_span.attributes['extra']).must_equal('context') + end + + it 'finishes spans even when complex subscribers blow up' do + ActiveSupport::Notifications.subscribe(notification_pattern, CrashingEndSubscriber.new) + OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_pattern) + + expect do + ActiveSupport::Notifications.instrument(notification_name, extra: 'context') + end.must_raise RuntimeError + + _(last_span).wont_be_nil + _(last_span.name).must_equal(notification_name) + _(last_span.attributes['extra']).must_equal('context') + end end end end