Home Original page

Fix max recursion bug by removing logging.log calls in `emit` by DylanRussell · Pull Request #4586 · open-telemetry/opentelemetry-python

Conversation

@DylanRussell

Description

Remove logging.log calls from BatchProcessor.emit. Any log calls in that function can get routed back to emit and ultimately result in a maximum recursion depth exceeded exception.

Fixes: 4585

How Has This Been Tested?

Added a unit test to prevent this.

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • [x ] No.

Checklist:

  • [ x] Followed the style guidelines of this project
  • [ x] Changelogs have been updated
  • [ x] Unit tests have been added
  • [ x] Documentation has been updated

emdneto

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test is failing at main. LGTM with a changelog.

@DylanRussell

tammy-baylis-swi

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this 🙂

aabmass

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can make the test much simpler to just test a minimal repro of the reported issue

@DylanRussell

Ok updated test to attach handler to SDK logger instead of root, and remove it after test is done.

@aabmass aabmass linked an issue

May 15, 2025

that may be closed by this pull request

aabmass

emdneto

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aabmass, just a reminder that the current main branch is different from release/v1.33.x-0.54bx branch in terms of the structure of directories for the SDK. In the release branch there's no _shared_internal directory because this change was merged this week. So, probably would be better that in the backport PR we just remove the log line or maybe would be worth @DylanRussell open the PR against the release branch instead of main.

@DylanRussell

Moved this to #4588 .. Had this code in my main branch by mistake.. moved it to another branch, but couldn't edit this PR to move to that branch. So just opened up a new PR