-
Notifications
You must be signed in to change notification settings - Fork 209
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
base: main
Are you sure you want to change the base?
Conversation
…metry-ruby-contrib into update-suppress-config
Was this or the related Issue discussed at a SIG? seems like i'm missing some context, at the very least I have some ambivalence about multiple config options that do the same thing. |
I have not attended SIG so most likely not. This PR drops the |
👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the |
👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the |
@@ -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? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kaylareopelle I realize this is a common pattern we have in the code base already but I would recommend we look into refactoring cases like this where we create separate mixins for "untraced" instead of evaluating this logic with every request.
@@ -65,6 +69,15 @@ def gem_version | |||
|
|||
private | |||
|
|||
def resolve_config(config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are we providing backward compatability for the deprecated option?
Is it not supposed to work at all?
#1502
Description: The
aws_sdk
instrumentation now suppresses internal spans by default. This ensures that the additional low-level instrumentation does not generate additional costs for customers unknowingly.The following work has been done to accommodate:
:suppress_internal_instrumentation
config:enable_internal_instrumentation
which is set false by defaultTo enable internal spans, users will have explicitly configure as follows: