Skip to content

Conversation

@sentrivana
Copy link
Contributor

Testing last master -> potel-base sync

szokeasaurusrex and others added 6 commits June 5, 2025 15:39
We are always just using the current scope anyway; it is less confusing
if we eliminate the parameter

Stacked on:
  - #4423
### PR Summary
This small PR resolves the `threading` library warnings, which you can
find in the [CI
logs](https://github.com/getsentry/sentry-python/actions/runs/15490014338/job/43613162178#step:6:3523):
```python
/home/runner/work/sentry-python/sentry-python/tests/conftest.py:608: DeprecationWarning: setDaemon() is deprecated, set the daemon attribute instead
    mock_server_thread.setDaemon(True)
```

Signed-off-by: Emmanuel Ferdman <[email protected]>
Hi! In the Python SDK, more specifically
`sentry_sdk/integrations/logging.py`, a couple of loggers are ignored to
avoid recursion errors.
```py
_IGNORED_LOGGERS = set(
    ["sentry_sdk.errors", "urllib3.connectionpool", "urllib3.connection"]
)
```
Log records from these loggers are discarded, using an exact match on
`record.name`. Unforunately, this breaks if the user modifies
`record.name`, e.g. for formatting, which is what we were doing for log
display (before becoming aware that Sentry relied on it after
investigating an infinite recursion issue).
```py
class _LengthFormatter(logging.Formatter):
    def format(self, record):
        """
        Format a log record's header to a constant length
        """
        if len(record.name) > _MAX_LOGGER_NAME_LENGTH:
            sep = "..."
            cut = _MAX_LOGGER_NAME_LENGTH // 3 - len(sep)
            record.name = record.name[:cut] + sep + record.name[-(_MAX_LOGGER_NAME_LENGTH - cut - len(sep)) :]
        record.name = record.name.ljust(_MAX_LOGGER_NAME_LENGTH)

        record.levelname = record.levelname.ljust(_MAX_LOGGER_LEVEL_NAME_LENGTH)
        return super().format(record)
```

As you can see, `record.name` is right-padded with blank spaces. We have
found a workaround since, but given that it has taken us quite some time
to find the issue, I thought that maybe it could affect others. This PR
proposes matching `record.name.strip()` instead for increased
robustness.
Allow to send Loguru logs to Sentry.

We can't parametrize them nicely, but this is a good first step.

Also:
* Move some tests around. Tests specific to the stdlib logging
integration were moved from the generic sentry logs tests to the logging
integration tests.
* Remove `@minimum_python_37` from some tests that don't need 3.7+. 
* Dedupe some code by moving it to a superclass (`_LoguruBaseHandler`)


Closes #4151

---------

Co-authored-by: Daniel Szoke <[email protected]>
@codecov
Copy link

codecov bot commented Jun 12, 2025

Codecov Report

Attention: Patch coverage is 77.46479% with 16 lines in your changes missing coverage. Please review.

Project coverage is 84.77%. Comparing base (6ad4031) to head (673c34c).
Report is 12 commits behind head on potel-base.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/integrations/loguru.py 75.00% 2 Missing and 9 partials ⚠️
sentry_sdk/logger.py 66.66% 2 Missing and 2 partials ⚠️
sentry_sdk/client.py 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           potel-base    #4458      +/-   ##
==============================================
- Coverage       84.79%   84.77%   -0.03%     
==============================================
  Files             144      144              
  Lines           14742    14788      +46     
  Branches         2346     2361      +15     
==============================================
+ Hits            12501    12537      +36     
- Misses           1525     1527       +2     
- Partials          716      724       +8     
Files with missing lines Coverage Δ
sentry_sdk/integrations/logging.py 87.75% <100.00%> (+1.00%) ⬆️
sentry_sdk/client.py 84.88% <88.88%> (-0.03%) ⬇️
sentry_sdk/logger.py 87.09% <66.66%> (-12.91%) ⬇️
sentry_sdk/integrations/loguru.py 85.22% <75.00%> (-7.37%) ⬇️

... and 2 files with indirect coverage changes

@sentrivana sentrivana merged commit 673c34c into potel-base Jun 12, 2025
121 of 122 checks passed
@sentrivana sentrivana deleted the ivana/potel/sync branch June 12, 2025 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants