Skip to content
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.

Sign up for GitHub

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

feat: Add compatibility with env OTEL_SDK_DISABLED #1773

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+14 −0
Open

feat: Add compatibility with env OTEL_SDK_DISABLED #1773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ffac037
feat: disable SDK with env OTEL_SDK_DISABLED is defined as true
pedrofurtado Dec 4, 2024
0b2653d
feat: add tests
pedrofurtado Dec 4, 2024
3debcde
fix: revert test
pedrofurtado Dec 4, 2024
93d77cb
feat: fix rubocop
pedrofurtado Dec 4, 2024
5a7ae60
Update sdk/lib/opentelemetry/sdk.rb
pedrofurtado Dec 5, 2024
3ee1b46
feat: add test
pedrofurtado Dec 5, 2024
0e69b0e
Merge branch 'main' into feat/env-otel-sdk-disabled
pedrofurtado Dec 5, 2024
7bbf5c1
fix: fix rubocop
pedrofurtado Dec 5, 2024
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
5 changes: 5 additions & 0 deletions sdk/lib/opentelemetry/sdk.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ module SDK
# c.use_all
# end
def configure
if ENV['OTEL_SDK_DISABLED'] == 'true'
pedrofurtado marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

@robertlaurin is this the best place for us to check this?

Does it make sense to check that in the instance method instead? https://github.com/open-telemetry/opentelemetry-ruby/blob/main/sdk/lib/opentelemetry/sdk/configurator.rb#L144

Do we want to support being able to change the envar and re-initialize in a running program?

Copy link
Author

Choose a reason for hiding this comment

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

Here in our applications works like a charm in this place (using monkey patch).

The PR content was based on this comment: #1605 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that was my comment 😄

I reached out to @robertlaurin here since he designed this component and wanted to solicit his input about where this check should happen.

If a user is not using the DSL, they can still create an instance of the Configurator and call the configure method bypassing the envar.

They could also not use the configurator at all and assemble the tracer provider and resources themselves.

I just want to make sure we are hooking this into the appropriate scope, so folks are not surprised that it does not work as anticipated in specific scenarios; or if we are OK with this minimal implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I had similar questions @kaylareopelle to the ones you brought up

Copy link
Contributor

Choose a reason for hiding this comment

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

thank you! missed this! (link to comment)

OpenTelemetry.logger.warn 'Environment variable OTEL_SDK_DISABLED is defined as true. SDK is disabled.'
return
end

configurator = Configurator.new
yield configurator if block_given?
configurator.configure
Expand Down
9 changes: 9 additions & 0 deletions sdk/test/opentelemetry/sdk_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,13 @@
_(received_message).must_match(/unexpected configuration error due to unknown keyword: .*invalid_option/)
end
end

describe '#configure (sdk disabled)' do
it 'ignore configuration when sdk is disabled by env' do
config = OpenTelemetry::TestHelpers.with_env('OTEL_SDK_DISABLED' => 'true') do
OpenTelemetry::SDK.configure
end
_(config).must_equal nil
end
end
end
Loading