diff --git a/logfire/_internal/cli.py b/logfire/_internal/cli.py index acdbe1ff1..14e373934 100644 --- a/logfire/_internal/cli.py +++ b/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('.', '-') diff --git a/tests/test_cli.py b/tests/test_cli.py index c566f9cd1..1346ec9ac 100644 --- a/tests/test_cli.py +++ b/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])