Add adapter name guard before telemetry call and make call in the current thread

Author: keithdoggett

build/config.teamcity.yml

@@ -11,6 +11,7 @@ connections:
       user: root
       requiressl: disable
       min_messages: warning
+      disable_cockroachdb_telemetry: true
     arunit_without_prepared_statements:
       database: activerecord_unittest
       host: localhost
@@ -19,10 +20,12 @@ connections:
       requiressl: disable
       min_messages: warning
       prepared_statements: false
+      disable_cockroachdb_telemetry: true
     arunit2:
       database: activerecord_unittest2
       host: localhost
       port: 26257
       user: root
       requiressl: disable
       min_messages: warning
+      disable_cockroachdb_telemetry: true

lib/active_record/connection_adapters/cockroachdb_adapter.rb

@@ -60,23 +60,22 @@ module CockroachDBConnectionPool
       def initialize(pool_config)
         super(pool_config)
         disable_telemetry = pool_config.db_config.configuration_hash[:disable_cockroachdb_telemetry]
-        return if disable_telemetry
-
-        Thread.new do
-          with_connection do |conn|
-            if conn.active?
-              begin
-                query = "SELECT crdb_internal.increment_feature_counter('ActiveRecord %d.%d')"
-                conn.execute(query % [ActiveRecord::VERSION::MAJOR, ActiveRecord::VERSION::MINOR])
-              rescue ActiveRecord::StatementInvalid
-                # The increment_feature_counter built-in is not supported on this
-                # CockroachDB version. Ignore.
-              rescue StandardError, NotImplementedError => e
-                conn.logger.warn "Unexpected error when incrementing feature counter: #{e}"
-              end
+        adapter = pool_config.db_config.configuration_hash[:adapter]
+        return if disable_telemetry || adapter != "cockroachdb"
+
+        with_connection do |conn|
+          if conn.active?
+            begin
+              query = "SELECT crdb_internal.increment_feature_counter('ActiveRecord %d.%d')"
+              conn.execute(query % [ActiveRecord::VERSION::MAJOR, ActiveRecord::VERSION::MINOR])
+            rescue ActiveRecord::StatementInvalid
+              # The increment_feature_counter built-in is not supported on this
+              # CockroachDB version. Ignore.
+            rescue StandardError => e
+              conn.logger.warn "Unexpected error when incrementing feature counter: #{e}"
             end
           end
-        end.join # close thread after execution
+        end
       end
     end
     ConnectionPool.prepend(CockroachDBConnectionPool)

test/cases/adapters/postgresql/postgresql_adapter_test.rb

@@ -15,6 +15,15 @@ def setup
         @connection_handler = ActiveRecord::Base.connection_handler
       end
 
+      def teardown
+        # use connection without follower_reads
+        database_config = { "adapter" => "cockroachdb", "database" => "activerecord_unittest" }
+        ar_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary")
+        database_config.update(ar_config.configuration_hash)
+
+        ActiveRecord::Base.establish_connection(database_config)
+      end
+
       def test_database_exists_returns_false_when_the_database_does_not_exist
         db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary")
         config = db_config.configuration_hash.dup
@@ -28,6 +37,19 @@ def test_database_exists_returns_true_when_the_database_exists
         assert ActiveRecord::ConnectionAdapters::CockroachDBAdapter.database_exists?(db_config.configuration_hash),
           "expected database #{db_config.database} to exist"
       end
+
+      def test_using_telemetry_builtin_connects_properly
+        database_config = { "adapter" => "cockroachdb", "database" => "activerecord_unittest" }
+        ar_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary")
+        database_config.update(ar_config.configuration_hash)
+        database_config[:disable_cockroachdb_telemetry] = false
+
+        ActiveRecord::Base.establish_connection(database_config)
+        conn = ActiveRecord::Base.connection
+        conn_config = conn.instance_variable_get("@config")
+
+        assert_equal(false, conn_config[:disable_cockroachdb_telemetry])
+      end
     end
   end
 end

test/config.yml

@@ -5,14 +5,17 @@ connections:
       host: localhost
       port: 26257
       user: root
+      disable_cockroachdb_telemetry: true
     arunit_without_prepared_statements:
       min_messages: warning
       prepared_statements: false
       host: localhost
       port: 26257
       user: root
+      disable_cockroachdb_telemetry: true
     arunit2:
       min_messages: warning
       host: localhost
       port: 26257
       user: root
+      disable_cockroachdb_telemetry: true