fix: defer boot_agent until after eventlet monkey_patch to prevent recursion error - #903
CagriYonca wants to merge 1 commit into
Conversation
pvital
left a comment
There was a problem hiding this comment.
Great work @CagriYonca.
I have a few considerations.
| # Eventlet deferred boot: if eventlet is present in the environment, | ||
| # defer boot_agent() until after eventlet.monkey_patch() has been called | ||
| # (which happens in each worker process after fork). This prevents the | ||
| # ssl.SSLContext global-rebind RecursionError seen with gunicorn eventlet workers. | ||
| if ( | ||
| (is_autowrapt_instrumented() or is_webhook_instrumented()) | ||
| and "INSTANA_DISABLE_AUTO_INSTR" not in os.environ | ||
| and importlib_util.find_spec("eventlet") | ||
| ): | ||
| _defer_boot_until_eventlet_patch() |
There was a problem hiding this comment.
Let me understand here. If gevent and eventlet are used at the same time (I don't know if that's possible or not), first we apply the gevent monkey patch and then the eventlet monkey patch?
Anyway, I see some repetitive code here; I guess we can reduce this to something like (consider the change starting on line 299):
| # Eventlet deferred boot: if eventlet is present in the environment, | |
| # defer boot_agent() until after eventlet.monkey_patch() has been called | |
| # (which happens in each worker process after fork). This prevents the | |
| # ssl.SSLContext global-rebind RecursionError seen with gunicorn eventlet workers. | |
| if ( | |
| (is_autowrapt_instrumented() or is_webhook_instrumented()) | |
| and "INSTANA_DISABLE_AUTO_INSTR" not in os.environ | |
| and importlib_util.find_spec("eventlet") | |
| ): | |
| _defer_boot_until_eventlet_patch() | |
| if ( | |
| (is_autowrapt_instrumented() or is_webhook_instrumented()) | |
| and "INSTANA_DISABLE_AUTO_INSTR" not in os.environ | |
| ): | |
| # Automatic gevent monkey patching | |
| # unless auto instrumentation is off, then the customer should do manual gevent monkey patching | |
| if importlib_util.find_spec("gevent"): | |
| apply_gevent_monkey_patch() | |
| # Eventlet deferred boot: if eventlet is present in the environment, | |
| # defer boot_agent() until after eventlet.monkey_patch() has been called | |
| # (which happens in each worker process after fork). This prevents the | |
| # ssl.SSLContext global-rebind RecursionError seen with gunicorn eventlet workers. | |
| if importlib_util.find_spec("eventlet"): | |
| _defer_boot_until_eventlet_patch() |
There was a problem hiding this comment.
I've refactored the block as you suggested, the shared condition is now extracted into a single outer block, with gevent and eventlet checks nested inside.
gevent and eventlet are not used simultaneously, these are Gunicorn's worker classes and they serve the same purpose. The two inner checks are independent branches for each case.
…cursion error Signed-off-by: Cagri Yonca <cagri@ibm.com>
c0dbd67 to
38966d8
Compare
| if is_truthy(os.environ.get("INSTANA_AUTOPROFILE", None)): | ||
| _start_profiler() | ||
|
|
||
| boot_agent() |
There was a problem hiding this comment.
If is_worker is True, boot_agent() is called twice
| if is_worker: | ||
| if is_truthy(os.environ.get("INSTANA_AUTOPROFILE", None)): | ||
| _start_profiler() | ||
| boot_agent() |
| args: tuple[object, ...], | ||
| kwargs: dict[str, object], | ||
| ) -> Optional[object]: | ||
| result = wrapped(*args, **kwargs) |
There was a problem hiding this comment.
eventlet.monkey_patch() returns None
| result = wrapped(*args, **kwargs) | |
| wrapped(*args, **kwargs) |
| if is_truthy(os.environ.get("INSTANA_AUTOPROFILE", None)): | ||
| _start_profiler() | ||
| boot_agent() | ||
| return result |
There was a problem hiding this comment.
| return result |
Reason: same as here
| instance: object, | ||
| args: tuple[object, ...], | ||
| kwargs: dict[str, object], | ||
| ) -> Optional[object]: |
There was a problem hiding this comment.
| ) -> Optional[object]: | |
| ) -> None: |
Reason: same as here
| import sys | ||
| from importlib import util as importlib_util | ||
| from typing import Tuple | ||
| from typing import Optional |
There was a problem hiding this comment.
| from typing import Optional |
Reason: same as here
Problem & Root Cause
When running under Gunicorn with the
eventletworker class, Instana's early auto-instrumentation eagerly imports third-party libraries in the master process beforeeventlet.monkey_patchexecutes.Modules such as
urllib3andpymongocapture module-level references to the unpatchedssl.SSLContext. When Eventlet subsequently patches thesslmodule,ssl.SSLContextis replaced with its green subclass. Initializing SSL connections later causessuperlookups inside property setters to resolve the global green subclass instead of the original class, resulting in an infinite recursion and aRecursionError.Solution
This PR defers
boot_agentexecution wheneventletis present in the environment:eventlet.monkey_patchusingwrapt.boot_agentand all eager instrumentation imports until aftereventlet.monkey_patchcompletes in the forked worker process.Changes
src/instana/__init__.py: Added_defer_boot_until_eventlet_patchto defer agent boot until worker post-patch.tests/frameworks/test_eventlet_autotrace.py: Added unit tests covering deferred boot registration, worker execution, and arbiter suppression.