Skip to content

feat: suppress internal spans with Faraday instrumentation #1506

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

Merged

Conversation

ezhang6811
Copy link
Contributor

Addresses #1502 for Faraday library instrumentation. Allow user to set suppress_internal_instrumentation flag when configuring Faraday instrumentation to suppress additional client spans generated from underlying Net::Http instrumentation.

Before merging, waiting for update on Github issue to see if internal span suppression can be made default behavior for both Faraday and AWS SDK.

@@ -19,6 +19,7 @@ To install the instrumentation, call `use` with the name of the instrumentation.
```ruby
OpenTelemetry::SDK.configure do |c|
c.use 'OpenTelemetry::Instrumentation::Faraday'
suppress_internal_instrumentation: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see a related PR from @jterapin renaming the option as enable_internal_instrumentation, which is language I think I prefer when naming the options.

Ideally these options would have symmetric naming and functionality.

#1533

Would you be amenable to changing the name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

Comment on lines 21 to 22
c.use 'OpenTelemetry::Instrumentation::Faraday'
enable_internal_instrumentation: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just noticed that this example is incorrect. The options need to be a parameter to use so there should be a comma here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks for catching that

@arielvalentin
Copy link
Collaborator

@ezhang6811 if the tests all pass I will merge and you should expect a Release this Tuesday 2025-06-03

@arielvalentin arielvalentin merged commit 2c11325 into open-telemetry:main Jun 3, 2025
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants