Skip to content

Commit

Permalink
feat!: Custom ActiveSupport Span Names
Browse files Browse the repository at this point in the history
The current implementation of ActiveSupport instrumentation sets the span name to the reverse tokenized name,
e.g. `render_template.action_view` is converted to `action_view render_template`

This default behavior can sometimes seem counter intuitive for users who use ActiveSupport Notifications to instrument their own code
or users who are familiar with Rails instrumentation names.

This change does a few things to address the issues listed above:

1. Uses the notification name by default as oppossed to the legacy span name
2. Allows users to provide a custom span name formatter lambda
3. Provides a proc with backward compatible span name formatter `OpenTelemetry::Instrumentation::ActiveSupport::LEGACY_NAME_FORMATTER`

See #957
  • Loading branch information
arielvalentin committed Jun 20, 2024
1 parent 4bb7262 commit a649c5a
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 23 deletions.
1 change: 1 addition & 0 deletions instrumentation/active_support/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# OpenTelemetry ActiveSupport Instrumentation

The Active Support instrumentation is a community-maintained instrumentation for the Active Support portion of the [Ruby on Rails][rails-home] web-application framework.

## How do I get started?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ module OpenTelemetry
module Instrumentation
# rubocop:disable Style/Documentation
module ActiveSupport
LEGACY_NAME_FORMATTER = ->(name) { name.split('.')[0..1].reverse.join(' ') }

# The SpanSubscriber is a special ActiveSupport::Notification subscription
# handler which turns notifications into generic spans, taking care to handle
# context appropriately.
Expand All @@ -20,11 +22,13 @@ def self.subscribe(
pattern,
notification_payload_transform = nil,
disallowed_notification_payload_keys = [],
kind = nil
kind = nil,
span_name_formatter = nil
)
subscriber = OpenTelemetry::Instrumentation::ActiveSupport::SpanSubscriber.new(
name: pattern,
tracer: tracer,
span_name_formatter: span_name_formatter,
notification_payload_transform: notification_payload_transform,
disallowed_notification_payload_keys: disallowed_notification_payload_keys,
kind: nil
Expand Down Expand Up @@ -57,8 +61,8 @@ def self.subscribe(
class SpanSubscriber
ALWAYS_VALID_PAYLOAD_TYPES = [TrueClass, FalseClass, String, Numeric, Symbol].freeze

def initialize(name:, tracer:, notification_payload_transform: nil, disallowed_notification_payload_keys: [], kind: nil)
@span_name = name.split('.')[0..1].reverse.join(' ').freeze
def initialize(name:, tracer:, notification_payload_transform: nil, disallowed_notification_payload_keys: [], kind: nil, span_name_formatter: nil)
@span_name = safe_span_name_for(span_name_formatter, name).dup.freeze
@tracer = tracer
@notification_payload_transform = notification_payload_transform
@disallowed_notification_payload_keys = disallowed_notification_payload_keys
Expand Down Expand Up @@ -128,6 +132,16 @@ def sanitized_value(value)
value
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
rescue StandardError => e
OpenTelemetry.handle_error(exception: e, message: 'Error calling span_name_formatter. Using default span name.')
name
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'pry-byebug'
spec.add_development_dependency 'rails', '>= 6.1'
spec.add_development_dependency 'rake', '~> 13.0'
spec.add_development_dependency 'rspec-mocks'
spec.add_development_dependency 'rubocop', '~> 1.64.0'
spec.add_development_dependency 'rubocop-performance', '~> 1.20'
spec.add_development_dependency 'simplecov', '~> 0.17.1'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
let(:exporter) { EXPORTER }
let(:last_span) { exporter.finished_spans.last }
let(:span_kind) { nil }
let(:notification_name) { 'bar.foo' }
let(:subscriber) do
OpenTelemetry::Instrumentation::ActiveSupport::SpanSubscriber.new(
name: 'bar.foo',
name: notification_name,
tracer: tracer,
kind: span_kind
)
Expand All @@ -36,7 +37,7 @@ def finish(name, id, payload)

it 'memoizes the span name' do
span, = subscriber.start('oh.hai', 'abc', {})
_(span.name).must_equal('foo bar')
_(span.name).must_equal(notification_name)
end

it 'uses the provided tracer' do
Expand Down Expand Up @@ -115,7 +116,7 @@ def finish(name, id, payload)
describe 'instrumentation option - disallowed_notification_payload_keys' do
let(:subscriber) do
OpenTelemetry::Instrumentation::ActiveSupport::SpanSubscriber.new(
name: 'bar.foo',
name: notification_name,
tracer: tracer,
notification_payload_transform: nil,
disallowed_notification_payload_keys: [:foo]
Expand Down Expand Up @@ -153,7 +154,7 @@ def finish(name, id, payload)
let(:transformer_proc) { ->(v) { v.transform_values { 'optimus prime' } } }
let(:subscriber) do
OpenTelemetry::Instrumentation::ActiveSupport::SpanSubscriber.new(
name: 'bar.foo',
name: notification_name,
tracer: tracer,
notification_payload_transform: transformer_proc,
disallowed_notification_payload_keys: [:foo]
Expand Down Expand Up @@ -205,57 +206,105 @@ def finish(name, id, payload)

describe 'instrument' do
before do
ActiveSupport::Notifications.unsubscribe('bar.foo')
ActiveSupport::Notifications.unsubscribe(notification_name)
end

it 'does not trace an event by default' do
ActiveSupport::Notifications.subscribe('bar.foo') do
ActiveSupport::Notifications.subscribe(notification_name) do
# pass
end
ActiveSupport::Notifications.instrument('bar.foo', extra: 'context')
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, 'bar.foo')
ActiveSupport::Notifications.instrument('bar.foo', extra: 'context')
OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name)
ActiveSupport::Notifications.instrument(notification_name, extra: 'context')

_(last_span).wont_be_nil
_(last_span.name).must_equal('foo bar')
_(last_span.name).must_equal(notification_name)
_(last_span.attributes['extra']).must_equal('context')
end

describe 'when using a custom span name formatter' do
describe 'when using the LEGACY_NAME_FORATTER' do
it 'uses the user supplied formatter' do
OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, OpenTelemetry::Instrumentation::ActiveSupport::LEGACY_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

describe 'when using a custom formatter' do
it 'uses the user supplied formatter' do
OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, ->(name) { "custom.#{name}" })
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

describe 'when using a invalid formatter' do
it 'defaults to the notification name' do
OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name, nil, nil, ->(_) {})
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, ->(_) { 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('bar.foo') { raise 'boom' }
OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, 'bar.foo')
ActiveSupport::Notifications.subscribe(notification_name) { raise 'boom' }
OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name)

expect do
ActiveSupport::Notifications.instrument('bar.foo', extra: 'context')
ActiveSupport::Notifications.instrument(notification_name, extra: 'context')
end.must_raise RuntimeError

_(last_span).wont_be_nil
_(last_span.name).must_equal('foo bar')
_(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('bar.foo', CrashingEndSubscriber.new)
OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, 'bar.foo')
ActiveSupport::Notifications.subscribe(notification_name, CrashingEndSubscriber.new)
OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name)

expect do
ActiveSupport::Notifications.instrument('bar.foo', extra: 'context')
ActiveSupport::Notifications.instrument(notification_name, extra: 'context')
end.must_raise RuntimeError

_(last_span).wont_be_nil
_(last_span.name).must_equal('foo bar')
_(last_span.name).must_equal(notification_name)
_(last_span.attributes['extra']).must_equal('context')
end

it 'supports unsubscribe' do
obj = OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, 'bar.foo')
obj = OpenTelemetry::Instrumentation::ActiveSupport.subscribe(tracer, notification_name)
ActiveSupport::Notifications.unsubscribe(obj)

ActiveSupport::Notifications.instrument('bar.foo', extra: 'context')
ActiveSupport::Notifications.instrument(notification_name, extra: 'context')

_(obj.class).must_equal(ActiveSupport::Notifications::Fanout::Subscribers::Evented)
_(last_span).must_be_nil
Expand Down
1 change: 1 addition & 0 deletions instrumentation/active_support/test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
require 'opentelemetry-instrumentation-active_support'

require 'minitest/autorun'
require 'rspec/mocks/minitest_integration'
require 'webmock/minitest'

# global opentelemetry-sdk setup:
Expand Down

0 comments on commit a649c5a

Please sign in to comment.