diff --git a/api/lib/opentelemetry/internal/proxy_tracer_provider.rb b/api/lib/opentelemetry/internal/proxy_tracer_provider.rb index 05ddf2463b..01526ae5d8 100644 --- a/api/lib/opentelemetry/internal/proxy_tracer_provider.rb +++ b/api/lib/opentelemetry/internal/proxy_tracer_provider.rb @@ -13,9 +13,22 @@ module Internal # It delegates to a "real" TracerProvider after the global tracer provider is registered. # It returns {ProxyTracer} instances until the delegate is installed. class ProxyTracerProvider < Trace::TracerProvider - Key = Struct.new(:name, :version) + Key = Struct.new(:name, :version, :attributes) private_constant(:Key) + # Wraps a legacy TracerProvider whose `#tracer` method does not accept + # the `attributes:` keyword argument, dropping the argument on delegation. + class LegacyProviderWrapper + def initialize(legacy_provider) + @legacy_provider = legacy_provider + end + + def tracer(name = nil, version = nil, attributes: nil) # rubocop:disable Lint/UnusedMethodArgument + @legacy_provider.tracer(name, version) + end + end + private_constant(:LegacyProviderWrapper) + # Returns a new {ProxyTracerProvider} instance. # # @return [ProxyTracerProvider] @@ -35,25 +48,45 @@ def delegate=(provider) return end + provider = LegacyProviderWrapper.new(provider) unless supports_attributes?(provider) + @mutex.synchronize do @delegate = provider - @registry.each { |key, tracer| tracer.delegate = provider.tracer(key.name, key.version) } + @registry.each { |key, tracer| tracer.delegate = @delegate.tracer(key.name, key.version, attributes: key.attributes) } end end # Returns a {Tracer} instance. # - # @param [optional String] name Instrumentation package name - # @param [optional String] version Instrumentation package version + # Supports both positional arguments (legacy) and keyword arguments: + # tracer('name', '1.0') # legacy positional + # tracer(name: 'name', version: '1.0', attributes: {...}) # keyword + # + # When both positional and keyword arguments are provided for the same + # parameter, the keyword argument takes precedence. + # + # @param [String] name Instrumentation scope name + # @param [String] version Instrumentation scope version + # @param [Hash{String => String, Numeric, Boolean, Array}] attributes + # Instrumentation scope attributes # # @return [Tracer] - def tracer(name = nil, version = nil) + def tracer(deprecated_name = nil, deprecated_version = nil, name: nil, version: nil, attributes: nil) + name ||= deprecated_name + version ||= deprecated_version @mutex.synchronize do - return @delegate.tracer(name, version) unless @delegate.nil? + return @delegate.tracer(name, version, attributes: attributes) unless @delegate.nil? - @registry[Key.new(name, version)] ||= ProxyTracer.new + @registry[Key.new(name, version, attributes)] ||= ProxyTracer.new end end + + private + + def supports_attributes?(provider) + provider.respond_to?(:tracer) && + provider.method(:tracer).parameters.any? { |_, n| n == :attributes } + end end end end diff --git a/api/lib/opentelemetry/trace/tracer_provider.rb b/api/lib/opentelemetry/trace/tracer_provider.rb index cdbab3ea52..c90b465bd9 100644 --- a/api/lib/opentelemetry/trace/tracer_provider.rb +++ b/api/lib/opentelemetry/trace/tracer_provider.rb @@ -10,11 +10,20 @@ module Trace class TracerProvider # Returns a {Tracer} instance. # - # @param [optional String] name Instrumentation package name - # @param [optional String] version Instrumentation package version + # Supports both positional arguments (legacy) and keyword arguments: + # tracer('name', '1.0') # legacy positional + # tracer(name: 'name', version: '1.0', attributes: {...}) # keyword + # + # When both positional and keyword arguments are provided for the same + # parameter, the keyword argument takes precedence. + # + # @param [String] name Instrumentation scope name + # @param [String] version Instrumentation scope version + # @param [Hash{String => String, Numeric, Boolean, Array}] attributes + # Instrumentation scope attributes # # @return [Tracer] - def tracer(name = nil, version = nil) + def tracer(deprecated_name = nil, deprecated_version = nil, name: nil, version: nil, attributes: nil) @tracer ||= Tracer.new end end diff --git a/api/test/opentelemetry/trace/tracer_provider_test.rb b/api/test/opentelemetry/trace/tracer_provider_test.rb index aa94a7f74d..1ae03dc220 100644 --- a/api/test/opentelemetry/trace/tracer_provider_test.rb +++ b/api/test/opentelemetry/trace/tracer_provider_test.rb @@ -10,10 +10,45 @@ let(:tracer_provider) { OpenTelemetry::Trace::TracerProvider.new } describe '.tracer' do - it 'returns the same tracer for the same arguments' do + # Legacy positional calling conventions + it 'returns a tracer with no arguments' do + tracer = tracer_provider.tracer + _(tracer).must_be_instance_of(OpenTelemetry::Trace::Tracer) + end + + it 'returns a tracer with name only' do + tracer = tracer_provider.tracer('component') + _(tracer).must_be_instance_of(OpenTelemetry::Trace::Tracer) + end + + it 'returns the same tracer for the same positional arguments' do tracer1 = tracer_provider.tracer('component', '1.0') tracer2 = tracer_provider.tracer('component', '1.0') _(tracer1).must_equal(tracer2) end + + # Keyword calling conventions + it 'accepts all keyword arguments' do + tracer = tracer_provider.tracer(name: 'component', version: '1.0', attributes: { 'key' => 'value' }) + _(tracer).must_be_instance_of(OpenTelemetry::Trace::Tracer) + end + + it 'accepts name keyword only' do + tracer = tracer_provider.tracer(name: 'component') + _(tracer).must_be_instance_of(OpenTelemetry::Trace::Tracer) + end + + # Mixed positional + keyword + it 'accepts positional arguments with attributes keyword' do + tracer = tracer_provider.tracer('component', '1.0', attributes: { 'key' => 'value' }) + _(tracer).must_be_instance_of(OpenTelemetry::Trace::Tracer) + end + + # Nil attributes equivalence + it 'returns the same tracer without attributes' do + tracer1 = tracer_provider.tracer('component', '1.0') + tracer2 = tracer_provider.tracer('component', '1.0', attributes: nil) + _(tracer1).must_equal(tracer2) + end end end diff --git a/api/test/opentelemetry_test.rb b/api/test/opentelemetry_test.rb index b059365aea..9e3fc0f2c5 100644 --- a/api/test/opentelemetry_test.rb +++ b/api/test/opentelemetry_test.rb @@ -45,6 +45,17 @@ def tracer(name = nil, version = nil) end end + class AttributeAwareTracerProvider < OpenTelemetry::Trace::TracerProvider + attr_reader :last_name, :last_version, :last_attributes + + def tracer(deprecated_name = nil, deprecated_version = nil, name: nil, version: nil, attributes: nil) + @last_name = name || deprecated_name + @last_version = version || deprecated_version + @last_attributes = attributes + CustomTracer.new + end + end + describe '.tracer_provider=' do after do # Ensure we don't leak custom tracer factories and tracers to other tests @@ -63,6 +74,38 @@ def tracer(name = nil, version = nil) OpenTelemetry.tracer_provider = CustomTracerProvider.new _(default_tracer_provider.tracer).must_be_instance_of(CustomTracer) end + + it 'delegates to a provider that does not support attributes without error' do + OpenTelemetry.tracer_provider.tracer('component', '1.0', attributes: { 'key' => 'value' }) + OpenTelemetry.tracer_provider = CustomTracerProvider.new + _(OpenTelemetry.tracer_provider.tracer('component', '1.0')).must_be_instance_of(CustomTracer) + end + + it 'delegates attributes to a provider that supports them' do + attrs = { 'key' => 'value' } + OpenTelemetry.tracer_provider.tracer('component', '1.0', attributes: attrs) + provider = AttributeAwareTracerProvider.new + OpenTelemetry.tracer_provider = provider + _(provider.last_attributes).must_equal(attrs) + end + + it 'replays keyword-style tracers when delegate is set' do + OpenTelemetry.tracer_provider.tracer(name: 'component', version: '1.0', attributes: { 'k' => 'v' }) + provider = AttributeAwareTracerProvider.new + OpenTelemetry.tracer_provider = provider + _(provider.last_name).must_equal('component') + _(provider.last_version).must_equal('1.0') + _(provider.last_attributes).must_equal('k' => 'v') + end + + it 'delegates tracers obtained after delegate assignment with attributes' do + provider = AttributeAwareTracerProvider.new + OpenTelemetry.tracer_provider = provider + OpenTelemetry.tracer_provider.tracer('component', '1.0', attributes: { 'key' => 'value' }) + _(provider.last_name).must_equal('component') + _(provider.last_version).must_equal('1.0') + _(provider.last_attributes).must_equal('key' => 'value') + end end describe '.handle_error' do diff --git a/sdk/lib/opentelemetry/sdk/instrumentation_scope.rb b/sdk/lib/opentelemetry/sdk/instrumentation_scope.rb index 606738ebbc..dc29db13dd 100644 --- a/sdk/lib/opentelemetry/sdk/instrumentation_scope.rb +++ b/sdk/lib/opentelemetry/sdk/instrumentation_scope.rb @@ -8,6 +8,7 @@ module OpenTelemetry module SDK # InstrumentationScope is a struct containing scope information for export. InstrumentationScope = Struct.new(:name, - :version) + :version, + :attributes) end end diff --git a/sdk/lib/opentelemetry/sdk/trace/tracer.rb b/sdk/lib/opentelemetry/sdk/trace/tracer.rb index 1224b989ad..e8fb72eb15 100644 --- a/sdk/lib/opentelemetry/sdk/trace/tracer.rb +++ b/sdk/lib/opentelemetry/sdk/trace/tracer.rb @@ -13,13 +13,15 @@ class Tracer < OpenTelemetry::Trace::Tracer # # Returns a new {Tracer} instance. # - # @param [String] name Instrumentation package name - # @param [String] version Instrumentation package version + # @param [String] name Instrumentation scope name + # @param [String] version Instrumentation scope version # @param [TracerProvider] tracer_provider TracerProvider that initialized the tracer + # @param [Hash{String => String, Numeric, Boolean, Array}] attributes + # Instrumentation scope attributes # # @return [Tracer] - def initialize(name, version, tracer_provider) - @instrumentation_scope = InstrumentationScope.new(name, version) + def initialize(name, version, tracer_provider, attributes: nil) + @instrumentation_scope = InstrumentationScope.new(name, version, attributes || {}.freeze) @tracer_provider = tracer_provider end diff --git a/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb b/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb index 3569451635..54c9de3c89 100644 --- a/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb +++ b/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb @@ -9,8 +9,10 @@ module SDK module Trace # {TracerProvider} is the SDK implementation of {OpenTelemetry::Trace::TracerProvider}. class TracerProvider < OpenTelemetry::Trace::TracerProvider # rubocop:disable Metrics/ClassLength - Key = Struct.new(:name, :version) - private_constant(:Key) + Key = Struct.new(:name, :version, :attributes) + EMPTY_ATTRIBUTES = {}.freeze + + private_constant(:Key, :EMPTY_ATTRIBUTES) attr_accessor :span_limits, :id_generator, :sampler attr_reader :resource @@ -44,15 +46,28 @@ def initialize(sampler: sampler_from_environment(Samplers.parent_based(root: Sam # Returns a {Tracer} instance. # - # @param [optional String] name Instrumentation package name - # @param [optional String] version Instrumentation package version + # Supports both positional arguments (legacy) and keyword arguments: + # tracer('name', '1.0') # legacy positional + # tracer(name: 'name', version: '1.0', attributes: {...}) # keyword + # + # When both positional and keyword arguments are provided for the same + # parameter, the keyword argument takes precedence. + # + # @param [String] name Instrumentation scope name + # @param [String] version Instrumentation scope version + # @param [Hash{String => String, Numeric, Boolean, Array}] attributes + # Instrumentation scope attributes # # @return [Tracer] - def tracer(name = nil, version = nil) - name ||= '' - version ||= '' + def tracer(deprecated_name = nil, deprecated_version = nil, name: nil, version: nil, attributes: nil) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity + name ||= deprecated_name || '' + version ||= deprecated_version || '' + attributes = attributes&.dup&.freeze || EMPTY_ATTRIBUTES OpenTelemetry.logger.warn 'calling TracerProvider#tracer without providing a tracer name.' if name.empty? - @registry_mutex.synchronize { @registry[Key.new(name, version)] ||= Tracer.new(name, version, self) } + @registry_mutex.synchronize do + @registry[Key.new(name, version, attributes)] ||= + Tracer.new(name, version, self, attributes: attributes) + end end # Attempts to stop all the activity for this {TracerProvider}. Calls diff --git a/sdk/test/opentelemetry/sdk/trace/tracer_provider_test.rb b/sdk/test/opentelemetry/sdk/trace/tracer_provider_test.rb index 718813f735..ba45df67d1 100644 --- a/sdk/test/opentelemetry/sdk/trace/tracer_provider_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/tracer_provider_test.rb @@ -147,6 +147,7 @@ end describe '#tracer' do + # Legacy positional calling conventions it 'returns the same tracer for the same arguments' do OpenTelemetry::TestHelpers.with_test_logger do |log_stream| tracer1 = tracer_provider.tracer('component', '1.0') @@ -156,6 +157,11 @@ end end + it 'defaults version to empty string when given positional name only' do + tracer = tracer_provider.tracer('component') + _(tracer).wont_be_nil + end + it 'returns different tracers for different names' do tracer1 = tracer_provider.tracer('component1', '1.0') tracer2 = tracer_provider.tracer('component2', '1.0') @@ -168,11 +174,68 @@ _(tracer1).wont_equal(tracer2) end - it 'warn when no name is passed for the tracer' do + it 'warns when no name is passed for the tracer' do OpenTelemetry::TestHelpers.with_test_logger do |log_stream| tracer_provider.tracer _(log_stream.string).must_match(/calling TracerProvider#tracer without providing a tracer name./) end end + + # Keyword calling conventions + it 'accepts all keyword arguments' do + tracer = tracer_provider.tracer(name: 'component', version: '1.0', attributes: { 'key' => 'value' }) + _(tracer).wont_be_nil + end + + it 'returns the same tracer for equivalent positional and keyword arguments' do + tracer1 = tracer_provider.tracer('component', '1.0') + tracer2 = tracer_provider.tracer(name: 'component', version: '1.0') + _(tracer1).must_equal(tracer2) + end + + # Mixed positional + keyword + it 'accepts positional name with keyword version' do + tracer = tracer_provider.tracer('component', version: '1.0') + _(tracer).wont_be_nil + end + + it 'prefers keyword arguments over positional arguments' do + tracer1 = tracer_provider.tracer('positional', '1.0') + tracer2 = tracer_provider.tracer('keyword', '2.0', name: 'keyword', version: '2.0') + _(tracer1).wont_equal(tracer2) + end + + # Attributes + it 'returns the same tracer for the same name, version, and attributes' do + tracer1 = tracer_provider.tracer('component', '1.0', attributes: { 'key' => 'value' }) + tracer2 = tracer_provider.tracer('component', '1.0', attributes: { 'key' => 'value' }) + _(tracer1).must_equal(tracer2) + end + + it 'returns different tracers for different attributes' do + tracer1 = tracer_provider.tracer('component', '1.0', attributes: { 'key' => 'value1' }) + tracer2 = tracer_provider.tracer('component', '1.0', attributes: { 'key' => 'value2' }) + _(tracer1).wont_equal(tracer2) + end + + it 'returns different tracers for attributes vs no attributes' do + tracer1 = tracer_provider.tracer('component', '1.0') + tracer2 = tracer_provider.tracer('component', '1.0', attributes: { 'key' => 'value' }) + _(tracer1).wont_equal(tracer2) + end + + it 'treats nil attributes the same as no attributes' do + tracer1 = tracer_provider.tracer('component', '1.0') + tracer2 = tracer_provider.tracer('component', '1.0', attributes: nil) + _(tracer1).must_equal(tracer2) + end + + it 'does not allow mutation of attributes after tracer creation' do + attrs = { 'key' => 'value' } + tracer_provider.tracer('component', '1.0', attributes: attrs) + attrs['key'] = 'mutated' + tracer = tracer_provider.tracer('component', '1.0', attributes: { 'key' => 'value' }) + _(tracer).wont_be_nil + end end end diff --git a/sdk/test/opentelemetry/sdk/trace/tracer_test.rb b/sdk/test/opentelemetry/sdk/trace/tracer_test.rb index 7bba2783a3..e780aed9f8 100644 --- a/sdk/test/opentelemetry/sdk/trace/tracer_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/tracer_test.rb @@ -18,6 +18,36 @@ Samplers::ConstantSampler.new(decision: Decision::RECORD_ONLY, description: 'RecordSampler') end + describe 'instrumentation scope attributes' do + it 'creates a tracer with empty attributes by default' do + tracer = Tracer.new('component', '1.0', tracer_provider) + span = tracer.start_root_span('test') + _(span.instrumentation_scope.attributes).must_equal({}) + end + + it 'normalizes nil attributes to empty hash' do + tracer = Tracer.new('component', '1.0', tracer_provider, attributes: nil) + span = tracer.start_root_span('test') + _(span.instrumentation_scope.attributes).must_equal({}) + end + + it 'creates a tracer with attributes' do + attrs = { 'key' => 'value' } + tracer = Tracer.new('component', '1.0', tracer_provider, attributes: attrs) + span = tracer.start_root_span('test') + _(span.instrumentation_scope.attributes).must_equal(attrs) + end + + it 'propagates attributes through to exported span data' do + attrs = { 'key' => 'value' } + tracer = tracer_provider.tracer('component', '1.0', attributes: attrs) + span = tracer.start_root_span('test') + _(span.instrumentation_scope.name).must_equal('component') + _(span.instrumentation_scope.version).must_equal('1.0') + _(span.instrumentation_scope.attributes).must_equal(attrs) + end + end + describe '#start_root_span' do it 'provides a default name' do _(tracer.start_root_span(nil).name).wont_be_nil