Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Circular import error between redis integrations #7783

Closed
beepboop271 opened this issue Nov 29, 2023 · 2 comments · Fixed by #7794
Closed

Circular import error between redis integrations #7783

beepboop271 opened this issue Nov 29, 2023 · 2 comments · Fixed by #7794
Assignees
@emmettbutler

Comments

@beepboop271
Copy link

Summary of problem

aredis, aioredis, and yaaredis integrations import from the redis integration.

If for example aredis and redis are both installed, a circular import occurs when importing aredis after patching.

This same error has been an issue before: #5601 (comment)

and fixed before: #5608, by moving the utils being imported from the redis integration into a common trace_utils_redis file.

However a cross-integration import was added a few months ago, breaking it again:

dd-trace-py/ddtrace/contrib/aredis/patch.py

Lines 14 to 16 in 67c5ec7

from ..redis.asyncio_patch import _run_redis_command_async
from ..trace_utils_redis import _trace_redis_cmd
from ..trace_utils_redis import _trace_redis_execute_pipeline

https://github.com/DataDog/dd-trace-py/blob/v2.3.1/ddtrace/contrib/aredis/patch.py#L14-L16

Which version of dd-trace-py are you using?

2.3.1

Which version of pip are you using?

pip 23.3.1 python 3.9.9

Which libraries and their versions are you using?

pip freeze:

aredis==1.1.8
async-timeout==4.0.3
attrs==23.1.0
bytecode==0.15.1
cattrs==23.2.2
ddsketch==2.0.4
ddtrace==2.3.1
Deprecated==1.2.14
envier==0.4.0
exceptiongroup==1.2.0
importlib-metadata==6.8.0
opentelemetry-api==1.21.0
protobuf==4.25.1
redis==5.0.1
six==1.16.0
typing_extensions==4.8.0
wrapt==1.16.0
xmltodict==0.13.0
zipp==3.17.0

How can we reproduce your problem?

Minimal reproduction:

  1. fresh venv
  2. pip install ddtrace aredis redis
  3. run:
from ddtrace import patch_all
patch_all()
import aredis

What is the result that you get?

failed to import ddtrace module 'ddtrace.contrib.aredis' when patching on import
Traceback (most recent call last):
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/_monkey.py", line 165, in on_import
    imported_module = importlib.import_module(path)
  File "/home/coder/.pyenv/versions/3.9.9/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/internal/module.py", line 211, in _exec_module
    self.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/contrib/aredis/__init__.py", line 76, in <module>
    from .patch import get_version
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/internal/module.py", line 211, in _exec_module
    self.loader.exec_module(module)
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/contrib/aredis/patch.py", line 14, in <module>
    from ..redis.asyncio_patch import _run_redis_command_async
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/internal/module.py", line 211, in _exec_module
    self.loader.exec_module(module)
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/contrib/redis/__init__.py", line 75, in <module>
    with require_modules(required_modules) as missing_modules:
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/internal/utils/importlib.py", line 20, in __init__
    import_module(module)
  File "/home/coder/.pyenv/versions/3.9.9/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/vendor/wrapt/importer.py", line 177, in _exec_module
    notify_module_loaded(module)
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/vendor/wrapt/decorators.py", line 470, in _synchronized
    return wrapped(*args, **kwargs)
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/vendor/wrapt/importer.py", line 136, in notify_module_loaded
    hook(module)
  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/_monkey.py", line 176, in on_import
    imported_module.patch()
AttributeError: partially initialized module 'ddtrace.contrib.redis' has no attribute 'patch' (most likely due to a circular import)

Notice the frame about halfway through the trace:

  File "/workspace/test/.venv/lib/python3.9/site-packages/ddtrace/contrib/aredis/patch.py", line 14, in <module>
    from ..redis.asyncio_patch import _run_redis_command_async

What is the result that you expected?

No error thrown
@emmettbutler
Copy link
Collaborator

I'm going to close this as a duplicate of #7241

@emmettbutler emmettbutler closed this as not planned Won't fix, can't repro, duplicate, stale Nov 30, 2023
@emmettbutler
Copy link
Collaborator

On second thought, there's an important difference between the two.

@emmettbutler emmettbutler reopened this Nov 30, 2023
This was referenced Nov 30, 2023
Closed
Merged
@emmettbutler emmettbutler self-assigned this Nov 30, 2023
emmettbutler added a commit that referenced this issue Nov 30, 2023
@emmettbutler @wconti27
fix(redis): remove circular import from redis integrations (#7794)
6de31a4 
This change fixes #7783 by
removing the imports of `contrib.redis` code from `contrib.yaaredis` and
`contrib.aredis`.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] 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)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

---------

Co-authored-by: William Conti <58711692+wconti27@users.noreply.github.com>
@github-actions github-actions bot mentioned this issue Nov 30, 2023
Merged
18 tasks
github-actions bot pushed a commit that referenced this issue Nov 30, 2023
@emmettbutler @github-actions
fix(redis): remove circular import from redis integrations (#7794)
bede0e2 
This change fixes #7783 by
removing the imports of `contrib.redis` code from `contrib.yaaredis` and
`contrib.aredis`.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] 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)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

---------

Co-authored-by: William Conti <58711692+wconti27@users.noreply.github.com>
(cherry picked from commit 6de31a4)
@github-actions github-actions bot mentioned this issue Nov 30, 2023
Merged
18 tasks
github-actions bot pushed a commit that referenced this issue Nov 30, 2023
@emmettbutler @github-actions
fix(redis): remove circular import from redis integrations (#7794)
3bba928 
This change fixes #7783 by
removing the imports of `contrib.redis` code from `contrib.yaaredis` and
`contrib.aredis`.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] 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)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

---------

Co-authored-by: William Conti <58711692+wconti27@users.noreply.github.com>
(cherry picked from commit 6de31a4)
@github-actions github-actions bot mentioned this issue Nov 30, 2023
Merged
18 tasks
github-actions bot pushed a commit that referenced this issue Nov 30, 2023
@emmettbutler @github-actions
fix(redis): remove circular import from redis integrations (#7794)
c6272e3 
This change fixes #7783 by
removing the imports of `contrib.redis` code from `contrib.yaaredis` and
`contrib.aredis`.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] 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)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

---------

Co-authored-by: William Conti <58711692+wconti27@users.noreply.github.com>
(cherry picked from commit 6de31a4)
emmettbutler added a commit that referenced this issue Dec 1, 2023
@github-actions @emmettbutler
fix(redis): remove circular import from redis integrations [backport …
998ba5e 
…2.2] (#7800)

Backport 6de31a4 from #7794 to 2.2.

This change fixes #7783 by
removing the imports of `contrib.redis` code from `contrib.yaaredis` and
`contrib.aredis`.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] 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)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

Co-authored-by: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
majorgreys pushed a commit that referenced this issue Dec 6, 2023
@github-actions @emmettbutler
fix(redis): remove circular import from redis integrations [backport …
66292da 
…2.1] (#7799)

Backport 6de31a4 from #7794 to 2.1.

This change fixes #7783 by
removing the imports of `contrib.redis` code from `contrib.yaaredis` and
`contrib.aredis`.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] 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)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

Co-authored-by: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
emmettbutler added a commit that referenced this issue Dec 6, 2023
@github-actions @emmettbutler @ZStriker19
fix(redis): remove circular import from redis integrations [backport …
0301f25 
…2.3] (#7801)

Backport 6de31a4 from #7794 to 2.3.

This change fixes #7783 by
removing the imports of `contrib.redis` code from `contrib.yaaredis` and
`contrib.aredis`.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] 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)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

Co-authored-by: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
Co-authored-by: Zachary Groves <32471391+ZStriker19@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@emmettbutler emmettbutler

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(redis): remove circular import from redis integrations DataDog/dd-trace-py
2 participants
@emmettbutler @beepboop271