diff --git a/ddtrace/_monkey.py b/ddtrace/_monkey.py index 8ede9f49ca4..03417fe0d0f 100644 --- a/ddtrace/_monkey.py +++ b/ddtrace/_monkey.py @@ -157,9 +157,6 @@ } -DEFAULT_MODULES_PREFIX = "ddtrace.contrib" - - class PatchException(Exception): """Wraps regular `Exception` class when patching modules""" @@ -170,19 +167,14 @@ class ModuleNotFoundException(PatchException): pass -def _on_import_factory(module, prefix="ddtrace.contrib", raise_errors=True, patch_indicator=True): +def _on_import_factory(module, path_f, raise_errors=True, patch_indicator=True): # type: (str, str, bool, Union[bool, List[str]]) -> Callable[[Any], None] """Factory to create an import hook for the provided module name""" def on_import(hook): # Import and patch module try: - try: - imported_module = importlib.import_module("%s.internal.%s.patch" % (prefix, module)) - except ImportError: - # Some integrations do not have an internal patch module, so we use the public one - # FIXME: This is a temporary solution until we refactor the patching logic. - imported_module = importlib.import_module("%s.%s" % (prefix, module)) + imported_module = importlib.import_module(path_f % (module,)) imported_module.patch() if hasattr(imported_module, "patch_submodules"): imported_module.patch_submodules(patch_indicator) @@ -209,7 +201,7 @@ def on_import(hook): name, True, PATCH_MODULES.get(module) is True, "", version=v ) elif hasattr(imported_module, "get_version"): - # TODO: Ensure every integration defines either get_version or get_versions in their patch.py module + # Some integrations/iast patchers do not define get_version version = imported_module.get_version() telemetry.telemetry_writer.add_integration( module, True, PATCH_MODULES.get(module) is True, "", version=version @@ -258,8 +250,8 @@ def patch_all(**patch_modules): load_common_appsec_modules() -def patch(raise_errors=True, patch_modules_prefix=DEFAULT_MODULES_PREFIX, **patch_modules): - # type: (bool, str, Union[List[str], bool]) -> None +def patch(raise_errors=True, **patch_modules): + # type: (bool, Union[List[str], bool]) -> None """Patch only a set of given modules. :param bool raise_errors: Raise error if one patch fail. @@ -270,17 +262,22 @@ def patch(raise_errors=True, patch_modules_prefix=DEFAULT_MODULES_PREFIX, **patc contribs = {c: patch_indicator for c, patch_indicator in patch_modules.items() if patch_indicator} for contrib, patch_indicator in contribs.items(): # Check if we have the requested contrib. - if not os.path.isfile(os.path.join(os.path.dirname(__file__), "contrib", contrib, "__init__.py")): + if not os.path.isfile(os.path.join(os.path.dirname(__file__), "contrib", "internal", contrib, "patch.py")): if raise_errors: raise ModuleNotFoundException( - "integration module ddtrace.contrib.%s does not exist, " - "module will not have tracing available" % contrib + "integration module ddtrace.contrib.internal.%s.patch does not exist, " + "automatic instrumentation is disabled for this library" % contrib ) modules_to_patch = _MODULES_FOR_CONTRIB.get(contrib, (contrib,)) for module in modules_to_patch: # Use factory to create handler to close over `module` and `raise_errors` values from this loop when_imported(module)( - _on_import_factory(contrib, raise_errors=raise_errors, patch_indicator=patch_indicator) + _on_import_factory( + contrib, + "ddtrace.contrib.internal.%s.patch", + raise_errors=raise_errors, + patch_indicator=patch_indicator, + ) ) # manually add module to patched modules diff --git a/ddtrace/appsec/_iast/_patch_modules.py b/ddtrace/appsec/_iast/_patch_modules.py index 7ae1b0986fb..e91438ebd49 100644 --- a/ddtrace/appsec/_iast/_patch_modules.py +++ b/ddtrace/appsec/_iast/_patch_modules.py @@ -19,10 +19,6 @@ def patch_iast(patch_modules=IAST_PATCH): from ddtrace._monkey import _on_import_factory for module in (m for m, e in patch_modules.items() if e): - when_imported("hashlib")( - _on_import_factory(module, prefix="ddtrace.appsec._iast.taint_sinks", raise_errors=False) - ) + when_imported("hashlib")(_on_import_factory(module, "ddtrace.appsec._iast.taint_sinks.%s", raise_errors=False)) - when_imported("json")( - _on_import_factory("json_tainting", prefix="ddtrace.appsec._iast._patches", raise_errors=False) - ) + when_imported("json")(_on_import_factory("json_tainting", "ddtrace.appsec._iast._patches.%s", raise_errors=False)) diff --git a/ddtrace/contrib/cherrypy/__init__.py b/ddtrace/contrib/cherrypy/__init__.py index b8fbd426e00..0ae66d24918 100644 --- a/ddtrace/contrib/cherrypy/__init__.py +++ b/ddtrace/contrib/cherrypy/__init__.py @@ -54,8 +54,8 @@ def index(self): """ -from ddtrace.contrib.internal.cherrypy.middleware import TraceMiddleware -from ddtrace.contrib.internal.cherrypy.middleware import get_version # noqa: F401 +from ddtrace.contrib.internal.cherrypy.patch import TraceMiddleware +from ddtrace.contrib.internal.cherrypy.patch import get_version # noqa: F401 from ddtrace.internal.utils.deprecations import DDTraceDeprecationWarning from ddtrace.vendor.debtcollector import deprecate diff --git a/ddtrace/contrib/flask_cache/__init__.py b/ddtrace/contrib/flask_cache/__init__.py index ae6fcf003db..d35ab94f6e6 100644 --- a/ddtrace/contrib/flask_cache/__init__.py +++ b/ddtrace/contrib/flask_cache/__init__.py @@ -45,8 +45,8 @@ def counter(): """ -from ddtrace.contrib.internal.flask_cache.tracers import get_traced_cache -from ddtrace.contrib.internal.flask_cache.tracers import get_version # noqa: F401 +from ddtrace.contrib.internal.flask_cache.patch import get_traced_cache +from ddtrace.contrib.internal.flask_cache.patch import get_version # noqa: F401 from ddtrace.internal.utils.deprecations import DDTraceDeprecationWarning from ddtrace.vendor.debtcollector import deprecate diff --git a/ddtrace/contrib/internal/aioredis/patch.py b/ddtrace/contrib/internal/aioredis/patch.py index 62b8ec9f80c..78342aa162a 100644 --- a/ddtrace/contrib/internal/aioredis/patch.py +++ b/ddtrace/contrib/internal/aioredis/patch.py @@ -12,9 +12,9 @@ from ddtrace.constants import _SPAN_MEASURED_KEY from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils +from ddtrace.contrib.internal.redis_utils import ROW_RETURNING_COMMANDS from ddtrace.contrib.internal.redis_utils import _run_redis_command_async -from ddtrace.contrib.redis_utils import ROW_RETURNING_COMMANDS -from ddtrace.contrib.redis_utils import determine_row_count +from ddtrace.contrib.internal.redis_utils import determine_row_count from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes from ddtrace.ext import db diff --git a/ddtrace/contrib/internal/cassandra/patch.py b/ddtrace/contrib/internal/cassandra/patch.py index ad375683633..3632e6943b1 100644 --- a/ddtrace/contrib/internal/cassandra/patch.py +++ b/ddtrace/contrib/internal/cassandra/patch.py @@ -1,14 +1,2 @@ -from ddtrace.internal.utils.deprecations import DDTraceDeprecationWarning -from ddtrace.vendor.debtcollector import deprecate - from .session import patch # noqa: F401 from .session import unpatch # noqa: F401 - - -deprecate( - ("%s is deprecated" % (__name__)), - message="Avoid using this package directly. " - "Use ``import ddtrace.auto`` or the ``ddtrace-run`` command to enable and configure this integration.", - category=DDTraceDeprecationWarning, - removal_version="3.0.0", -) diff --git a/ddtrace/contrib/internal/cherrypy/middleware.py b/ddtrace/contrib/internal/cherrypy/patch.py similarity index 100% rename from ddtrace/contrib/internal/cherrypy/middleware.py rename to ddtrace/contrib/internal/cherrypy/patch.py diff --git a/ddtrace/contrib/internal/fastapi/patch.py b/ddtrace/contrib/internal/fastapi/patch.py index 485c0424a5f..c79baefc59d 100644 --- a/ddtrace/contrib/internal/fastapi/patch.py +++ b/ddtrace/contrib/internal/fastapi/patch.py @@ -10,7 +10,7 @@ from ddtrace.contrib.internal.asgi.middleware import TraceMiddleware from ddtrace.contrib.internal.starlette.patch import _trace_background_tasks from ddtrace.contrib.internal.starlette.patch import traced_handler -from ddtrace.contrib.starlette.patch import traced_route_init +from ddtrace.contrib.internal.starlette.patch import traced_route_init from ddtrace.internal.logger import get_logger from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.utils.wrappers import unwrap as _u diff --git a/ddtrace/contrib/internal/flask_cache/tracers.py b/ddtrace/contrib/internal/flask_cache/patch.py similarity index 100% rename from ddtrace/contrib/internal/flask_cache/tracers.py rename to ddtrace/contrib/internal/flask_cache/patch.py diff --git a/ddtrace/contrib/internal/pytest/_plugin_v2.py b/ddtrace/contrib/internal/pytest/_plugin_v2.py index ece2098e05e..0b25af62e51 100644 --- a/ddtrace/contrib/internal/pytest/_plugin_v2.py +++ b/ddtrace/contrib/internal/pytest/_plugin_v2.py @@ -7,9 +7,9 @@ from ddtrace import DDTraceDeprecationWarning from ddtrace import config as dd_config from ddtrace._monkey import patch -from ddtrace.contrib.coverage import patch as patch_coverage from ddtrace.contrib.internal.coverage.constants import PCT_COVERED_KEY from ddtrace.contrib.internal.coverage.data import _coverage_data +from ddtrace.contrib.internal.coverage.patch import patch as patch_coverage from ddtrace.contrib.internal.coverage.patch import run_coverage_report from ddtrace.contrib.internal.coverage.utils import _is_coverage_invoked_by_coverage_run from ddtrace.contrib.internal.coverage.utils import _is_coverage_patched diff --git a/ddtrace/contrib/internal/redis/patch.py b/ddtrace/contrib/internal/redis/patch.py index 28beeb4d979..78d0ceaf22a 100644 --- a/ddtrace/contrib/internal/redis/patch.py +++ b/ddtrace/contrib/internal/redis/patch.py @@ -6,9 +6,9 @@ from ddtrace import config from ddtrace._trace.utils_redis import _instrument_redis_cmd from ddtrace._trace.utils_redis import _instrument_redis_execute_pipeline +from ddtrace.contrib.internal.redis_utils import ROW_RETURNING_COMMANDS +from ddtrace.contrib.internal.redis_utils import determine_row_count from ddtrace.contrib.internal.trace_utils import unwrap -from ddtrace.contrib.redis_utils import ROW_RETURNING_COMMANDS -from ddtrace.contrib.redis_utils import determine_row_count from ddtrace.internal import core from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.utils.formats import CMD_MAX_LEN diff --git a/ddtrace/llmobs/_llmobs.py b/ddtrace/llmobs/_llmobs.py index ca830b8d310..5277accd838 100644 --- a/ddtrace/llmobs/_llmobs.py +++ b/ddtrace/llmobs/_llmobs.py @@ -498,7 +498,7 @@ def _patch_integrations() -> None: integrations_to_patch.update( {k: asbool(v) for k, v in dd_patch_modules_to_str.items() if k in SUPPORTED_LLMOBS_INTEGRATIONS.values()} ) - patch(**integrations_to_patch) # type: ignore[arg-type] + patch(**integrations_to_patch) log.debug("Patched LLM integrations: %s", list(SUPPORTED_LLMOBS_INTEGRATIONS.values())) @classmethod diff --git a/docs/contributing-integrations.rst b/docs/contributing-integrations.rst index 0dab68b5053..3ae93789809 100644 --- a/docs/contributing-integrations.rst +++ b/docs/contributing-integrations.rst @@ -211,7 +211,8 @@ What does a complete PR look like when adding a new integration? The following is the check list for ensuring you have all of the components to have a complete PR that is ready for review. -- Patch code for your new integration under ``ddtrace/contrib/your_integration_name``. +- Define `patch` and `unpatch` functions for your new integration under ``ddtrace/contrib/internal/your_integration_name``. +- Document your integration in a ``ddtrace/contrib/_integration_name.py`` module and reference the doc string in ``docs/integrations.rst``. - Test code for the above in ``tests/contrib/your_integration_name``. - The virtual environment configurations for your tests in ``riotfile.py``. - The Circle CI configurations for your tests in ``.circleci/config.templ.yml``. diff --git a/releasenotes/notes/ensure-all-integrations-have-patch-module-ef2ced613d1f777a.yaml b/releasenotes/notes/ensure-all-integrations-have-patch-module-ef2ced613d1f777a.yaml new file mode 100644 index 00000000000..da5c951017a --- /dev/null +++ b/releasenotes/notes/ensure-all-integrations-have-patch-module-ef2ced613d1f777a.yaml @@ -0,0 +1,4 @@ +--- +other: + - | + cassandra,cherrypy,flask_cache,starlette: Ensures a deprecation warning is not raised when patching these integrations via ``ddtrace-run`` and ``import ddtrace.auto``. diff --git a/tests/contrib/asyncio/test_tracer.py b/tests/contrib/asyncio/test_tracer.py index e7aaa94d903..cb474786b72 100644 --- a/tests/contrib/asyncio/test_tracer.py +++ b/tests/contrib/asyncio/test_tracer.py @@ -4,7 +4,7 @@ import pytest from ddtrace.constants import ERROR_MSG -from ddtrace.contrib.asyncio.compat import asyncio_current_task +from ddtrace.contrib.internal.asyncio.compat import asyncio_current_task from ddtrace.contrib.internal.asyncio.patch import patch from ddtrace.contrib.internal.asyncio.patch import unpatch diff --git a/tests/contrib/cherrypy/test_middleware.py b/tests/contrib/cherrypy/test_middleware.py index 9bc4a600136..41b5e4fec32 100644 --- a/tests/contrib/cherrypy/test_middleware.py +++ b/tests/contrib/cherrypy/test_middleware.py @@ -15,7 +15,7 @@ from ddtrace.constants import ERROR_MSG from ddtrace.constants import ERROR_STACK from ddtrace.constants import ERROR_TYPE -from ddtrace.contrib.internal.cherrypy.middleware import TraceMiddleware +from ddtrace.contrib.internal.cherrypy.patch import TraceMiddleware from ddtrace.ext import http from tests.contrib.patch import emit_integration_and_version_to_test_agent from tests.utils import TracerTestCase @@ -55,7 +55,7 @@ def setUp(self): ) def test_and_emit_get_version(self): - from ddtrace.contrib.internal.cherrypy.middleware import get_version + from ddtrace.contrib.internal.cherrypy.patch import get_version version = get_version() assert type(version) == str @@ -543,7 +543,7 @@ def test_service_name_schema(ddtrace_run_python_code_in_subprocess, schema_versi from cherrypy.test import helper from tests.utils import TracerTestCase from tests.contrib.cherrypy.web import StubApp -from ddtrace.contrib.internal.cherrypy.middleware import TraceMiddleware +from ddtrace.contrib.internal.cherrypy.patch import TraceMiddleware class TestCherrypy(TracerTestCase, helper.CPWebCase): @staticmethod def setup_server(): @@ -602,7 +602,7 @@ def test_operation_name_schema(ddtrace_run_python_code_in_subprocess, schema_ver from cherrypy.test import helper from tests.utils import TracerTestCase from tests.contrib.cherrypy.web import StubApp -from ddtrace.contrib.internal.cherrypy.middleware import TraceMiddleware +from ddtrace.contrib.internal.cherrypy.patch import TraceMiddleware class TestCherrypy(TracerTestCase, helper.CPWebCase): @staticmethod def setup_server(): diff --git a/tests/contrib/flask_cache/test.py b/tests/contrib/flask_cache/test.py index b7be36a10eb..25ed861dbe2 100644 --- a/tests/contrib/flask_cache/test.py +++ b/tests/contrib/flask_cache/test.py @@ -1,8 +1,8 @@ # -*- coding: utf-8 -*- from flask import Flask -from ddtrace.contrib.internal.flask_cache.tracers import CACHE_BACKEND -from ddtrace.contrib.internal.flask_cache.tracers import get_traced_cache +from ddtrace.contrib.internal.flask_cache.patch import CACHE_BACKEND +from ddtrace.contrib.internal.flask_cache.patch import get_traced_cache from ddtrace.ext import net from ddtrace.internal.schema import DEFAULT_SPAN_SERVICE_NAME from tests.opentracer.utils import init_tracer diff --git a/tests/contrib/flask_cache/test_utils.py b/tests/contrib/flask_cache/test_utils.py index 7d7e200cd9f..aa4d529c05f 100644 --- a/tests/contrib/flask_cache/test_utils.py +++ b/tests/contrib/flask_cache/test_utils.py @@ -3,7 +3,7 @@ from flask import Flask from ddtrace._trace.tracer import Tracer -from ddtrace.contrib.internal.flask_cache.tracers import get_traced_cache +from ddtrace.contrib.internal.flask_cache.patch import get_traced_cache from ddtrace.contrib.internal.flask_cache.utils import _extract_client from ddtrace.contrib.internal.flask_cache.utils import _extract_conn_tags from ddtrace.contrib.internal.flask_cache.utils import _resource_from_cache_prefix diff --git a/tests/contrib/flask_cache/test_wrapper_safety.py b/tests/contrib/flask_cache/test_wrapper_safety.py index 0b98e1e2557..d144b9ddc83 100644 --- a/tests/contrib/flask_cache/test_wrapper_safety.py +++ b/tests/contrib/flask_cache/test_wrapper_safety.py @@ -3,8 +3,8 @@ import pytest from redis.exceptions import ConnectionError -from ddtrace.contrib.internal.flask_cache.tracers import CACHE_BACKEND -from ddtrace.contrib.internal.flask_cache.tracers import get_traced_cache +from ddtrace.contrib.internal.flask_cache.patch import CACHE_BACKEND +from ddtrace.contrib.internal.flask_cache.patch import get_traced_cache from ddtrace.ext import net from tests.utils import TracerTestCase diff --git a/tests/contrib/langgraph/test_langgraph_patch.py b/tests/contrib/langgraph/test_langgraph_patch.py index 5e57cbb3f63..5326ef0f4b0 100644 --- a/tests/contrib/langgraph/test_langgraph_patch.py +++ b/tests/contrib/langgraph/test_langgraph_patch.py @@ -2,9 +2,9 @@ import sys from tempfile import NamedTemporaryFile -from ddtrace.contrib.langgraph import get_version -from ddtrace.contrib.langgraph import patch -from ddtrace.contrib.langgraph import unpatch +from ddtrace.contrib.internal.langgraph.patch import get_version +from ddtrace.contrib.internal.langgraph.patch import patch +from ddtrace.contrib.internal.langgraph.patch import unpatch from tests.contrib.patch import PatchTestCase from tests.utils import call_program diff --git a/tests/tracer/test_monkey.py b/tests/tracer/test_monkey.py index 9b6f1ee86a6..d4eb1522176 100644 --- a/tests/tracer/test_monkey.py +++ b/tests/tracer/test_monkey.py @@ -38,8 +38,8 @@ def test_patch_raise_exception_manual_patch(self): _monkey.patch(module_dne=True) assert ( - "integration module ddtrace.contrib.module_dne does not exist, module will not have tracing available" - in str(me.exception) + "integration module ddtrace.contrib.internal.module_dne.patch does not exist, " + "automatic instrumentation is disabled for this library" in str(me.exception) ) assert "module_dne" not in _monkey._PATCHED_MODULES