From 79069a3b41828cf194c279dcfc6b155a64f8b080 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 19 Dec 2024 13:31:41 -0500 Subject: [PATCH] fix(library): catch exceptions raised while enabling ddtrace integrations (#11759) ## Description - Improves the error message generated when `ddtrace` failed to patch/enable an integration. - Ensure patching modules and sub-modules are wrapped in a try-except. The ddtrace library should not crash an application if an integration can not be patched. ## Motivation Prevent issues like this: https://github.com/DataDog/dd-trace-py/issues/11603 ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --- ddtrace/_monkey.py | 16 +++++++++------- ...efactor-patch-error-ssi-1a2e9fe206d6d6df.yaml | 4 ++++ tests/telemetry/test_telemetry.py | 6 ++---- 3 files changed, 15 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/refactor-patch-error-ssi-1a2e9fe206d6d6df.yaml diff --git a/ddtrace/_monkey.py b/ddtrace/_monkey.py index 8dd83558c83..488211e46b1 100644 --- a/ddtrace/_monkey.py +++ b/ddtrace/_monkey.py @@ -173,17 +173,22 @@ def on_import(hook): path = "%s.%s" % (prefix, module) try: imported_module = importlib.import_module(path) + imported_module.patch() + if hasattr(imported_module, "patch_submodules"): + imported_module.patch_submodules(patch_indicator) except Exception as e: if raise_errors: raise - error_msg = "failed to import ddtrace module %r when patching on import" % (path,) - log.error(error_msg, exc_info=True) - telemetry.telemetry_writer.add_integration(module, False, PATCH_MODULES.get(module) is True, error_msg) + log.error( + "failed to enable ddtrace support for %s: %s", + module, + str(e), + ) + telemetry.telemetry_writer.add_integration(module, False, PATCH_MODULES.get(module) is True, str(e)) telemetry.telemetry_writer.add_count_metric( "tracers", "integration_errors", 1, (("integration_name", module), ("error_type", type(e).__name__)) ) else: - imported_module.patch() if hasattr(imported_module, "get_versions"): versions = imported_module.get_versions() for name, v in versions.items(): @@ -196,9 +201,6 @@ def on_import(hook): module, True, PATCH_MODULES.get(module) is True, "", version=version ) - if hasattr(imported_module, "patch_submodules"): - imported_module.patch_submodules(patch_indicator) - return on_import diff --git a/releasenotes/notes/refactor-patch-error-ssi-1a2e9fe206d6d6df.yaml b/releasenotes/notes/refactor-patch-error-ssi-1a2e9fe206d6d6df.yaml new file mode 100644 index 00000000000..8afc2e7595f --- /dev/null +++ b/releasenotes/notes/refactor-patch-error-ssi-1a2e9fe206d6d6df.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + Integrations: Improved error handling for exceptions raised during the startup of ddtrace integrations. This reduces the likelihood of the ddtrace library raising unhandled exceptions. \ No newline at end of file diff --git a/tests/telemetry/test_telemetry.py b/tests/telemetry/test_telemetry.py index d767090f6d2..558e9961afc 100644 --- a/tests/telemetry/test_telemetry.py +++ b/tests/telemetry/test_telemetry.py @@ -243,14 +243,12 @@ def test_handled_integration_error(test_agent_session, run_python_code_in_subpro _, stderr, status, _ = run_python_code_in_subprocess(code, env=env) assert status == 0, stderr - expected_stderr = b"failed to import" - assert expected_stderr in stderr + assert b"failed to enable ddtrace support for sqlite3" in stderr integrations_events = test_agent_session.get_events("app-integrations-change", subprocess=True) assert len(integrations_events) == 1 assert ( - integrations_events[0]["payload"]["integrations"][0]["error"] - == "failed to import ddtrace module 'ddtrace.contrib.sqlite3' when patching on import" + integrations_events[0]["payload"]["integrations"][0]["error"] == "module 'sqlite3' has no attribute 'connect'" ) # Get metric containing the integration error