Skip to content

Commit

Permalink
fix(library): catch exceptions raised while enabling ddtrace integrat…
Browse files Browse the repository at this point in the history
…ions (#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:
#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)
  • Loading branch information
mabdinur authored Dec 19, 2024
1 parent 89d82c3 commit 79069a3
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 11 deletions.
16 changes: 9 additions & 7 deletions ddtrace/_monkey.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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


Expand Down
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 2 additions & 4 deletions tests/telemetry/test_telemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 79069a3

Please sign in to comment.