Repository: scoutapp/scout_apm_python
Pull request #348 (2019-11-04 13:36:03)

Remove exec calls from Elasticsearch instrument
Part of #342.

Also tidy up the import pattern, smiilar to Redis changes in #341, and fix index name tracking when not passed as a keyword argument.

The speed difference is smaller than I thought.

Before:

```
In [2]: %timeit Instrument().install(); Instrument().uninstall()
95.5 µs ± 2.07 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
```

After:
```
In [2]: %timeit Instrument().install(); Instrument().uninstall()
89.5 µs ± 674 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)
```

However the other benefits still stand - code being linted, etc.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9db38c2..aa56cab 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,8 @@
 
 - Track SQL Alchemy `executemany` calls as multi-queries
   ([PR #340](https://github.com/scoutapp/scout_apm_python/pull/340)).
+- Track Elasticsearch index name when it's not passed as a keyword argument
+  ([PR #348](https://github.com/scoutapp/scout_apm_python/pull/348)).
 
 ## [2.7.0] 2019-11-03
 
diff --git a/src/scout_apm/core/__init__.py b/src/scout_apm/core/__init__.py
index e2756d8..790ba85 100644
--- a/src/scout_apm/core/__init__.py
+++ b/src/scout_apm/core/__init__.py
@@ -9,7 +9,7 @@ from scout_apm.core import objtrace
 from scout_apm.core.config import scout_config
 from scout_apm.core.core_agent_manager import CoreAgentManager
 from scout_apm.core.instrument_manager import InstrumentManager
-from scout_apm.core.metadata import AppMetadata
+from scout_apm.core.metadata import report_app_metadata
 from scout_apm.core.socket import CoreAgentSocket
 
 logger = logging.getLogger(__name__)
@@ -41,7 +41,7 @@ def install(config=None):
     logger.debug("APM Launching on PID: %s", os.getpid())
     launched = CoreAgentManager().launch()
 
-    AppMetadata.report()
+    report_app_metadata()
     if launched:
         # Stop the thread to avoid running threads pre-fork
         CoreAgentSocket.instance().stop()
diff --git a/src/scout_apm/core/context.py b/src/scout_apm/core/context.py
new file mode 100644
index 0000000..e49bfdb
--- /dev/null
+++ b/src/scout_apm/core/context.py
@@ -0,0 +1,23 @@
+# coding=utf-8
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+import threading
+
+from scout_apm.core.tracked_request import TrackedRequest
+
+
+class ThreadLocalContext(object):
+    def __init__(self):
+        self._context = threading.local()
+
+    def get_tracked_request(self):
+        if not hasattr(self._context, "tracked_request"):
+            self._context.tracked_request = TrackedRequest()
+        return self._context.tracked_request
+
+    def clear_tracked_request(self, instance):
+        if getattr(self._context, "tracked_request", None) is instance:
+            del self._context.tracked_request
+
+
+context = ThreadLocalContext()
diff --git a/src/scout_apm/core/metadata.py b/src/scout_apm/core/metadata.py
index 0bd6ba2..9f723b6 100644
--- a/src/scout_apm/core/metadata.py
+++ b/src/scout_apm/core/metadata.py
@@ -2,7 +2,6 @@
 from __future__ import absolute_import, division, print_function, unicode_literals
 
 import datetime as dt
-import logging
 import sys
 from os import getpid
 
@@ -10,59 +9,51 @@ from scout_apm.core.commands import ApplicationEvent
 from scout_apm.core.config import scout_config
 from scout_apm.core.socket import CoreAgentSocket
 
-logger = logging.getLogger(__name__)
 
-
-class AppMetadata(object):
-    @classmethod
-    def report(cls):
-        event = ApplicationEvent(
+def report_app_metadata():
+    CoreAgentSocket.instance().send(
+        ApplicationEvent(
             event_type="scout.metadata",
-            event_value=cls.data(),
+            event_value=get_metadata(),
             source="Pid: " + str(getpid()),
             timestamp=dt.datetime.utcnow(),
         )
-        CoreAgentSocket.instance().send(event)
-
-    @classmethod
-    def data(cls):
-        try:
-            data = {
-                "language": "python",
-                "language_version": "{}.{}.{}".format(*sys.version_info[:3]),
-                # Deprecated: (see #327)
-                "version": "{}.{}.{}".format(*sys.version_info[:3]),
-                "server_time": dt.datetime.utcnow().isoformat() + "Z",
-                "framework": scout_config.value("framework"),
-                "framework_version": scout_config.value("framework_version"),
-                "environment": "",
-                "app_server": scout_config.value("app_server"),
-                "hostname": scout_config.value("hostname"),
-                "database_engine": "",  # Detected
-                "database_adapter": "",  # Raw
-                "application_name": "",  # Environment.application_name,
-                "libraries": cls.get_python_packages_versions(),
-                "paas": "",
-                "application_root": scout_config.value("application_root"),
-                "scm_subdirectory": scout_config.value("scm_subdirectory"),
-                "git_sha": scout_config.value("revision_sha"),
-            }
-        except Exception as e:
-            logger.debug("Exception in AppMetadata: %r", e)
-            data = {}
-
-        return data
-
-    @classmethod
-    def get_python_packages_versions(cls):
-        try:
-            import pkg_resources
-        except ImportError:
-            return []
-
-        return list(
-            sorted(
-                (distribution.project_name, distribution.version)
-                for distribution in pkg_resources.working_set
-            )
+    )
+
+
+def get_metadata():
+    data = {
+        "language": "python",
+        "language_version": "{}.{}.{}".format(*sys.version_info[:3]),
+        "server_time": dt.datetime.utcnow().isoformat() + "Z",
+        "framework": scout_config.value("framework"),
+        "framework_version": scout_config.value("framework_version"),
+        "environment": "",
+        "app_server": scout_config.value("app_server"),
+        "hostname": scout_config.value("hostname"),
+        "database_engine": "",
+        "database_adapter": "",
+        "application_name": "",
+        "libraries": get_python_packages_versions(),
+        "paas": "",
+        "application_root": scout_config.value("application_root"),
+        "scm_subdirectory": scout_config.value("scm_subdirectory"),
+        "git_sha": scout_config.value("revision_sha"),
+    }
+    # Deprecated - see #327:
+    data["version"] = data["language_version"]
+    return data
+
+
+def get_python_packages_versions():
+    try:
+        import pkg_resources
+    except ImportError:
+        return []
+
+    return list(
+        sorted(
+            (distribution.project_name, distribution.version)
+            for distribution in pkg_resources.working_set
         )
+    )
diff --git a/src/scout_apm/core/thread_local.py b/src/scout_apm/core/thread_local.py
deleted file mode 100644
index 51a8b98..0000000
--- a/src/scout_apm/core/thread_local.py
+++ /dev/null
@@ -1,45 +0,0 @@
-# coding=utf-8
-from __future__ import absolute_import, division, print_function, unicode_literals
-
-import threading
-
-
-class ThreadLocalSingleton(object):
-    _instance_lock = threading.Lock()
-
-    @classmethod
-    def instance(cls, *args, **kwargs):
-        with cls._instance_lock:
-            # The thread local dict doesn't exist at all.
-            # Need to create both it and the instance.
-            if not hasattr(cls, "_thread_lookup"):
-                cls.__new_instance(args, kwargs)
-                return cls._thread_lookup.instance
-
-            # Somehow we have the thread local dict, but failed
-            # to make the instance. Just reinitialize both.
-            if not hasattr(cls._thread_lookup, "instance"):
-                cls.__new_instance(args, kwargs)
-                return cls._thread_lookup.instance
-
-            # The instance is None, so we'll need to recreate it
-            # (and the thread local dict too).
-            if cls._thread_lookup.instance is None:
-                cls.__new_instance(args, kwargs)
-                return cls._thread_lookup.instance
-
-            # We made it through the checks, return the instance
-            # that we know exists.
-            return cls._thread_lookup.instance
-
-    # Releasing is under the same lock as creating,
-    # so it shouldn't step on each other.
-    def release(self):
-        with self.__class__._instance_lock:
-            if getattr(self.__class__._thread_lookup, "instance", None) is self:
-                self.__class__._thread_lookup.instance = None
-
-    @classmethod
-    def __new_instance(cls, args, kwargs):
-        cls._thread_lookup = threading.local()
-        cls._thread_lookup.instance = cls(*args, **kwargs)
diff --git a/src/scout_apm/core/tracked_request.py b/src/scout_apm/core/tracked_request.py
index c0f7250..9b8bc1d 100644
--- a/src/scout_apm/core/tracked_request.py
+++ b/src/scout_apm/core/tracked_request.py
@@ -11,12 +11,11 @@ from scout_apm.core.n_plus_one_call_set import NPlusOneCallSet
 from scout_apm.core.samplers.memory import get_rss_in_mb
 from scout_apm.core.samplers.thread import SamplersThread
 from scout_apm.core.socket import CoreAgentSocket
-from scout_apm.core.thread_local import ThreadLocalSingleton
 
 logger = logging.getLogger(__name__)
 
 
-class TrackedRequest(ThreadLocalSingleton):
+class TrackedRequest(object):
     """
     This is a container which keeps track of all module instances for a single
     request. For convenience they are made available as attributes based on
@@ -35,6 +34,12 @@ class TrackedRequest(ThreadLocalSingleton):
         "callset",
     )
 
+    @classmethod
+    def instance(cls):
+        from scout_apm.core.context import context
+
+        return context.get_tracked_request()
+
     def __init__(self):
         self.request_id = "req-" + str(uuid4())
         self.start_time = dt.datetime.utcnow()
@@ -112,12 +117,9 @@ class TrackedRequest(ThreadLocalSingleton):
                 CoreAgentSocket.instance().send(batch_command)
             SamplersThread.ensure_started()
 
-        # This can fail if the Tracked Request was created directly,
-        # not through instance()
-        try:
-            self.release()
-        except Exception:
-            pass
+        from scout_apm.core.context import context
+
+        context.clear_tracked_request(self)
 
     def _get_mem_delta(self):
         current_mem = get_rss_in_mb()
diff --git a/src/scout_apm/instruments/elasticsearch.py b/src/scout_apm/instruments/elasticsearch.py
index 9febdf6..b255134 100644
--- a/src/scout_apm/instruments/elasticsearch.py
+++ b/src/scout_apm/instruments/elasticsearch.py
@@ -7,6 +7,12 @@ import wrapt
 
 from scout_apm.core.tracked_request import TrackedRequest
 
+try:
+    from elasticsearch import Elasticsearch, Transport
+except ImportError:
+    Elasticsearch = None
+    Transport = None
+
 logger = logging.getLogger(__name__)
 
 
@@ -41,10 +47,7 @@ class Instrument(object):
     ]
 
     def installable(self):
-        try:
-            from elasticsearch import Elasticsearch  # noqa: F401
-            from elasticsearch import Transport  # noqa: F401
-        except ImportError:
+        if Elasticsearch is None:
             logger.info("Unable to import for Elasticsearch instruments")
             return False
         if self.installed:
@@ -73,65 +76,19 @@ class Instrument(object):
         self.uninstrument_transport()
 
     def instrument_client(self):
-        try:
-            from elasticsearch import Elasticsearch  # noqa: F401
-        except ImportError:
-            logger.info(
-                "Unable to import for Elasticsearch Client instruments. "
-                "Instrument install failed."
-            )
-            return False
-
-        for method_str in self.__class__.CLIENT_METHODS:
-            try:
-                code_str = """\
-@wrapt.decorator
-def wrapped_{method_str}(wrapped, instance, args, kwargs):
-    tracked_request = TrackedRequest.instance()
-    index = kwargs.get('index', 'Unknown')
-    if isinstance(index, (list, tuple)):
-        index = ','.join(index)
-    index = index.title()
-    name = '/'.join(['Elasticsearch', index, '{camel_name}'])
-    tracked_request.start_span(operation=name, ignore_children=True)
-
-    try:
-        return wrapped(*args, **kwargs)
-    finally:
-        tracked_request.stop_span()
-
-
-Elasticsearch.{method_str} = wrapped_{method_str}(Elasticsearch.{method_str})
-""".format(
-                    method_str=method_str,
-                    camel_name="".join(c.title() for c in method_str.split("_")),
-                )
-
-                exec(code_str)
-                logger.info("Instrumented Elasticsearch Elasticsearch.%s", method_str)
-
-            except Exception as e:
-                logger.warning(
-                    "Unable to instrument for Elasticsearch Elasticsearch.%s: %r",
-                    method_str,
-                    e,
-                )
-                return False
-        return True
+        for name in self.__class__.CLIENT_METHODS:
+            method = getattr(Elasticsearch, name, None)
+            if method is not None:
+                setattr(Elasticsearch, name, wrap_client_method(method))
 
     def uninstrument_client(self):
-        from elasticsearch import Elasticsearch
-
-        for method_str in self.__class__.CLIENT_METHODS:
-            setattr(
-                Elasticsearch,
-                method_str,
-                getattr(Elasticsearch, method_str).__wrapped__,
-            )
+        for name in self.__class__.CLIENT_METHODS:
+            method = getattr(Elasticsearch, name, None)
+            if method is not None:
+                setattr(Elasticsearch, name, method.__wrapped__)
 
     def instrument_transport(self):
         try:
-            from elasticsearch import Transport
 
             def _sanitize_name(name):
                 try:
@@ -200,6 +157,31 @@ Elasticsearch.{method_str} = wrapped_{method_str}(Elasticsearch.{method_str})
         return True
 
     def uninstrument_transport(self):
-        from elasticsearch import Transport
 
         Transport.perform_request = Transport.perform_request.__wrapped__
+
+
+@wrapt.decorator
+def wrap_client_method(wrapped, instance, args, kwargs):
+    def _get_index(index, *args, **kwargs):
+        return index
+
+    try:
+        index = _get_index(*args, **kwargs)
+    except TypeError:
+        index = "Unknown"
+    else:
+        if not index:
+            index = "Unknown"
+        if isinstance(index, (list, tuple)):
+            index = ",".join(index)
+    index = index.title()
+    camel_name = "".join(c.title() for c in wrapped.__name__.split("_"))
+    operation = "Elasticsearch/{}/{}".format(index, camel_name)
+    tracked_request = TrackedRequest.instance()
+    tracked_request.start_span(operation=operation, ignore_children=True)
+
+    try:
+        return wrapped(*args, **kwargs)
+    finally:
+        tracked_request.stop_span()
diff --git a/src/scout_apm/instruments/redis.py b/src/scout_apm/instruments/redis.py
index f2617ac..ed8b1e2 100644
--- a/src/scout_apm/instruments/redis.py
+++ b/src/scout_apm/instruments/redis.py
@@ -7,29 +7,26 @@ import wrapt
 
 from scout_apm.core.tracked_request import TrackedRequest
 
-logger = logging.getLogger(__name__)
-
-
-def import_Redis_and_Pipeline():
-    import redis  # noqa: F401
-
+try:
+    import redis
+except ImportError:
+    redis = None
+else:
     if redis.VERSION[0] >= 3:
-        from redis import Redis  # noqa: F401
-        from redis.client import Pipeline  # noqa: F401
+        from redis import Redis
+        from redis.client import Pipeline
     else:  # pragma: no cover
-        from redis import StrictRedis as Redis  # noqa: F401
-        from redis.client import BasePipeline as Pipeline  # noqa: F401
+        from redis import StrictRedis as Redis
+        from redis.client import BasePipeline as Pipeline
 
-    return Redis, Pipeline
+logger = logging.getLogger(__name__)
 
 
 class Instrument(object):
     installed = False
 
     def installable(self):
-        try:
-            Redis, Pipeline = import_Redis_and_Pipeline()
-        except ImportError:
+        if redis is None:
             logger.info("Unable to import for Redis instruments")
             return False
         if self.installed:
@@ -61,56 +58,52 @@ class Instrument(object):
         self.unpatch_pipeline()
 
     def patch_redis(self):
-        try:
-            Redis, _Pipeline = import_Redis_and_Pipeline()
+        @wrapt.decorator
+        def wrapped_execute_command(wrapped, instance, args, kwargs):
+            try:
+                op = args[0]
+            except (IndexError, TypeError):
+                op = "Unknown"
 
-            @wrapt.decorator
-            def wrapped_execute_command(wrapped, instance, args, kwargs):
-                try:
-                    op = args[0]
-                except (IndexError, TypeError):
-                    op = "Unknown"
+            tracked_request = TrackedRequest.instance()
+            tracked_request.start_span(operation="Redis/{}".format(op))
 
-                tracked_request = TrackedRequest.instance()
-                tracked_request.start_span(operation="Redis/{}".format(op))
-
-                try:
-                    return wrapped(*args, **kwargs)
-                finally:
-                    tracked_request.stop_span()
+            try:
+                return wrapped(*args, **kwargs)
+            finally:
+                tracked_request.stop_span()
 
+        try:
             Redis.execute_command = wrapped_execute_command(Redis.execute_command)
-
-        except Exception as e:
+        except Exception as exc:
             logger.warning(
-                "Unable to instrument for Redis StrictRedis.execute_command: %r", e
+                "Unable to instrument for Redis Redis.execute_command: %r",
+                exc,
+                exc_info=exc,
             )
 
     def unpatch_redis(self):
-        Redis, _Pipeline = import_Redis_and_Pipeline()
-
         Redis.execute_command = Redis.execute_command.__wrapped__
 
     def patch_pipeline(self):
-        try:
-            _Redis, Pipeline = import_Redis_and_Pipeline()
+        @wrapt.decorator
+        def wrapped_execute(wrapped, instance, args, kwargs):
+            tracked_request = TrackedRequest.instance()
+            tracked_request.start_span(operation="Redis/MULTI")
 
-            @wrapt.decorator
-            def wrapped_execute(wrapped, instance, args, kwargs):
-                tracked_request = TrackedRequest.instance()
-                tracked_request.start_span(operation="Redis/MULTI")
-
-                try:
-                    return wrapped(*args, **kwargs)
-                finally:
-                    tracked_request.stop_span()
+            try:
+                return wrapped(*args, **kwargs)
+            finally:
+                tracked_request.stop_span()
 
+        try:
             Pipeline.execute = wrapped_execute(Pipeline.execute)
-
-        except Exception as e:
-            logger.warning("Unable to instrument for Redis BasePipeline.execute: %r", e)
+        except Exception as exc:
+            logger.warning(
+                "Unable to instrument for Redis BasePipeline.execute: %r",
+                exc,
+                exc_info=exc,
+            )
 
     def unpatch_pipeline(self):
-        _Redis, Pipeline = import_Redis_and_Pipeline()
-
         Pipeline.execute = Pipeline.execute.__wrapped__


Test patch:
diff --git a/tests/integration/instruments/test_elasticsearch.py b/tests/integration/instruments/test_elasticsearch.py
index f40e4c5..b22bfac 100644
--- a/tests/integration/instruments/test_elasticsearch.py
+++ b/tests/integration/instruments/test_elasticsearch.py
@@ -5,15 +5,14 @@ import os
 from contextlib import contextmanager
 
 import elasticsearch
+import elasticsearch.exceptions
 import pytest
 
 from scout_apm.instruments.elasticsearch import Instrument
 from tests.compat import mock
-from tests.tools import pretend_package_unavailable
 
 # e.g. export ELASTICSEARCH_URL="http://localhost:9200/"
 ELASTICSEARCH_URL = os.environ.get("ELASTICSEARCH_URL")
-
 skip_if_elasticsearch_not_running = pytest.mark.skipif(
     ELASTICSEARCH_URL is None, reason="MongoDB isn't available"
 )
@@ -36,47 +35,112 @@ def es_with_scout():
 
 
 @skip_if_elasticsearch_not_running
-def test_search():
+def test_search(tracked_request):
     with es_with_scout() as es:
         es.search()
 
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Elasticsearch/Unknown/Search"
+
 
 @skip_if_elasticsearch_not_running
-def test_search_no_indexes_string():
+def test_search_arg_named_index(tracked_request):
+    with es_with_scout() as es, pytest.raises(elasticsearch.exceptions.NotFoundError):
+        es.search("myindex")
+
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Elasticsearch/Myindex/Search"
+
+
+@skip_if_elasticsearch_not_running
+def test_search_kwarg_named_index(tracked_request):
+    with es_with_scout() as es, pytest.raises(elasticsearch.exceptions.NotFoundError):
+        es.search(index="myindex")
+
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Elasticsearch/Myindex/Search"
+
+
+@skip_if_elasticsearch_not_running
+def test_search_arg_empty_index(tracked_request):
+    with es_with_scout() as es:
+        es.search("")
+
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Elasticsearch/Unknown/Search"
+
+
+@skip_if_elasticsearch_not_running
+def test_search_kwarg_empty_index(tracked_request):
     with es_with_scout() as es:
-        es.search(index="", body={"query": {"term": {"user": "kimchy"}}})
+        es.search(index="")
+
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Elasticsearch/Unknown/Search"
 
 
 @skip_if_elasticsearch_not_running
-def test_search_no_indexes_list():
+def test_search_kwarg_empty_index_list(tracked_request):
     with es_with_scout() as es:
-        es.search(index=[], body={"query": {"term": {"user": "kimchy"}}})
+        es.search(index=[])
+
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Elasticsearch/Unknown/Search"
 
 
 @skip_if_elasticsearch_not_running
-def test_perform_request_missing_url():
+def test_perform_request_missing_url(tracked_request):
     with es_with_scout() as es:
         with pytest.raises(TypeError):
             # Transport instrumentation doesn't crash if url is missing.
             # This raises a TypeError when calling the original method.
             es.transport.perform_request("GET", params={}, body=None)
 
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Elasticsearch/Unknown"
+
 
 @skip_if_elasticsearch_not_running
-def test_perform_request_bad_url():
+def test_perform_request_bad_url(tracked_request):
     with es_with_scout() as es:
         with pytest.raises(TypeError):
             # Transport instrumentation doesn't crash if url has the wrong type.
             # This raises a TypeError when calling the original method.
             es.transport.perform_request("GET", None, params={}, body=None)
 
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Elasticsearch/Unknown"
+
 
 @skip_if_elasticsearch_not_running
-def test_perform_request_unknown_url():
+def test_perform_request_unknown_url(tracked_request):
     with es_with_scout() as es:
         # Transport instrumentation doesn't crash if url is unknown.
         es.transport.perform_request("GET", "/_nodes", params={}, body=None)
 
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Elasticsearch/Unknown"
+
+
+@skip_if_elasticsearch_not_running
+def test_perform_request_known_url(tracked_request):
+    with es_with_scout() as es:
+        # Transport instrumentation doesn't crash if url is unknown.
+        es.transport.perform_request("GET", "/_count", params={}, body=None)
+
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Elasticsearch/Count"
+
 
 def test_installed():
     with es_with_scout():
@@ -91,31 +155,22 @@ def test_installable():
 
 
 def test_installable_no_elasticsearch_module():
-    with pretend_package_unavailable("elasticsearch"):
+    with mock.patch("scout_apm.instruments.elasticsearch.Elasticsearch", new=None):
         assert not instrument.installable()
 
 
 def test_install_no_elasticsearch_module():
-    with pretend_package_unavailable("elasticsearch"):
+    with mock.patch("scout_apm.instruments.elasticsearch.Elasticsearch", new=None):
         assert not instrument.install()
         assert not Instrument.installed
 
 
-def test_instrument_client_no_elasticsearch_module():
-    with pretend_package_unavailable("elasticsearch"):
-        assert not instrument.instrument_client()
-
-
-@mock.patch(
-    "scout_apm.instruments.elasticsearch.wrapt.decorator", side_effect=RuntimeError
-)
-def test_instrument_client_install_failure(mock_decorator):
-    assert not instrument.instrument_client()
-
-
-def test_instrument_transport_no_elasticsearch_module():
-    with pretend_package_unavailable("elasticsearch"):
-        assert not instrument.instrument_transport()
+def test_instrument_client_install_missing_attribute():
+    with mock.patch(
+        "scout_apm.instruments.elasticsearch.Elasticsearch"
+    ) as mock_elasticsearch:
+        del mock_elasticsearch.bulk
+    instrument.instrument_client()  # no crash
 
 
 @mock.patch(
diff --git a/tests/integration/instruments/test_redis.py b/tests/integration/instruments/test_redis.py
index e23d1e8..eaf1607 100644
--- a/tests/integration/instruments/test_redis.py
+++ b/tests/integration/instruments/test_redis.py
@@ -1,6 +1,7 @@
 # coding=utf-8
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import logging
 import os
 from contextlib import contextmanager
 
@@ -78,12 +79,12 @@ def test_installable():
 
 
 def test_installable_no_redis_module():
-    with pretend_package_unavailable("redis"):
+    with mock.patch("scout_apm.instruments.redis.redis", new=None):
         assert not instrument.installable()
 
 
 def test_install_no_redis_module():
-    with pretend_package_unavailable("redis"):
+    with mock.patch("scout_apm.instruments.redis.redis", new=None):
         assert not instrument.install()
         assert not Instrument.installed
 
@@ -93,9 +94,17 @@ def test_patch_redis_no_redis_module():
         instrument.patch_redis()  # doesn't crash
 
 
-@mock.patch("scout_apm.instruments.redis.wrapt.decorator", side_effect=RuntimeError)
-def test_patch_redis_install_failure(mock_decorator):
+@mock.patch("scout_apm.instruments.redis.Redis")
+def test_patch_redis_install_failure(mock_redis, caplog):
+    del mock_redis.execute_command
     instrument.patch_redis()  # doesn't crash
+    assert len(caplog.record_tuples) == 1
+    logger, level, message = caplog.record_tuples[0]
+    assert logger == "scout_apm.instruments.redis"
+    assert level == logging.WARNING
+    assert message.startswith(
+        "Unable to instrument for Redis Redis.execute_command: AttributeError"
+    )
 
 
 def test_patch_pipeline_no_redis_module():
@@ -103,10 +112,19 @@ def test_patch_pipeline_no_redis_module():
         instrument.patch_pipeline()  # doesn't crash
 
 
-@mock.patch("scout_apm.instruments.redis.wrapt.decorator", side_effect=RuntimeError)
-def test_patch_pipeline_install_failure(mock_decorator):
+@mock.patch("scout_apm.instruments.redis.Pipeline")
+def test_patch_pipeline_install_failure(mock_pipeline, caplog):
+    del mock_pipeline.execute
     instrument.patch_pipeline()  # doesn't crash
 
+    assert len(caplog.record_tuples) == 1
+    logger, level, message = caplog.record_tuples[0]
+    assert logger == "scout_apm.instruments.redis"
+    assert level == logging.WARNING
+    assert message.startswith(
+        "Unable to instrument for Redis BasePipeline.execute: AttributeError"
+    )
+
 
 def test_install_is_idempotent():
     with redis_with_scout():
diff --git a/tests/unit/core/test_metadata.py b/tests/unit/core/test_metadata.py
index 6967fac..4515105 100644
--- a/tests/unit/core/test_metadata.py
+++ b/tests/unit/core/test_metadata.py
@@ -1,16 +1,16 @@
 # coding=utf-8
 from __future__ import absolute_import, division, print_function, unicode_literals
 
-import py
+import pytest
 
-from scout_apm.core.metadata import AppMetadata
+from scout_apm.core.metadata import report_app_metadata
 from tests.compat import mock
 from tests.tools import pretend_package_unavailable
 
 
 @mock.patch("scout_apm.core.socket.CoreAgentSocket.send")
 def test_report_app_metadata(send):
-    AppMetadata().report()
+    report_app_metadata()
 
     assert send.call_count == 1
     (command,), kwargs = send.call_args
@@ -18,31 +18,16 @@ def test_report_app_metadata(send):
 
     message = command.message()
     assert message["ApplicationEvent"]["event_type"] == "scout.metadata"
-    # py.test is installed, since it's running tests right now.
-    assert ("py", py.version) in message["ApplicationEvent"]["event_value"]["libraries"]
-
-
-@mock.patch("scout_apm.core.socket.CoreAgentSocket.send")
-def test_report_app_metadata_error_getting_data(send):
-    with mock.patch(
-        "scout_apm.core.metadata.AppMetadata.get_python_packages_versions",
-        side_effect=RuntimeError,
-    ):
-        AppMetadata().report()
-
-    assert send.call_count == 1
-    (command,), kwargs = send.call_args
-    assert kwargs == {}
-
-    message = command.message()
-    assert message["ApplicationEvent"]["event_type"] == "scout.metadata"
-    assert message["ApplicationEvent"]["event_value"] == {}
+    data = message["ApplicationEvent"]["event_value"]
+    assert data["language"] == "python"
+    # pytest is installed, since it's running tests right now.
+    assert ("pytest", pytest.__version__) in data["libraries"]
 
 
 @mock.patch("scout_apm.core.socket.CoreAgentSocket.send")
 def test_report_app_metadata_no_pkg_resources(send):
     with pretend_package_unavailable("pkg_resources"):
-        AppMetadata().report()
+        report_app_metadata()
 
     assert send.call_count == 1
     (command,), kwargs = send.call_args