diff --git a/instrumentation/aws_sdk/README.md b/instrumentation/aws_sdk/README.md index fcf4de3683..d56a06d05c 100644 --- a/instrumentation/aws_sdk/README.md +++ b/instrumentation/aws_sdk/README.md @@ -20,7 +20,7 @@ To install the instrumentation, call `use` with the name of the instrumentation. OpenTelemetry::SDK.configure do |c| c.use 'OpenTelemetry::Instrumentation::AwsSdk', { inject_messaging_context: true, - suppress_internal_instrumentation: true + enable_internal_instrumentation: true } end ``` @@ -36,8 +36,10 @@ end This instrumentation offers the following configuration options: * `:inject_messaging_context` (default: `false`): When set to `true`, adds context key/value to Message Attributes for SQS/SNS messages. -* `suppress_internal_instrumentation` (default: `false`): When set to `true`, any spans with - span kind of `internal` are suppressed from traces. +* `:enable_internal_instrumentation` (default: `false`): When set to `true`, any spans with + span kind of `internal` are traced. +* `:suppress_internal_instrumentation`: **Deprecated**. This configuration has been + deprecated in a favor of `:enable_internal_instrumentation` ## Integration with SDK V3's Telemetry support AWS SDK for Ruby V3 added support for Observability which includes a new configuration, diff --git a/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/handler.rb b/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/handler.rb index 40ec604c70..b335bca41a 100644 --- a/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/handler.rb +++ b/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/handler.rb @@ -23,7 +23,7 @@ def call(context) ) do |span| MessagingHelper.inject_context_if_supported(context, client_method, service_id) - if HandlerHelper.instrumentation_config[:suppress_internal_instrumentation] + if HandlerHelper.skip_internal_instrumentation? OpenTelemetry::Common::Utilities.untraced { super } else super @@ -32,7 +32,6 @@ def call(context) OpenTelemetry::SemanticConventions::Trace::HTTP_STATUS_CODE, context.http_response.status_code ) - if (err = response.error) span.record_exception(err) span.status = Trace::Status.error(err.to_s) diff --git a/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/handler_helper.rb b/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/handler_helper.rb index 9b5a01304b..9b744083a7 100644 --- a/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/handler_helper.rb +++ b/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/handler_helper.rb @@ -14,6 +14,10 @@ def instrumentation_config AwsSdk::Instrumentation.instance.config end + def skip_internal_instrumentation? + instrumentation_config[:enable_internal_instrumentation] == false + end + def client_method(service_id, context) "#{service_id}.#{context.operation.name}".delete(' ') end diff --git a/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/instrumentation.rb b/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/instrumentation.rb index b47e652a0e..1a0ac5a664 100644 --- a/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/instrumentation.rb +++ b/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/instrumentation.rb @@ -19,18 +19,20 @@ module AwsSdk # - `false` **(default)** - Context key/value will not be added. # - `true` - Context key/value will be added. # - # ### `:suppress_internal_instrumentation` + # ### `:enable_internal_instrumentation` + # Enables tracing of spans of `internal` span kind. # - # Disables tracing of spans of `internal` span kind. + # - `false` **(default)** - Internal spans are not traced + # - `true` - Internal spans are traced. # - # - `false` **(default)** - Internal spans are traced. - # - `true` - Internal spans are not traced. + # ### `:suppress_internal_instrumentation` (deprecated) + # This configuration has been deprecated in a favor of `:enable_internal_instrumentation` # - # @example An explicit default configuration + # @example An explicit default configurations # OpenTelemetry::SDK.configure do |c| # c.use 'OpenTelemetry::Instrumentation::AwsSdk', { # inject_messaging_context: false, - # suppress_internal_instrumentation: false + # enable_internal_instrumentation: false # } # end class Instrumentation < OpenTelemetry::Instrumentation::Base @@ -51,7 +53,7 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base end option :inject_messaging_context, default: false, validate: :boolean - option :suppress_internal_instrumentation, default: false, validate: :boolean + option :enable_internal_instrumentation, default: false, validate: :boolean def gem_version if Gem.loaded_specs['aws-sdk'] diff --git a/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/patches/telemetry.rb b/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/patches/telemetry.rb index 68c5141291..96a9a6d6d4 100644 --- a/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/patches/telemetry.rb +++ b/instrumentation/aws_sdk/lib/opentelemetry/instrumentation/aws_sdk/patches/telemetry.rb @@ -26,7 +26,7 @@ def span_wrapper(context, &) ) do |span| MessagingHelper.inject_context_if_supported(context, client_method, service_id) - if HandlerHelper.instrumentation_config[:suppress_internal_instrumentation] + if HandlerHelper.skip_internal_instrumentation? OpenTelemetry::Common::Utilities.untraced { super } else yield span diff --git a/instrumentation/aws_sdk/test/opentelemetry/handler_test.rb b/instrumentation/aws_sdk/test/opentelemetry/handler_test.rb index 8b178948cd..041f6149f5 100644 --- a/instrumentation/aws_sdk/test/opentelemetry/handler_test.rb +++ b/instrumentation/aws_sdk/test/opentelemetry/handler_test.rb @@ -4,7 +4,7 @@ # # SPDX-License-Identifier: Apache-2.0 -require 'test_helper' +require_relative '../test_helper' describe OpenTelemetry::Instrumentation::AwsSdk do describe 'AwsSdk Plugin' do @@ -95,8 +95,7 @@ client.publish(message: 'msg', phone_number: '123456') _(span.name).must_equal('phone_number publish') - _(span.attributes[otel_semantic::MESSAGING_DESTINATION]) - .must_equal('phone_number') + _(span.attributes[otel_semantic::MESSAGING_DESTINATION]).must_equal('phone_number') end end @@ -178,8 +177,7 @@ client.get_queue_url(queue_name: 'queue-name') - _(span.attributes['messaging.destination']) - .must_equal('unknown') + _(span.attributes['messaging.destination']).must_equal('unknown') _(span.attributes).wont_include('messaging.url') end end @@ -193,8 +191,7 @@ client.list_tables - _(span.attributes[otel_semantic::DB_SYSTEM]) - .must_equal('dynamodb') + _(span.attributes[otel_semantic::DB_SYSTEM]).must_equal('dynamodb') end end end diff --git a/instrumentation/aws_sdk/test/opentelemetry/instrumentation_test.rb b/instrumentation/aws_sdk/test/opentelemetry/instrumentation_test.rb index 7ffe3d33f9..ebee550c02 100644 --- a/instrumentation/aws_sdk/test/opentelemetry/instrumentation_test.rb +++ b/instrumentation/aws_sdk/test/opentelemetry/instrumentation_test.rb @@ -65,4 +65,11 @@ instrumentation.instance_variable_set(:@installed, false) end end + + describe '#install with default options' do + it 'with default options' do + _(instrumentation.config[:inject_messaging_context]).must_equal(false) + _(instrumentation.config[:enable_internal_instrumentation]).must_equal(false) + end + end end diff --git a/instrumentation/aws_sdk/test/opentelemetry/patches/telemetry_test.rb b/instrumentation/aws_sdk/test/opentelemetry/patches/telemetry_test.rb index 55fd838864..5fcd476b56 100644 --- a/instrumentation/aws_sdk/test/opentelemetry/patches/telemetry_test.rb +++ b/instrumentation/aws_sdk/test/opentelemetry/patches/telemetry_test.rb @@ -8,14 +8,11 @@ describe OpenTelemetry::Instrumentation::AwsSdk do describe 'Telemetry plugin' do - let(:instrumentation_gem_version) do - OpenTelemetry::Instrumentation::AwsSdk::Instrumentation.instance.gem_version - end + let(:instrumentation_instance) { OpenTelemetry::Instrumentation::AwsSdk::Instrumentation.instance } let(:otel_semantic) { OpenTelemetry::SemanticConventions::Trace } let(:exporter) { EXPORTER } let(:spans) { exporter.finished_spans } let(:otel_provider) { Aws::Telemetry::OTelProvider.new } - let(:stub_span) { spans.find { |s| s.name == 'Handler.StubResponses' } } let(:client_attrs) do { 'aws.region' => 'us-stubbed-1', @@ -24,31 +21,15 @@ } end - let(:stub_attrs) do - { - 'http.status_code' => '200', - 'net.protocol.name' => 'http', - 'net.protocol.version' => '1.1' - } - end - before do exporter.reset end describe 'Lambda' do let(:service_name) { 'Lambda' } - let(:service_uri) do - 'https://lambda.us-east-1.amazonaws.com/2015-03-31/functions/' - end - let(:client) do - Aws::Lambda::Client.new( - telemetry_provider: otel_provider, - stub_responses: true - ) - end + let(:service_uri) { 'https://lambda.us-east-1.amazonaws.com/2015-03-31/functions/' } + let(:client) { Aws::Lambda::Client.new(telemetry_provider: otel_provider, stub_responses: true) } let(:client_span) { spans.find { |s| s.name == 'Lambda.ListFunctions' } } - let(:internal_span) { spans.find { |s| s.name == 'Handler.NetHttp' } } let(:expected_client_attrs) do client_attrs.tap do |attrs| @@ -58,69 +39,62 @@ end end - let(:expected_stub_attrs) { stub_attrs.tap { |a| a['http.method'] = 'GET' } } - - let(:expected_internal_attrs) do - stub_attrs.tap do |attrs| - attrs['net.peer.name'] = 'lambda.us-east-1.amazonaws.com' - attrs['net.peer.port'] = '443' - end - end - - it 'creates spans with all the supplied parameters' do + it 'create a client span with all the supplied parameters' do skip unless TestHelper.telemetry_plugin?(service_name) client.list_functions _(client_span.name).must_equal('Lambda.ListFunctions') - _(stub_span.name).must_equal('Handler.StubResponses') _(client_span.kind).must_equal(:client) - _(stub_span.kind).must_equal(:internal) TestHelper.match_span_attrs(expected_client_attrs, client_span, self) - TestHelper.match_span_attrs(expected_stub_attrs, stub_span, self) - _(stub_span.parent_span_id).must_equal(client_span.span_id) end - it 'creates spans with all the non-stubbed parameters' do + it 'should have correct span attributes when error' do + skip unless TestHelper.telemetry_plugin?(service_name) + stub_request(:get, 'foo').to_return(status: 400) + + begin + client.list_functions + rescue Aws::Lambda::Errors::BadRequest + _(client_span.status.code).must_equal(2) + _(client_span.events[0].name).must_equal('exception') + end + end + + it 'creates internal spans when enabled' do skip unless TestHelper.telemetry_plugin?(service_name) - stub_request(:get, 'https://lambda.us-east-1.amazonaws.com/2015-03-31/functions/') + stub_request(:get, 'https://lambda.us-east-1.amazonaws.com/2015-03-31/functions/') client = Aws::Lambda::Client.new( telemetry_provider: otel_provider, credentials: Aws::Credentials.new('akid', 'secret'), region: 'us-east-1' ) + + instrumentation_instance.config[:enable_internal_instrumentation] = true client.list_functions - _(client_span.name).must_equal('Lambda.ListFunctions') + internal_span = spans.find { |s| s.name == 'Handler.NetHttp' } _(internal_span.name).must_equal('Handler.NetHttp') - _(client_span.kind).must_equal(:client) _(internal_span.kind).must_equal(:internal) - _(client_span.attributes['aws.region']).must_equal('us-east-1') - TestHelper.match_span_attrs(expected_internal_attrs, internal_span, self) - end - - it 'should have correct span attributes when error' do - skip unless TestHelper.telemetry_plugin?(service_name) - stub_request(:get, 'foo').to_return(status: 400) - - begin - client.list_functions - rescue Aws::Lambda::Errors::BadRequest - _(client_span.status.code).must_equal(2) - _(client_span.events[0].name).must_equal('exception') - _(internal_span.attributes['http.status_code']).must_equal('400') - end + TestHelper.match_span_attrs( + { + 'http.method' => 'GET', + 'http.status_code' => '200', + 'net.protocol.name' => 'http', + 'net.protocol.version' => '1.1', + 'net.peer.name' => 'lambda.us-east-1.amazonaws.com', + 'net.peer.port' => '443' + }, + internal_span, + self + ) + instrumentation_instance.config[:enable_internal_instrumentation] = false end end describe 'SNS' do let(:service_name) { 'SNS' } - let(:client) do - Aws::SNS::Client.new( - telemetry_provider: otel_provider, - stub_responses: true - ) - end + let(:client) { Aws::SNS::Client.new(telemetry_provider: otel_provider, stub_responses: true) } let(:client_span) { spans.find { |s| s.name.include?('SNS.Publish') } } let(:expected_client_attrs) do @@ -134,46 +108,31 @@ end end - let(:expected_stub_attrs) { stub_attrs.tap { |a| a['http.method'] = 'POST' } } - it 'creates spans with appropriate messaging attributes' do skip unless TestHelper.telemetry_plugin?(service_name) - client.publish( - message: 'msg', - topic_arn: 'arn:aws:sns:fake:123:TopicName' - ) + client.publish(message: 'msg', topic_arn: 'arn:aws:sns:fake:123:TopicName') _(client_span.name).must_equal('SNS.Publish.TopicName.Publish') _(client_span.kind).must_equal(:producer) - _(stub_span.name).must_equal('Handler.StubResponses') - _(stub_span.kind).must_equal(:internal) TestHelper.match_span_attrs(expected_client_attrs, client_span, self) - TestHelper.match_span_attrs(expected_stub_attrs, stub_span, self) - _(stub_span.parent_span_id).must_equal(client_span.span_id) end it 'creates a span that includes a phone number' do # skip if using aws-sdk version before phone_number supported (v2.3.18) - skip if Gem::Version.new('2.3.18') > instrumentation_gem_version + skip if Gem::Version.new('2.3.18') > instrumentation_instance.gem_version skip unless TestHelper.telemetry_plugin?(service_name) client.publish(message: 'msg', phone_number: '123456') _(client_span.name).must_equal('SNS.Publish.phone_number.Publish') - _(client_span.attributes[otel_semantic::MESSAGING_DESTINATION]) - .must_equal('phone_number') + _(client_span.attributes[otel_semantic::MESSAGING_DESTINATION]).must_equal('phone_number') end end describe 'SQS' do let(:service_name) { 'SQS' } - let(:client) do - Aws::SQS::Client.new( - telemetry_provider: otel_provider, - stub_responses: true - ) - end + let(:client) { Aws::SQS::Client.new(telemetry_provider: otel_provider, stub_responses: true) } let(:queue_url) { 'https://sqs.us-east-1.amazonaws.com/1/QueueName' } let(:expected_client_base_attrs) do client_attrs.tap do |attrs| @@ -185,8 +144,6 @@ end end - let(:expected_stub_attrs) { stub_attrs.tap { |a| a['http.method'] = 'POST' } } - describe '#SendMessage' do let(:client_span) { spans.find { |s| s.name.include?('SQS.SendMessage') } } let(:expected_client_attrs) do @@ -202,11 +159,7 @@ _(client_span.name).must_equal('SQS.SendMessage.QueueName.Publish') _(client_span.kind).must_equal(:producer) - _(stub_span.name).must_equal('Handler.StubResponses') - _(stub_span.kind).must_equal(:internal) TestHelper.match_span_attrs(expected_client_attrs, client_span, self) - TestHelper.match_span_attrs(expected_stub_attrs, stub_span, self) - _(stub_span.parent_span_id).must_equal(client_span.span_id) end end @@ -228,11 +181,7 @@ _(client_span.name).must_equal('SQS.SendMessageBatch.QueueName.Publish') _(client_span.kind).must_equal(:producer) - _(stub_span.name).must_equal('Handler.StubResponses') - _(stub_span.kind).must_equal(:internal) TestHelper.match_span_attrs(expected_client_attrs, client_span, self) - TestHelper.match_span_attrs(expected_stub_attrs, stub_span, self) - _(stub_span.parent_span_id).must_equal(client_span.span_id) end end @@ -252,11 +201,7 @@ _(client_span.name).must_equal('SQS.ReceiveMessage.QueueName.Receive') _(client_span.kind).must_equal(:consumer) - _(stub_span.name).must_equal('Handler.StubResponses') - _(stub_span.kind).must_equal(:internal) TestHelper.match_span_attrs(expected_client_attrs, client_span, self) - TestHelper.match_span_attrs(expected_stub_attrs, stub_span, self) - _(stub_span.parent_span_id).must_equal(client_span.span_id) end end @@ -275,13 +220,7 @@ end describe 'DynamoDB' do - let(:client) do - Aws::DynamoDB::Client.new( - telemetry_provider: otel_provider, - stub_responses: true - ) - end - let(:client_span) { TestHelper.find_span(spans, 'DynamoDB.ListTables') } + let(:client) { Aws::DynamoDB::Client.new(telemetry_provider: otel_provider, stub_responses: true) } let(:client_span) { spans.find { |s| s.name == 'DynamoDB.ListTables' } } it 'creates a span with dynamodb-specific attribute' do @@ -289,8 +228,7 @@ client.list_tables - _(client_span.attributes[otel_semantic::DB_SYSTEM]) - .must_equal('dynamodb') + _(client_span.attributes[otel_semantic::DB_SYSTEM]).must_equal('dynamodb') end end end