Fix database instance metric bug (#905)

* Remove enable_datastore_instance_feature

This was added in 2016 when the database instance feature was first developed. It
appears to be a method of gating this feature internally within the agent at the time
that it was implemented. However, it is not needed now and database instrumentations
that don't call this are actually broken in that the metric that is used to create the
service map (namely `Datastore/instance/MySQL/<host>/<port>`) does not get created due
to not calling this enable feature function.

* Rename cross agent test

* Add Database/instance metric check

Author: hmstepanek

newrelic/api/database_trace.py

@@ -44,11 +44,6 @@ def register_database_client(
     dbapi2_module._nr_explain_query = explain_query
     dbapi2_module._nr_explain_stmts = explain_stmts
     dbapi2_module._nr_instance_info = instance_info
-    dbapi2_module._nr_datastore_instance_feature_flag = False
-
-
-def enable_datastore_instance_feature(dbapi2_module):
-    dbapi2_module._nr_datastore_instance_feature_flag = True
 
 
 class DatabaseTrace(TimeTrace):
@@ -153,12 +148,7 @@ def finalize_data(self, transaction, exc=None, value=None, tb=None):
 
         if instance_enabled or db_name_enabled:
 
-            if (
-                self.dbapi2_module
-                and self.connect_params
-                and self.dbapi2_module._nr_datastore_instance_feature_flag
-                and self.dbapi2_module._nr_instance_info is not None
-            ):
+            if self.dbapi2_module and self.connect_params and self.dbapi2_module._nr_instance_info is not None:
 
                 instance_info = self.dbapi2_module._nr_instance_info(*self.connect_params)
 

newrelic/hooks/database_asyncpg.py

@@ -12,11 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from newrelic.api.database_trace import (
-    DatabaseTrace,
-    enable_datastore_instance_feature,
-    register_database_client,
-)
+from newrelic.api.database_trace import DatabaseTrace, register_database_client
 from newrelic.api.datastore_trace import DatastoreTrace
 from newrelic.common.object_wrapper import ObjectProxy, wrap_function_wrapper
 
@@ -43,7 +39,6 @@ def instance_info(cls, args, kwargs):
     quoting_style="single+dollar",
     instance_info=PostgresApi.instance_info,
 )
-enable_datastore_instance_feature(PostgresApi)
 
 
 class ProtocolProxy(ObjectProxy):
@@ -94,9 +89,7 @@ async def query(self, query, *args, **kwargs):
 
     async def prepare(self, stmt_name, query, *args, **kwargs):
         with DatabaseTrace(
-            "PREPARE {stmt_name} FROM '{query}'".format(
-                stmt_name=stmt_name, query=query
-            ),
+            "PREPARE {stmt_name} FROM '{query}'".format(stmt_name=stmt_name, query=query),
             dbapi2_module=PostgresApi,
             connect_params=getattr(self, "_nr_connect_params", None),
             source=self.__wrapped__.prepare,
@@ -131,9 +124,7 @@ def proxy_protocol(wrapped, instance, args, kwargs):
 def wrap_connect(wrapped, instance, args, kwargs):
     host = port = database_name = None
     if "addr" in kwargs:
-        host, port, database_name = PostgresApi._instance_info(
-            kwargs["addr"], None, kwargs.get("params")
-        )
+        host, port, database_name = PostgresApi._instance_info(kwargs["addr"], None, kwargs.get("params"))
 
     with DatastoreTrace(
         PostgresApi._nr_database_product,

newrelic/hooks/database_mysqldb.py

@@ -14,99 +14,114 @@
 
 import os
 
-from newrelic.api.database_trace import (enable_datastore_instance_feature,
-        DatabaseTrace, register_database_client)
+from newrelic.api.database_trace import DatabaseTrace, register_database_client
 from newrelic.api.function_trace import FunctionTrace
 from newrelic.api.transaction import current_transaction
 from newrelic.common.object_names import callable_name
 from newrelic.common.object_wrapper import wrap_object
+from newrelic.hooks.database_dbapi2 import ConnectionFactory as DBAPI2ConnectionFactory
+from newrelic.hooks.database_dbapi2 import ConnectionWrapper as DBAPI2ConnectionWrapper
 
-from newrelic.hooks.database_dbapi2 import (ConnectionWrapper as
-        DBAPI2ConnectionWrapper, ConnectionFactory as DBAPI2ConnectionFactory)
 
 class ConnectionWrapper(DBAPI2ConnectionWrapper):
-
     def __enter__(self):
         transaction = current_transaction()
         name = callable_name(self.__wrapped__.__enter__)
         with FunctionTrace(name, source=self.__wrapped__.__enter__):
-           cursor = self.__wrapped__.__enter__()
+            cursor = self.__wrapped__.__enter__()
 
         # The __enter__() method of original connection object returns
         # a new cursor instance for use with 'as' assignment. We need
         # to wrap that in a cursor wrapper otherwise we will not track
         # any queries done via it.
 
-        return self.__cursor_wrapper__(cursor, self._nr_dbapi2_module,
-               self._nr_connect_params, None)
+        return self.__cursor_wrapper__(cursor, self._nr_dbapi2_module, self._nr_connect_params, None)
 
     def __exit__(self, exc, value, tb):
         transaction = current_transaction()
         name = callable_name(self.__wrapped__.__exit__)
         with FunctionTrace(name, source=self.__wrapped__.__exit__):
             if exc is None:
-                with DatabaseTrace('COMMIT', self._nr_dbapi2_module, self._nr_connect_params, source=self.__wrapped__.__exit__):
+                with DatabaseTrace(
+                    "COMMIT", self._nr_dbapi2_module, self._nr_connect_params, source=self.__wrapped__.__exit__
+                ):
                     return self.__wrapped__.__exit__(exc, value, tb)
             else:
-                with DatabaseTrace('ROLLBACK', self._nr_dbapi2_module, self._nr_connect_params, source=self.__wrapped__.__exit__):
+                with DatabaseTrace(
+                    "ROLLBACK", self._nr_dbapi2_module, self._nr_connect_params, source=self.__wrapped__.__exit__
+                ):
                     return self.__wrapped__.__exit__(exc, value, tb)
 
+
 class ConnectionFactory(DBAPI2ConnectionFactory):
 
     __connection_wrapper__ = ConnectionWrapper
 
+
 def instance_info(args, kwargs):
-    def _bind_params(host=None, user=None, passwd=None, db=None, port=None,
-            unix_socket=None, conv=None, connect_timeout=None, compress=None,
-            named_pipe=None, init_command=None, read_default_file=None,
-            read_default_group=None, *args, **kwargs):
-        return (host, port, db, unix_socket,
-                read_default_file, read_default_group)
+    def _bind_params(
+        host=None,
+        user=None,
+        passwd=None,
+        db=None,
+        port=None,
+        unix_socket=None,
+        conv=None,
+        connect_timeout=None,
+        compress=None,
+        named_pipe=None,
+        init_command=None,
+        read_default_file=None,
+        read_default_group=None,
+        *args,
+        **kwargs
+    ):
+        return (host, port, db, unix_socket, read_default_file, read_default_group)
 
     params = _bind_params(*args, **kwargs)
     host, port, db, unix_socket, read_default_file, read_default_group = params
     explicit_host = host
 
     port_path_or_id = None
     if read_default_file or read_default_group:
-        host = host or 'default'
-        port_path_or_id = 'unknown'
+        host = host or "default"
+        port_path_or_id = "unknown"
     elif not host:
-        host = 'localhost'
+        host = "localhost"
 
-    if host == 'localhost':
+    if host == "localhost":
         # precedence: explicit -> cnf (if used) -> env -> 'default'
-        port_path_or_id = (unix_socket or
-                port_path_or_id or
-                os.getenv('MYSQL_UNIX_PORT', 'default'))
+        port_path_or_id = unix_socket or port_path_or_id or os.getenv("MYSQL_UNIX_PORT", "default")
     elif explicit_host:
         # only reach here if host is explicitly passed in
         port = port and str(port)
         # precedence: explicit -> cnf (if used) -> env -> '3306'
-        port_path_or_id = (port or
-                port_path_or_id or
-                os.getenv('MYSQL_TCP_PORT', '3306'))
+        port_path_or_id = port or port_path_or_id or os.getenv("MYSQL_TCP_PORT", "3306")
 
     # There is no default database if omitted from the connect params
     # In this case, we should report unknown
-    db = db or 'unknown'
+    db = db or "unknown"
 
     return (host, port_path_or_id, db)
 
-def instrument_mysqldb(module):
-    register_database_client(module, database_product='MySQL',
-            quoting_style='single+double', explain_query='explain',
-            explain_stmts=('select',), instance_info=instance_info)
 
-    enable_datastore_instance_feature(module)
+def instrument_mysqldb(module):
+    register_database_client(
+        module,
+        database_product="MySQL",
+        quoting_style="single+double",
+        explain_query="explain",
+        explain_stmts=("select",),
+        instance_info=instance_info,
+    )
 
-    wrap_object(module, 'connect', ConnectionFactory, (module,))
+    wrap_object(module, "connect", ConnectionFactory, (module,))
 
     # The connect() function is actually aliased with Connect() and
     # Connection, the later actually being the Connection type object.
     # Instrument Connect(), but don't instrument Connection in case that
     # interferes with direct type usage. If people are using the
     # Connection object directly, they should really be using connect().
 
-    if hasattr(module, 'Connect'):
-        wrap_object(module, 'Connect', ConnectionFactory, (module,))
+    if hasattr(module, "Connect"):
+        wrap_object(module, "Connect", ConnectionFactory, (module,))