Capture module versions (#588)

* Change n is None to not n

None types are falsey so we can shorten this expression to `if not module`.

* Use in instead of .find

`in` is more performant than find for search a string so use this instead.

* Simplify and combine sub path module logic

Do not include module.sub_paths as separate modules. Skip these except for
`newrelic.hooks`.

* Exclude standard lib/built-in modules

Previously, we were capturing standard library and built-in Python modules as plugins.
These are included with the version of Python the user installed and are not packages
that need to be captured so exclude these from the list.

* Capture module versions

* Fixup: remove pkg_resources check

* Ignore pylint function-redefined

* Check plugin version info in tests

newrelic/core/environment.py

@@ -20,6 +20,7 @@
 import os
 import platform
 import sys
+import sysconfig
 
 import newrelic
 from newrelic.common.system_info import (
@@ -178,41 +179,50 @@ def environment_settings():
     env.extend(dispatcher)
 
     # Module information.
+    purelib = sysconfig.get_path("purelib")
+    platlib = sysconfig.get_path("platlib")
 
     plugins = []
 
+    get_version = None
+    # importlib was introduced into the standard library starting in Python3.8.
+    if "importlib" in sys.modules and hasattr(sys.modules["importlib"], "metadata"):
+        get_version = sys.modules["importlib"].metadata.version
+    elif "pkg_resources" in sys.modules:
+
+        def get_version(name):  # pylint: disable=function-redefined
+            return sys.modules["pkg_resources"].get_distribution(name).version
+
     # Using any iterable to create a snapshot of sys.modules can occassionally
     # fail in a rare case when modules are imported in parallel by different
     # threads.
     #
     # TL;DR: Do NOT use an iterable on the original sys.modules to generate the
     # list
-
     for name, module in sys.modules.copy().items():
+        # Exclude lib.sub_paths as independent modules except for newrelic.hooks.
+        if "." in name and not name.startswith("newrelic.hooks."):
+            continue
         # If the module isn't actually loaded (such as failed relative imports
         # in Python 2.7), the module will be None and should not be reported.
-        if module is None:
+        if not module:
+            continue
+        # Exclude standard library/built-in modules.
+        # Third-party modules can be installed in either purelib or platlib directories.
+        # See https://docs.python.org/3/library/sysconfig.html#installation-paths.
+        if (
+            not hasattr(module, "__file__")
+            or not module.__file__
+            or not module.__file__.startswith(purelib)
+            or not module.__file__.startswith(platlib)
+        ):
             continue
 
-        if name.startswith("newrelic.hooks."):
-            plugins.append(name)
-
-        elif name.find(".") == -1 and hasattr(module, "__file__"):
-            # XXX This is disabled as it can cause notable overhead in
-            # pathalogical cases. Will be replaced with a new system
-            # where have a allowlist of packages we really want version
-            # information for and will work out on case by case basis
-            # how to extract that from the modules themselves.
-
-            # try:
-            #     if 'pkg_resources' in sys.modules:
-            #         version = pkg_resources.get_distribution(name).version
-            #         if version:
-            #             name = '%s (%s)' % (name, version)
-            # except Exception:
-            #     pass
-
-            plugins.append(name)
+        try:
+            version = get_version(name)
+            plugins.append("%s (%s)" % (name, version))
+        except Exception:
+            pass
 
     env.append(("Plugin List", plugins))
 

tests/agent_unittests/test_environment.py

@@ -12,8 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import pytest
 import sys
+
+import pytest
+
 from newrelic.core.environment import environment_settings
 
 
@@ -29,7 +31,7 @@ class Module(object):
 
 def test_plugin_list():
     # Let's pretend we fired an import hook
-    import newrelic.hooks.adapter_gunicorn
+    import newrelic.hooks.adapter_gunicorn  # noqa: F401
 
     environment_info = environment_settings()
 
@@ -41,6 +43,8 @@ def test_plugin_list():
 
     # Check that bogus plugins don't get reported
     assert "newrelic.hooks.newrelic" not in plugin_list
+    # Check that plugin that should get reported has version info.
+    assert "pytest (%s)" % (pytest.__version__) in plugin_list
 
 
 class NoIteratorDict(object):
@@ -62,7 +66,7 @@ def __contains__(self, *args, **kwargs):
 
 def test_plugin_list_uses_no_sys_modules_iterator(monkeypatch):
     modules = NoIteratorDict(sys.modules)
-    monkeypatch.setattr(sys, 'modules', modules)
+    monkeypatch.setattr(sys, "modules", modules)
 
     # If environment_settings iterates over sys.modules, an attribute error will be generated
     environment_info = environment_settings()
@@ -113,9 +117,7 @@ def test_plugin_list_uses_no_sys_modules_iterator(monkeypatch):
         ),
     ),
 )
-def test_uvicorn_dispatcher(
-    monkeypatch, loaded_modules, dispatcher, dispatcher_version, worker_version
-):
+def test_uvicorn_dispatcher(monkeypatch, loaded_modules, dispatcher, dispatcher_version, worker_version):
     # Let's pretend we load some modules
     for name, module in loaded_modules.items():
         monkeypatch.setitem(sys.modules, name, module)