Skip to content
Open
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
17 changes: 14 additions & 3 deletions lib/datadog/core/configuration/config_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ def key?(name)
alias_method :include?, :key?
alias_method :member?, :key?

# This method will be used by datadog-ci-rb once it will bump its minimum dependancy of dd-trace-rb to 2.27.0.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# This method will be used by datadog-ci-rb once it will bump its minimum dependancy of dd-trace-rb to 2.27.0.
# This method will be used by datadog-ci-rb after it increases minimum required version of dd-trace-rb to 2.27.0.

# Currently, datadog-ci-rb is compatible with all version of dd-trace-rb > 2.4.0, and we introduced ConfigHelper in 2.21.0.
# For version of dd-trace-rb > 2.21.0 and < 2.27.0, we still monkey-patch DATADOG_ENV and use ConfigHelper
# (although checks that envs are registered will be ignored as we were not supporting datadog-ci-rb yet),
# but we have to use ConfigHelper#instance_variable_get(:@source_env) to set environment variables, as ConfigHelper#[]= was not defined.
# Once datadog-ci-rb will bump its minimum dependancy of dd-trace-rb to the one that implements ConfigHelper#[]= (2.27.0),
# we can stop using ConfigHelper#instance_variable_get(:@source_env) and use ConfigHelper#[]= instead.
# datadog-ci-rb do not plan to bump its minimum dependancy of dd-trace-rb to 2.27.0 in its next release, so we cannot use ConfigHelper#[]= for now.
def []=(name, value)
Copy link
Member

Choose a reason for hiding this comment

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

I am confused why this method is being added and immediately stated to be unused in the upcoming datadog-ci release, can a comment be added explaining this in more detail please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

datadog-ci-rb is compatible with dd-trace-rb 2.4.0. To make config inversion work on datadog-ci-rb, we monkey-patch DATADOG_ENV, which was introduced in dd-trace-rb 2.21.0 IIRC. For version lower that 2.21.0, DATADOG_ENV will be equal to ENV, but versions above that is a ConfigHelper instance. It is not planned to bump dd-trace-rb dependency in datadog-ci-rb yet, but I added this so it is future proof, and can be used to simplify the code once datadog-ci-rb bumps dd-trace-rb dependency

@source_env[name] = value
end

# Returns the environment variable value if the environment variable is a supported Datadog configuration (starts with DD_ or OTEL_)
# or if it is not a Datadog configuration. Otherwise, it returns nil.
#
Expand All @@ -53,9 +65,8 @@ def key?(name)
# @return [String, nil] The environment variable value
# @raise [RuntimeError] if the configuration is not supported
def get_environment_variable(name, default_value = nil, source_env: @source_env)
# datadog-ci-rb is using dd-trace-rb config DSL, which uses this method.
# Until we've correctly implemented support for datadog-ci-rb, we disable config inversion if ci is enabled.
if !defined?(::Datadog::CI) &&
# Skip if datadog-ci-rb is loaded but does not support config inversion yet
if (!defined?(::Datadog::CI) || defined?(::Datadog::CI::Configuration::SUPPORTED_CONFIGURATION_NAMES)) &&
(name.start_with?('DD_', 'OTEL_') || @alias_to_canonical[name]) &&
!@supported_configurations.include?(name)
if defined?(@raise_on_unknown_env_var) && @raise_on_unknown_env_var # Only enabled for tests!
Expand Down
7 changes: 5 additions & 2 deletions lib/datadog/core/configuration/deprecations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ module Datadog
module Core
module Configuration
module Deprecations
LOG_DEPRECATIONS_ONLY_ONCE = Datadog::Core::Utils::OnlyOnce.new
# Hash of OnlyOnce instances, as we may call log_deprecations_from_all_sources from datadog-ci-rb too with different deprecations set
LOG_DEPRECATIONS_ONLY_ONCE = {}

def self.log_deprecations_from_all_sources(logger, deprecations: DEPRECATIONS, alias_to_canonical: ALIAS_TO_CANONICAL)
LOG_DEPRECATIONS_ONLY_ONCE.run do
# This way of initializing the `OnlyOnce` is not thread-safe but that's OK here
LOG_DEPRECATIONS_ONLY_ONCE[deprecations] ||= Datadog::Core::Utils::OnlyOnce.new
LOG_DEPRECATIONS_ONLY_ONCE[deprecations].run do
Comment on lines +16 to +17
Copy link
Member

Choose a reason for hiding this comment

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

Note: The OnlyOnce is designed to be thread safe; when used in this manner it's not going to be thread-safe. I think in practice that's fine (I don't think we need to rely on its thread-safety properties here), but perhaps it's worth leaving a comment along the lines of

# This way of initializing the `OnlyOnce` is not thread-safe but that's OK here
LOG_DEPRECATIONS_ONLY_ONCE[deprecations] ||= Datadog::Core::Utils::OnlyOnce.new

log_deprecated_environment_variables(logger, ENV, 'environment', deprecations, alias_to_canonical)
customer_config = StableConfig.configuration.dig(:local, :config)
log_deprecated_environment_variables(logger, customer_config, 'local', deprecations, alias_to_canonical) if customer_config
Expand Down
2 changes: 2 additions & 0 deletions sig/datadog/core/configuration/config_helper.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ module Datadog
alias include? key?
alias member? key?

def []=: (String name, String value) -> void

def get_environment_variable: (String name, ?String? default_value, ?source_env: env_hash) -> String?

def resolve_env: (String name, ?source_env: env_hash) -> String
Expand Down
2 changes: 1 addition & 1 deletion sig/datadog/core/configuration/deprecations.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module Datadog
module Core
module Configuration
module Deprecations
LOG_DEPRECATIONS_ONLY_ONCE: Datadog::Core::Utils::OnlyOnce
LOG_DEPRECATIONS_ONLY_ONCE: Hash[Set[String], Datadog::Core::Utils::OnlyOnce]

def self.log_deprecations_from_all_sources: (Datadog::Core::Logger logger, ?deprecations: Set[String], ?alias_to_canonical: Hash[String, String]) -> void

Expand Down
4 changes: 2 additions & 2 deletions spec/datadog/core/configuration/config_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@
end
end

context 'when Datadog::CI is defined' do
context 'when Datadog::CI is defined but version is too low' do
before do
stub_const('Datadog::CI', Module.new)
stub_const('Datadog::CI::VERSION::STRING', '1.26.0')
end

subject do
Expand Down
2 changes: 1 addition & 1 deletion spec/datadog/core/configuration/deprecations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
end

before do
described_class.const_get('LOG_DEPRECATIONS_ONLY_ONCE').send(:reset_ran_once_state_for_tests)
described_class.const_get('LOG_DEPRECATIONS_ONLY_ONCE').each_value { |only_once| only_once.send(:reset_ran_once_state_for_tests) }
end

context 'when deprecated env is set in ENV' do
Expand Down
Loading