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

ddtrace/internal/sampling.py incorrectly logs with exc_info=True #8067

Closed
leifwalsh opened this issue Jan 11, 2024 · 1 comment · Fixed by #8403
Closed

ddtrace/internal/sampling.py incorrectly logs with exc_info=True #8067

leifwalsh opened this issue Jan 11, 2024 · 1 comment · Fixed by #8403

Comments

@leifwalsh
Copy link

Summary of problem

log.warning("failed to decode _dd.p.dm: %r", value, exc_info=True)
passes exc_info=True to logger.warning() in a non-exception context.

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

2.3.2, but it's still present in 2.4.0 and has been there for ages.

Which version of pip are you using?

Doesn't matter

Which libraries and their versions are you using?

Doesn't matter

How can we reproduce your problem?

Not really sure - we don't know why that line of code suddenly started getting executed recently, it never has before this week.

What is the result that you get?

This causes our logging middleware to try to get sys.exc_info() and then that code (incorrectly) assumes there must be an exception available, and since there isn't, it crashes. We'll fix our logging middleware, but IMO this library should also be fixed.

What is the result that you expected?

Don't pass exc_info unless you're in an exception handler.

@emmettbutler
Copy link
Collaborator

Thank you for the report, @leifwalsh. We'll look into it!

mabdinur added a commit that referenced this issue Feb 14, 2024
Resolve: #8067

## 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] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [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))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.
- [x] If change 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`.

## Reviewer Checklist

- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants