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] Move tracing integrations logging under a debug log #3785

Merged
merged 1 commit into from
Jul 18, 2024
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
4 changes: 4 additions & 0 deletions lib/datadog/core/diagnostics/environment_logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ def log_configuration!(prefix, data)
logger.info("DATADOG CONFIGURATION - #{prefix} - #{data}")
end

def log_debug!(prefix, data)
logger.debug("DATADOG CONFIGURATION - #{prefix} - #{data}")
Copy link
Contributor

Choose a reason for hiding this comment

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

s/CONFIGURATION/DEBUG/ ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The default logger format already includes the level:

D, [2024-07-16T15:54:39.386684 #327838] DEBUG -- datadog: [datadog] (dd-trace-rb/lib/datadog/core/diagnostics/environment_logger.rb:17:in `log_debug!') DATADOG CONFIGURATION - TRACING INTEGRATIONS - {"action_cable_analytics_enabled":"false"

so I'm not sure it's worth repeating in the string? Not a very strongly-held opinion though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant that inside the method, the assumption appears to be that this is debug logging for configuration, but is it possible for clients to log it something else at debug level which wouldn't be configuration?

Copy link
Member Author

Choose a reason for hiding this comment

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

The environment logger was built to gather and log configuration; when would it get used for something other than configuration? 🤔

end

def log_error!(prefix, type, error)
logger.warn("DATADOG ERROR - #{prefix} - #{type}: #{error}")
end
Expand Down
30 changes: 14 additions & 16 deletions lib/datadog/tracing/diagnostics/environment_logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ module EnvironmentLogger

def self.collect_and_log!(responses: nil)
if log?
env_data = EnvironmentCollector.collect_config!
log_configuration!('TRACING', env_data.to_json)
log_configuration!('TRACING', EnvironmentCollector.collect_config!.to_json)
log_debug!('TRACING INTEGRATIONS', EnvironmentCollector.collect_integrations_settings!.to_json)

if responses
err_data = EnvironmentCollector.collect_errors!(responses)
Expand All @@ -40,7 +40,6 @@ def collect_config!
sampling_rules: sampling_rules,
integrations_loaded: integrations_loaded,
partial_flushing_enabled: partial_flushing_enabled,
**instrumented_integrations_settings
}
end

Expand Down Expand Up @@ -128,6 +127,18 @@ def partial_flushing_enabled
!!Datadog.configuration.tracing.partial_flush.enabled
end

def collect_integrations_settings!
instrumented_integrations.each_with_object({}) do |(name, integration), result|
integration.configuration.to_h.each do |setting, value|
next if setting == :tracer # Skip internal objects

# Convert value to a string to avoid custom #to_json
# handlers possibly causing errors.
result[:"#{name}_#{setting}"] = value.to_s
end
end
end

private

def instrumented_integrations
Expand All @@ -139,19 +150,6 @@ def instrumented_integrations

Datadog.configuration.tracing.instrumented_integrations
end

# Capture all active integration settings into "integrationName_settingName: value" entries.
def instrumented_integrations_settings
instrumented_integrations.flat_map do |name, integration|
integration.configuration.to_h.flat_map do |setting, value|
next [] if setting == :tracer # Skip internal Ruby objects

# Convert value to a string to avoid custom #to_json
# handlers possibly causing errors.
[[:"integration_#{name}_#{setting}", value.to_s]]
end
end.to_h
end
end
end
end
Expand Down
40 changes: 30 additions & 10 deletions spec/datadog/tracing/diagnostics/environment_logger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,24 @@
end
end

context 'with integrations loaded' do
before { Datadog.configure { |c| c.tracing.instrument :http } }

it 'logs the integration settings as debug' do
expect(logger).to receive(:debug).with start_with('DATADOG CONFIGURATION - TRACING INTEGRATIONS') do |msg|
json = JSON.parse(msg.partition('- TRACING INTEGRATIONS -')[2].strip)
expect(json).to include(
'http_analytics_enabled' => 'false',
'http_analytics_sample_rate' => '1.0',
'http_distributed_tracing' => 'true',
'http_split_by_domain' => 'false',
)
end

collect_and_log!
end
end

context 'with agent error' do
subject(:collect_and_log!) { env_logger.collect_and_log!(responses: [response]) }

Expand Down Expand Up @@ -188,16 +206,6 @@
is_expected.to include integrations_loaded: end_with("@#{RUBY_VERSION}")
end

context 'with integration-specific settings' do
let(:options) { { service_name: 'my-http' } }

it { is_expected.to include integration_http_analytics_enabled: 'false' }
it { is_expected.to include integration_http_analytics_sample_rate: '1.0' }
it { is_expected.to include integration_http_service_name: 'my-http' }
it { is_expected.to include integration_http_distributed_tracing: 'true' }
it { is_expected.to include integration_http_split_by_domain: 'false' }
end

context 'with partial flushing enabled' do
before { expect(Datadog.configuration.tracing.partial_flush).to receive(:enabled).and_return(true) }

Expand All @@ -206,6 +214,18 @@
end
end

describe '#collect_integrations_settings!' do
subject(:collect_config!) { described_class.collect_integrations_settings! }

before { Datadog.configure { |c| c.tracing.instrument :http, service_name: 'my-http' } }

it { is_expected.to include http_analytics_enabled: 'false' }
it { is_expected.to include http_analytics_sample_rate: '1.0' }
it { is_expected.to include http_service_name: 'my-http' }
it { is_expected.to include http_distributed_tracing: 'true' }
it { is_expected.to include http_split_by_domain: 'false' }
end

describe '#collect_errors!' do
subject(:collect_errors!) { collector.collect_errors!([response]) }

Expand Down
Loading