Don't show urllib3 when requests is installed on logfire inspect (

#744)

logfire/_internal/cli.py

@@ -5,6 +5,7 @@
 import argparse
 import functools
 import importlib
+import importlib.metadata
 import importlib.util
 import logging
 import platform
@@ -107,7 +108,9 @@ def parse_clean(args: argparse.Namespace) -> None:
 
 # TODO(Marcelo): Automatically check if this list should be updated.
 # NOTE: List of packages from https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/instrumentation.
+STANDARD_LIBRARY_PACKAGES = {'urllib', 'sqlite3'}
 OTEL_PACKAGES: set[str] = {
+    *STANDARD_LIBRARY_PACKAGES,
     'aio_pika',
     'aiohttp',
     'aiopg',
@@ -136,11 +139,9 @@ def parse_clean(args: argparse.Namespace) -> None:
     'remoulade',
     'requests',
     'sqlalchemy',
-    'sqlite3',
     'starlette',
     'tornado',
     'tortoise_orm',
-    'urllib',
     'urllib3',
 }
 OTEL_PACKAGE_LINK = {'aiohttp': 'aiohttp-client', 'tortoise_orm': 'tortoiseorm', 'scikit-learn': 'sklearn'}
@@ -171,6 +172,8 @@ def parse_inspect(args: argparse.Namespace) -> None:
     # Drop packages that are dependencies of other packages.
     if packages.get('starlette') and packages.get('fastapi'):
         del packages['starlette']
+    if packages.get('urllib3') and packages.get('requests'):
+        del packages['urllib3']
 
     for name, otel_package in sorted(packages.items()):
         package_name = otel_package.replace('.', '-')

tests/test_cli.py

@@ -8,6 +8,7 @@
 import sys
 import webbrowser
 from contextlib import ExitStack
+from importlib.machinery import ModuleSpec
 from pathlib import Path
 from unittest.mock import call, patch
 
@@ -19,7 +20,7 @@
 
 import logfire._internal.cli
 from logfire import VERSION
-from logfire._internal.cli import OTEL_PACKAGES, main
+from logfire._internal.cli import STANDARD_LIBRARY_PACKAGES, main
 from logfire._internal.config import LogfireCredentials, sanitize_project_name
 from logfire.exceptions import LogfireConfigError
 
@@ -208,18 +209,58 @@ def test_inspect(
     assert capsys.readouterr().err.startswith('The following packages')
 
 
-def test_inspect_drop_dependant_packages(
-    tmp_dir_cwd: Path, logfire_credentials: LogfireCredentials, capsys: pytest.CaptureFixture[str]
+def packages_from_output(output: str) -> set[str]:
+    pattern = r'│\s*(\w+)\s*│\s*([\w-]+)\s*│'
+    matches = re.findall(pattern, output)
+    return {match[1] for match in matches}
+
+
+@pytest.mark.parametrize(
+    ('installed', 'should_install'),
+    [
+        (
+            ['fastapi'],
+            {
+                'opentelemetry-instrumentation-fastapi',
+                'opentelemetry-instrumentation-urllib',
+                'opentelemetry-instrumentation-sqlite3',
+            },
+        ),
+        (
+            ['fastapi', 'starlette'],
+            {
+                'opentelemetry-instrumentation-fastapi',
+                'opentelemetry-instrumentation-urllib',
+                'opentelemetry-instrumentation-sqlite3',
+            },
+        ),
+        (
+            ['urllib3', 'requests'],
+            {
+                'opentelemetry-instrumentation-requests',
+                'opentelemetry-instrumentation-urllib',
+                'opentelemetry-instrumentation-sqlite3',
+            },
+        ),
+    ],
+)
+def test_inspect_with_dependencies(
+    tmp_dir_cwd: Path,
+    logfire_credentials: LogfireCredentials,
+    installed: list[str],
+    should_install: list[str],
+    capsys: pytest.CaptureFixture[str],
 ) -> None:
     logfire_credentials.write_creds_file(tmp_dir_cwd / '.logfire')
-    with ExitStack() as stack:
-        find_spec = stack.enter_context(patch('importlib.util.find_spec'))
-        find_spec.side_effect = [True, None] * len(OTEL_PACKAGES)
 
+    def new_find_spec(name: str) -> ModuleSpec | None:
+        if name in STANDARD_LIBRARY_PACKAGES or name in installed:
+            return ModuleSpec(name, None)
+
+    with patch('importlib.util.find_spec', new=new_find_spec):
         main(['inspect'])
         output = capsys.readouterr().err
-        assert 'opentelemetry-instrumentation-fastapi' in output
-        assert 'opentelemetry-instrumentation-starlette' not in output
+        assert packages_from_output(output) == should_install
 
 
 @pytest.mark.parametrize('webbrowser_error', [False, True])