From 0b29c8e510d3f5a5b95747b7477099fd99a96c3f Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Sun, 26 Nov 2023 17:49:31 -0800 Subject: [PATCH 1/3] Capture Cockroach DB config in sentry-rails ActiveRecordSubscriber --- CHANGELOG.md | 1 + .../lib/sentry/rails/tracing/active_record_subscriber.rb | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccc65c59f..21ca7f079 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ - Respect custom `Delayed::Job.max_attempts` if it's defined [#2176](https://github.com/getsentry/sentry-ruby/pull/2176) - Fixed a bug where `Net::HTTP` instrumentation won't work for some IPv6 addresses [#2180](https://github.com/getsentry/sentry-ruby/pull/2180) - Allow non-string error message to be reported to sentry ([#2137](https://github.com/getsentry/sentry-ruby/pull/2137)) +- `sentry-rails` will now capture Cockroach DB adapter config into spans data [#2182](https://github.com/getsentry/sentry-ruby/pull/2182) ## 5.13.0 diff --git a/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb index 21eb1ea5c..716016f9e 100644 --- a/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb @@ -31,6 +31,10 @@ def self.subscribe! connection.pool.db_config.configuration_hash elsif connection.pool.respond_to?(:spec) connection.pool.spec.config + # CockroachDB pool shows up as NullPool, but we can grab + # it's configuration from the instance variable. + elsif connection.instance_variable_defined?(:@config) + connection.instance_variable_get(:@config) end next unless db_config From d67c265ab7f7b33c54f0d28e6a9d553330e24b77 Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Mon, 27 Nov 2023 12:21:12 -0800 Subject: [PATCH 2/3] Update changelog entry to Unreleased --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21ca7f079..27f8de40e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Bug Fixes - Network errors raised in `Sentry::HTTPTransport` will no longer be reported to Sentry [#2178](https://github.com/getsentry/sentry-ruby/pull/2178) +- `sentry-rails` will now capture Cockroach DB adapter config into spans data [#2182](https://github.com/getsentry/sentry-ruby/pull/2182) ## 5.14.0 @@ -35,7 +36,6 @@ - Respect custom `Delayed::Job.max_attempts` if it's defined [#2176](https://github.com/getsentry/sentry-ruby/pull/2176) - Fixed a bug where `Net::HTTP` instrumentation won't work for some IPv6 addresses [#2180](https://github.com/getsentry/sentry-ruby/pull/2180) - Allow non-string error message to be reported to sentry ([#2137](https://github.com/getsentry/sentry-ruby/pull/2137)) -- `sentry-rails` will now capture Cockroach DB adapter config into spans data [#2182](https://github.com/getsentry/sentry-ruby/pull/2182) ## 5.13.0 From 85f1ccf6fba666cf4c7ae92bef47ef923194bcf9 Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Mon, 27 Nov 2023 12:33:02 -0800 Subject: [PATCH 3/3] Unit test for the connection config --- .../tracing/active_record_subscriber_spec.rb | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/sentry-rails/spec/sentry/rails/tracing/active_record_subscriber_spec.rb b/sentry-rails/spec/sentry/rails/tracing/active_record_subscriber_spec.rb index 21dbb86e4..73485d0a4 100644 --- a/sentry-rails/spec/sentry/rails/tracing/active_record_subscriber_spec.rb +++ b/sentry-rails/spec/sentry/rails/tracing/active_record_subscriber_spec.rb @@ -64,6 +64,38 @@ expect(data["db.name"]).to include("db") expect(data["db.system"]).to eq("sqlite3") end + + it "records database configuration if connection.pool is not available" do + # Some adapters (like CockroachDB) don't provide ConnectionPool, + # so we have to grab the configuration off the Connection itself. + # See https://github.com/getsentry/sentry-ruby/issues/2110 + config = { adapter: "sqlite3", database: "dummy-database" } + allow_any_instance_of(ActiveRecord::ConnectionAdapters::SQLite3Adapter).to receive(:pool).and_return(nil) + allow_any_instance_of(ActiveRecord::ConnectionAdapters::SQLite3Adapter).to receive(:instance_variable_get).with(:@config).and_return(config) + + transaction = Sentry::Transaction.new(sampled: true, hub: Sentry.get_current_hub) + Sentry.get_current_scope.set_span(transaction) + + Post.all.to_a + + transaction.finish + + expect(transport.events.count).to eq(1) + + transaction = transport.events.first.to_hash + expect(transaction[:type]).to eq("transaction") + expect(transaction[:spans].count).to eq(1) + + span = transaction[:spans][0] + expect(span[:op]).to eq("db.sql.active_record") + expect(span[:description]).to eq("SELECT \"posts\".* FROM \"posts\"") + expect(span[:tags].key?(:cached)).to eq(false) + expect(span[:trace_id]).to eq(transaction.dig(:contexts, :trace, :trace_id)) + + data = span[:data] + expect(data["db.name"]).to eq("dummy-database") + expect(data["db.system"]).to eq("sqlite3") + end end context "when transaction is not sampled" do