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.

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

Redis integration considered enabled even when Redis client not installed #1734

Closed
Harmon758 opened this issue Nov 10, 2022 · 1 comment · Fixed by #2504
Closed

Redis integration considered enabled even when Redis client not installed #1734

Harmon758 opened this issue Nov 10, 2022 · 1 comment · Fixed by #2504
Assignees

Comments

@Harmon758
Copy link
Contributor

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.10.1

Steps to Reproduce

D:\>py -3.11 -m venv .venv

D:\>cd .venv

D:\.venv>Scripts\activate.bat

(.venv) D:\.venv>pip install sentry-sdk
Collecting sentry-sdk
  Using cached sentry_sdk-1.10.1-py2.py3-none-any.whl (166 kB)
Collecting certifi
  Using cached certifi-2022.9.24-py3-none-any.whl (161 kB)
Collecting urllib3>=1.26.11
  Using cached urllib3-1.26.12-py2.py3-none-any.whl (140 kB)
Installing collected packages: urllib3, certifi, sentry-sdk
Successfully installed certifi-2022.9.24 sentry-sdk-1.10.1 urllib3-1.26.12

[notice] A new release of pip available: 22.3 -> 22.3.1
[notice] To update, run: python.exe -m pip install --upgrade pip

(.venv) D:\.venv>python
Python 3.11.0 (main, Oct 24 2022, 18:26:48) [MSC v.1933 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sentry_sdk
>>> sentry_sdk.init(debug=True)
 [sentry] DEBUG: Setting up integrations (with default = True)
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.django.DjangoIntegration: Django not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.flask.FlaskIntegration: Flask is not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.starlette.StarletteIntegration: Starlette is not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.fastapi.FastApiIntegration: Starlette is not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.bottle.BottleIntegration: Bottle not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.falcon.FalconIntegration: Falcon not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.sanic.SanicIntegration: Sanic not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.celery.CeleryIntegration: Celery not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.rq.RqIntegration: RQ not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.aiohttp.AioHttpIntegration: AIOHTTP not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.tornado.TornadoIntegration: Tornado not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.sqlalchemy.SqlalchemyIntegration: SQLAlchemy not installed.
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.pyramid.PyramidIntegration: Pyramid not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.boto3.Boto3Integration: botocore is not installed
 [sentry] DEBUG: Setting up previously not enabled integration logging
 [sentry] DEBUG: Setting up previously not enabled integration stdlib
 [sentry] DEBUG: Setting up previously not enabled integration excepthook
 [sentry] DEBUG: Setting up previously not enabled integration dedupe
 [sentry] DEBUG: Setting up previously not enabled integration atexit
 [sentry] DEBUG: Setting up previously not enabled integration modules
 [sentry] DEBUG: Setting up previously not enabled integration argv
 [sentry] DEBUG: Setting up previously not enabled integration threading
 [sentry] DEBUG: Setting up previously not enabled integration redis
 [sentry] DEBUG: Did not enable default integration redis: Redis client not installed
 [sentry] DEBUG: Enabling integration logging
 [sentry] DEBUG: Enabling integration stdlib
 [sentry] DEBUG: Enabling integration excepthook
 [sentry] DEBUG: Enabling integration dedupe
 [sentry] DEBUG: Enabling integration atexit
 [sentry] DEBUG: Enabling integration modules
 [sentry] DEBUG: Enabling integration argv
 [sentry] DEBUG: Enabling integration threading
 [sentry] DEBUG: Enabling integration redis
 [sentry] DEBUG: Setting SDK name to 'sentry.python'
<sentry_sdk.hub._InitGuard object at 0x0000024182399E90>
>>> sentry_sdk.Hub.current.client.integrations
{'logging': <sentry_sdk.integrations.logging.LoggingIntegration object at 0x0000024181C3F1D0>, 'stdlib': <sentry_sdk.integrations.stdlib.StdlibIntegration object at 0x0000024181FA8E90>, 'excepthook': <sentry_sdk.integrations.excepthook.ExcepthookIntegration object at 0x00000241842D1D10>, 'dedupe': <sentry_sdk.integrations.dedupe.DedupeIntegration object at 0x00000241842D3990>, 'atexit': <sentry_sdk.integrations.atexit.AtexitIntegration object at 0x00000241842D3C10>, 'modules': <sentry_sdk.integrations.modules.ModulesIntegration object at 0x00000241842D3D90>, 'argv': <sentry_sdk.integrations.argv.ArgvIntegration object at 0x00000241842D3F50>, 'threading': <sentry_sdk.integrations.threading.ThreadingIntegration object at 0x00000241842EC710>, 'redis': <sentry_sdk.integrations.redis.RedisIntegration object at 0x0000024184376050>}

Expected Result

Redis integration should not be considered enabled.

Actual Result

Redis integration is considered enabled according to the logs:

> [sentry] DEBUG: Setting up previously not enabled integration redis
> [sentry] DEBUG: Did not enable default integration redis: Redis client not installed
…
> [sentry] DEBUG: Enabling integration redis

and the SDK integrations dictionary:

>>> sentry_sdk.Hub.current.client.integrations
{'logging': <sentry_sdk.integrations.logging.LoggingIntegration object at 0x0000024181C3F1D0>, …, 'redis': <sentry_sdk.integrations.redis.RedisIntegration object at 0x0000024184376050>}

This seems to be because setup_integrations does not remove default integrations that raise DidNotEnable from the integrations dictionary:

def setup_integrations(
integrations, with_defaults=True, with_auto_enabling_integrations=False
):
# type: (List[Integration], bool, bool) -> Dict[str, Integration]
"""Given a list of integration instances this installs them all. When
`with_defaults` is set to `True` then all default integrations are added
unless they were already provided before.
"""
integrations = dict(
(integration.identifier, integration) for integration in integrations or ()
)
logger.debug("Setting up integrations (with default = %s)", with_defaults)
# Integrations that are not explicitly set up by the user.
used_as_default_integration = set()
if with_defaults:
for integration_cls in iter_default_integrations(
with_auto_enabling_integrations
):
if integration_cls.identifier not in integrations:
instance = integration_cls()
integrations[instance.identifier] = instance
used_as_default_integration.add(instance.identifier)
for identifier, integration in iteritems(integrations):
with _installer_lock:
if identifier not in _installed_integrations:
logger.debug(
"Setting up previously not enabled integration %s", identifier
)
try:
type(integration).setup_once()
except NotImplementedError:
if getattr(integration, "install", None) is not None:
logger.warning(
"Integration %s: The install method is "
"deprecated. Use `setup_once`.",
identifier,
)
integration.install()
else:
raise
except DidNotEnable as e:
if identifier not in used_as_default_integration:
raise
logger.debug(
"Did not enable default integration %s: %s", identifier, e
)
_installed_integrations.add(identifier)
for identifier in integrations:
logger.debug("Enabling integration %s", identifier)
return integrations

Other automatically enabled integrations don't encounter this issue because they are never added to the integrations dictionary, as they raise DidNotEnable on import, when iterating over iter_default_integrations, through _generate_default_integrations_iterator.iter_default_integrations:

iter_default_integrations = _generate_default_integrations_iterator(
integrations=(
# stdlib/base runtime integrations
"sentry_sdk.integrations.logging.LoggingIntegration",
"sentry_sdk.integrations.stdlib.StdlibIntegration",
"sentry_sdk.integrations.excepthook.ExcepthookIntegration",
"sentry_sdk.integrations.dedupe.DedupeIntegration",
"sentry_sdk.integrations.atexit.AtexitIntegration",
"sentry_sdk.integrations.modules.ModulesIntegration",
"sentry_sdk.integrations.argv.ArgvIntegration",
"sentry_sdk.integrations.threading.ThreadingIntegration",
),
auto_enabling_integrations=_AUTO_ENABLING_INTEGRATIONS,
)
def _generate_default_integrations_iterator(integrations, auto_enabling_integrations):
# type: (Tuple[str, ...], Tuple[str, ...]) -> Callable[[bool], Iterator[Type[Integration]]]
def iter_default_integrations(with_auto_enabling_integrations):
# type: (bool) -> Iterator[Type[Integration]]
"""Returns an iterator of the default integration classes:"""
from importlib import import_module
if with_auto_enabling_integrations:
all_import_strings = integrations + auto_enabling_integrations
else:
all_import_strings = integrations
for import_string in all_import_strings:
try:
module, cls = import_string.rsplit(".", 1)
yield getattr(import_module(module), cls)
except (DidNotEnable, SyntaxError) as e:
logger.debug(
"Did not import default integration %s: %s", import_string, e
)
if isinstance(iter_default_integrations.__doc__, str):
for import_string in integrations:
iter_default_integrations.__doc__ += "\n- `{}`".format(import_string)
return iter_default_integrations

Redis seems to be the only automatically enabled integration that imports and raises DidNotEnable on ImportError within setup_once rather than beforehand.

However, other automatically enabled integrations that raise DidNotEnable in setup_once for other reasons also encounter this issue, e.g. aiohttp with v3.3:

D:\>py -3.7 -m venv .venv

D:\>cd .venv

D:\.venv>Scripts\activate.bat

(.venv) D:\.venv>pip install aiohttp==3.3.0 sentry-sdk
Collecting aiohttp==3.3.0
  Using cached aiohttp-3.3.0.tar.gz (722 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Collecting sentry-sdk
  Using cached sentry_sdk-1.10.1-py2.py3-none-any.whl (166 kB)
Collecting async-timeout<4.0,>=3.0
  Using cached async_timeout-3.0.1-py3-none-any.whl (8.2 kB)
Collecting chardet<4.0,>=2.0
  Using cached chardet-3.0.4-py2.py3-none-any.whl (133 kB)
Collecting attrs>=17.3.0
  Using cached attrs-22.1.0-py2.py3-none-any.whl (58 kB)
Collecting yarl<2.0,>=1.0
  Using cached yarl-1.8.1-cp37-cp37m-win_amd64.whl (56 kB)
Collecting multidict<5.0,>=4.0
  Using cached multidict-4.7.6-cp37-cp37m-win_amd64.whl (48 kB)
Collecting certifi
  Using cached certifi-2022.9.24-py3-none-any.whl (161 kB)
Collecting urllib3>=1.26.11; python_version >= "3.6"
  Using cached urllib3-1.26.12-py2.py3-none-any.whl (140 kB)
Collecting typing-extensions>=3.7.4; python_version < "3.8"
  Using cached typing_extensions-4.4.0-py3-none-any.whl (26 kB)
Collecting idna>=2.0
  Using cached idna-3.4-py3-none-any.whl (61 kB)
Building wheels for collected packages: aiohttp
  Building wheel for aiohttp (PEP 517) ... done
  Created wheel for aiohttp: filename=aiohttp-3.3.0-py3-none-any.whl size=347624 sha256=caa4a9597eb6975a3441bb3f93b79eb875ad4461fcef8d6e1e71a8cd2255383a
  Stored in directory: c:\users\user\appdata\local\pip\cache\wheels\87\04\63\86f05b8df3510ffa9a983ae853c8756c2d1c5f76c63ceac9a4
Successfully built aiohttp
Installing collected packages: async-timeout, chardet, attrs, multidict, typing-extensions, idna, yarl, aiohttp, certifi, urllib3, sentry-sdk
Successfully installed aiohttp-3.3.0 async-timeout-3.0.1 attrs-22.1.0 certifi-2022.9.24 chardet-3.0.4 idna-3.4 multidict-4.7.6 sentry-sdk-1.10.1 typing-extensions-4.4.0 urllib3-1.26.12 yarl-1.8.1
WARNING: You are using pip version 20.1.1; however, version 22.3.1 is available.
You should consider upgrading via the 'd:\.venv\scripts\python.exe -m pip install --upgrade pip' command.

(.venv) D:\.venv>python
Python 3.7.9 (tags/v3.7.9:13c94747c7, Aug 17 2020, 18:58:18) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sentry_sdk
>>> sentry_sdk.init(debug=True)
 [sentry] DEBUG: Setting up integrations (with default = True)
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.django.DjangoIntegration: Django not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.flask.FlaskIntegration: Flask is not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.starlette.StarletteIntegration: Starlette is not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.fastapi.FastApiIntegration: Starlette is not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.bottle.BottleIntegration: Bottle not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.falcon.FalconIntegration: Falcon not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.sanic.SanicIntegration: Sanic not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.celery.CeleryIntegration: Celery not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.rq.RqIntegration: RQ not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.tornado.TornadoIntegration: Tornado not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.sqlalchemy.SqlalchemyIntegration: SQLAlchemy not installed.
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.pyramid.PyramidIntegration: Pyramid not installed
 [sentry] DEBUG: Did not import default integration sentry_sdk.integrations.boto3.Boto3Integration: botocore is not installed
 [sentry] DEBUG: Setting up previously not enabled integration logging
 [sentry] DEBUG: Setting up previously not enabled integration stdlib
 [sentry] DEBUG: Setting up previously not enabled integration excepthook
 [sentry] DEBUG: Setting up previously not enabled integration dedupe
 [sentry] DEBUG: Setting up previously not enabled integration atexit
 [sentry] DEBUG: Setting up previously not enabled integration modules
 [sentry] DEBUG: Setting up previously not enabled integration argv
 [sentry] DEBUG: Setting up previously not enabled integration threading
 [sentry] DEBUG: Setting up previously not enabled integration aiohttp
 [sentry] DEBUG: Did not enable default integration aiohttp: AIOHTTP 3.4 or newer required.
 [sentry] DEBUG: Setting up previously not enabled integration redis
 [sentry] DEBUG: Did not enable default integration redis: Redis client not installed
 [sentry] DEBUG: Enabling integration logging
 [sentry] DEBUG: Enabling integration stdlib
 [sentry] DEBUG: Enabling integration excepthook
 [sentry] DEBUG: Enabling integration dedupe
 [sentry] DEBUG: Enabling integration atexit
 [sentry] DEBUG: Enabling integration modules
 [sentry] DEBUG: Enabling integration argv
 [sentry] DEBUG: Enabling integration threading
 [sentry] DEBUG: Enabling integration aiohttp
 [sentry] DEBUG: Enabling integration redis
 [sentry] DEBUG: Setting SDK name to 'sentry.python.aiohttp'
<sentry_sdk.hub._InitGuard object at 0x0000028F71A5DC08>
>>> sentry_sdk.Hub.current.client.integrations
{'logging': <sentry_sdk.integrations.logging.LoggingIntegration object at 0x0000028F71A5DC48>, 'stdlib': <sentry_sdk.integrations.stdlib.StdlibIntegration object at 0x0000028F71A70988>, 'excepthook': <sentry_sdk.integrations.excepthook.ExcepthookIntegration object at 0x0000028F71A709C8>, 'dedupe': <sentry_sdk.integrations.dedupe.DedupeIntegration object at 0x0000028F71A70A88>, 'atexit': <sentry_sdk.integrations.atexit.AtexitIntegration object at 0x0000028F71A70A48>, 'modules': <sentry_sdk.integrations.modules.ModulesIntegration object at 0x0000028F6F987A08>, 'argv': <sentry_sdk.integrations.argv.ArgvIntegration object at 0x0000028F71A83388>, 'threading': <sentry_sdk.integrations.threading.ThreadingIntegration object at 0x0000028F71A83648>, 'aiohttp': <sentry_sdk.integrations.aiohttp.AioHttpIntegration object at 0x0000028F71B2C7C8>, 'redis': <sentry_sdk.integrations.redis.RedisIntegration object at 0x0000028F71A9A788>}

Also of note, the only relevant test for this, test_auto_enabling_integrations_catches_import_error, ignores the Redis integration, seemingly with the reasoning that the redis package is always installed as a dependency for running tests. As far as I can tell, this isn't the case now and wasn't even the case when the test was added. More likely, the test fails (and failed) because the test checks for a log message starting with "Did not import default integration ", whereas the Redis integration will successfully import, but log "Did not enable default integration " later (even though it's still considered enabled).

def test_auto_enabling_integrations_catches_import_error(sentry_init, caplog):
caplog.set_level(logging.DEBUG)
REDIS = 12 # noqa: N806
sentry_init(auto_enabling_integrations=True, debug=True)
for import_string in _AUTO_ENABLING_INTEGRATIONS:
# Ignore redis in the test case, because it is installed as a
# dependency for running tests, and therefore always enabled.
if _AUTO_ENABLING_INTEGRATIONS[REDIS] == import_string:
continue
assert any(
record.message.startswith(
"Did not import default integration {}:".format(import_string)
)
for record in caplog.records
), "Problem with checking auto enabling {}".format(import_string)

@antonpirker
Copy link
Member

Hey @Harmon758

Thanks for reporting this! Good catch! I have put this on our internal backlog so we will fix this. (But please do not hold your breath on it, because we have a lot on our plates right now)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants