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.

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

[NO-TICKET] Fix breaking application boot due to concurrency issue in tracing #4303

Merged
merged 1 commit into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
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
18 changes: 15 additions & 3 deletions lib/datadog/tracing/contrib/extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ def configure(&block)
module Settings
InvalidIntegrationError = Class.new(StandardError)

# Used to avoid concurrency issues between registering integrations (e.g. mutation) and reporting the
# current integrations for logging/debugging/telemetry purposes (e.g. iteration) in the
# `@instrumented_integrations` hash.
#
# See https://github.com/DataDog/dd-trace-rb/issues/2851 for details on the original issue.
INSTRUMENTED_INTEGRATIONS_LOCK = Mutex.new

def self.included(base)
base.class_eval do
settings :contrib do
Expand Down Expand Up @@ -161,7 +168,10 @@ def instrument(integration_name, options = {}, &block)
configuration_name = options[:describes] || :default
filtered_options = options.reject { |k, _v| k == :describes }
integration.configure(configuration_name, filtered_options, &block)
instrumented_integrations[integration_name] = integration
INSTRUMENTED_INTEGRATIONS_LOCK.synchronize do
@instrumented_integrations ||= {}
@instrumented_integrations[integration_name] = integration
end

# Add to activation list
integrations_pending_activation << integration
Expand Down Expand Up @@ -192,14 +202,16 @@ def integrations_pending_activation
@integrations_pending_activation ||= Set.new
end

# This method is only for logging/debugging/telemetry purposes (e.g. iteration) in the
# `@instrumented_integrations` hash.
# @!visibility private
def instrumented_integrations
@instrumented_integrations ||= {}
INSTRUMENTED_INTEGRATIONS_LOCK.synchronize { (@instrumented_integrations&.dup || {}).freeze }
end

# @!visibility private
def reset!
instrumented_integrations.clear
INSTRUMENTED_INTEGRATIONS_LOCK.synchronize { @instrumented_integrations&.clear }
super
end

Expand Down
10 changes: 10 additions & 0 deletions spec/datadog/tracing/contrib/extensions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
before { registry.add(integration_name, integration) }
end

before do
allow(Datadog.logger).to receive(:warn)
end

context 'for' do
describe Datadog do
describe '#configure' do
Expand Down Expand Up @@ -244,6 +248,12 @@ def self.patch
end
end
end

describe '#instrumented_integrations' do
subject(:instrumented_integrations) { settings.instrumented_integrations }

it { is_expected.to be_frozen }
end
end
end
end
Expand Down
Loading