Skip to content

feat: Suppress internal spans by default #1533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions instrumentation/aws_sdk/README.md
Original file line number Diff line number Diff line change
@@ -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`
Comment on lines +41 to +42
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you are removing this option altogether resulting in a breaking change.

If you want to treat it as deprecated, as opposed to removed, I would recommend leaving the option present and giving precedence to the new option.


## Integration with SDK V3's Telemetry support
AWS SDK for Ruby V3 added support for Observability which includes a new configuration,
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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']
Original file line number Diff line number Diff line change
@@ -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
11 changes: 4 additions & 7 deletions instrumentation/aws_sdk/test/opentelemetry/handler_test.rb
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Loading
Oops, something went wrong.
Loading
Oops, something went wrong.