dd-trace-rb
dd-trace-rb copied to clipboard
ActiveRecord `describes:` with an invalid value logs full database config
Current behaviour If I configure the active_record tracing like so:
Datadog.configure do |c|
c.tracing.instrument :active_record, describes: 'some invalid string' do |gadget_db|
gadget_db.service_name = gadget_db_service_name
end
end
It understandably logs an error when starting up:
E, [2024-03-14T03:15:30.910202 #1816] ERROR -- ddtrace: [ddtrace] (/app/lib/datadog/tracing/contrib/active_record/configuration/resolver.rb:87:in `rescue in parse_matcher') Failed to resolve ActiveRecord configuration key "some invalid string". Cause: URI::InvalidURIError bad URI(is not URI?): "some invalid string" Source: /usr/local/lib/ruby/3.0.0/uri/rfc3986_parser.rb:67:in `split'
However, as the application continues it will start logging the following:
E, [2024-03-14T03:15:30.912526 #1816] ERROR -- ddtrace: [ddtrace] (/app/lib/datadog/tracing/contrib/active_record/configuration/resolver.rb:68:in `rescue in resolve') Failed to resolve ActiveRecord configuration key {:prepared_statements=>false, :adapter=>"mysql2", :username=>"root", :password=>"root", :port=>3306, :database=>"mysql", :host=>"mysql", :flags=>2}. Cause: NoMethodError undefined method `none?' for true:TrueClass Source: /app/lib/datadog/tracing/contrib/active_record/configuration/resolver.rb:61:in `block in resolve'
This is problematic for two reasons:
- It seems like it registered a configuration with matcher
true
. I believe this is because of this code, which will rescue an error and returntrue
(becauseDatadog.logger.error
returns true). - Because of the invalid matcher, we end up logging the database configuration including the password.
Expected behaviour
- Continue to log the resolve error at startup and then not include the bad configuration in matchers.
- Do not ever blindly log ActiveRecord matchers (which will contain the db password).
Steps to reproduce This patch adds a test case that will produce the above logs:
diff --git a/spec/datadog/tracing/contrib/active_record/multi_db_spec.rb b/spec/datadog/tracing/contrib/active_record/multi_db_spec.rb
index dca6f8bbd..9e802e1c1 100644
--- a/spec/datadog/tracing/contrib/active_record/multi_db_spec.rb
+++ b/spec/datadog/tracing/contrib/active_record/multi_db_spec.rb
@@ -215,6 +215,22 @@ RSpec.describe 'ActiveRecord multi-database implementation' do
end
end
+ context 'an invalid value' do
+ context 'for a typical server' do
+ before do
+ Datadog.configure do |c|
+ c.tracing.instrument :active_record, describes: 'some invalid string' do |gadget_db|
+ gadget_db.service_name = gadget_db_service_name
+ end
+ end
+ end
+
+ it do
+ count
+ end
+ end
+ end
+
context 'a Hash that describes a connection' do
before do
widget_db_connection_hash = { adapter: 'sqlite3', database: ':memory:' }
How does ddtrace
help you?
Environment
- ddtrace version: 1.20.0
-
Configuration block (
Datadog.configure ...
): See above patch - Ruby version: All
- Operating system: Linux
- Relevant library versions:
Update: It seems that any incorrect value causes logs to leak.
Example: If I specify describes: :replica2
and there is no replica2
entry in datadog.yml
I will get this error:
[2024-03-13T22:17:38] ERROR Datadog: Failed to resolve ActiveRecord configuration key :replica2. Cause: ActiveRecord::AdapterNotSpecified The `replica2` database is not configured for the `development` environment.
Available databases configurations are:
default
development: primary, replica
production: primary, replica
test: primary, replica
Source: /Users/peterhamilton/.rbenv/versions/3.0.6/lib/ruby/gems/3.0.0/gems/activerecord-6.1.7.6/lib/active_record/database_configurations.rb:194:in `resolve_symbol_connection'
And then when my code is running I will get:
[2024-03-13T22:17:45] ERROR Datadog: Failed to resolve ActiveRecord configuration key {:prepared_statements=>false, :adapter=>"mysql2", :encoding=>"utf8mb4", :collation=>"utf8mb4_unicode_ci", :pool=>2, :database=>"railsdev", :host=>"127.0.0.1", :username=>"root", :port=>"3306", :password=>"", :variables=>{"sql_mode"=>"NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,STRICT_ALL_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION"}, :flags=>2}. Cause: NoMethodError undefined method `none?' for true:TrueClass Source: /Users/peterhamilton/.rbenv/versions/3.0.6/lib/ruby/gems/3.0.0/gems/ddtrace-1.13.1/lib/datadog/tracing/contrib/active_record/configuration/resolver.rb:61:in `block in resolve'
Again, passwords leaking into logs is the main problem here.
@hamiltop Thanks for reporting! I will take care of this!