diff --git a/elasticapm/base.py b/elasticapm/base.py index 5a7a20932..01d2b2472 100644 --- a/elasticapm/base.py +++ b/elasticapm/base.py @@ -129,9 +129,16 @@ def __init__(self, config=None, **defaults): self.processors = [import_string(p) for p in self.config.processors] if self.config.processors else [] + if platform.python_implementation() == 'PyPy': + # PyPy introduces a `_functools.partial.__call__` frame due to our use + # of `partial` in AbstractInstrumentedModule + skip_modules = ('elasticapm.', '_functools') + else: + skip_modules = ('elasticapm.',) + def frames_collector_func(): return self._get_stack_info_for_trace( - stacks.iter_stack_frames(skip_top_modules=('elasticapm.',)), + stacks.iter_stack_frames(skip_top_modules=skip_modules), library_frame_context_lines=self.config.source_lines_span_library_frames, in_app_frame_context_lines=self.config.source_lines_span_app_frames, with_locals=self.config.collect_local_variables in ('all', 'transactions'), diff --git a/elasticapm/instrumentation/packages/base.py b/elasticapm/instrumentation/packages/base.py index 7826921a9..dc3ab9cea 100644 --- a/elasticapm/instrumentation/packages/base.py +++ b/elasticapm/instrumentation/packages/base.py @@ -8,129 +8,6 @@ logger = logging.getLogger(__name__) -# We have our own `BoundFunctionWrapper` and `FunctionWrapper` here because -# we want them to be able to know about `module` and `method` and supply it in -# the call to the wrapper. - -class OriginalNamesBoundFunctionWrapper(wrapt.BoundFunctionWrapper): - - def __init__(self, *args, **kwargs): - super(OriginalNamesBoundFunctionWrapper, self).__init__(*args, **kwargs) - self._self_module = self._self_parent._self_module - self._self_method = self._self_parent._self_method - - def __call__(self, *args, **kwargs): - # If enabled has been specified, then evaluate it at this point - # and if the wrapper is not to be executed, then simply return - # the bound function rather than a bound wrapper for the bound - # function. When evaluating enabled, if it is callable we call - # it, otherwise we evaluate it as a boolean. - - if self._self_enabled is not None: - if callable(self._self_enabled): - if not self._self_enabled(): - return self.__wrapped__(*args, **kwargs) - elif not self._self_enabled: - return self.__wrapped__(*args, **kwargs) - - # We need to do things different depending on whether we are - # likely wrapping an instance method vs a static method or class - # method. - - if self._self_binding == 'function': - if self._self_instance is None: - # This situation can occur where someone is calling the - # instancemethod via the class type and passing the instance - # as the first argument. We need to shift the args before - # making the call to the wrapper and effectively bind the - # instance to the wrapped function using a partial so the - # wrapper doesn't see anything as being different. - - if not args: - raise TypeError( - 'missing 1 required positional argument') - - instance, args = args[0], args[1:] - wrapped = functools.partial(self.__wrapped__, instance) - return self._self_wrapper(self._self_module, - self._self_method, - wrapped, instance, args, kwargs) - - return self._self_wrapper(self._self_module, - self._self_method, - self.__wrapped__, self._self_instance, - args, kwargs) - - else: - # As in this case we would be dealing with a classmethod or - # staticmethod, then _self_instance will only tell us whether - # when calling the classmethod or staticmethod they did it via an - # instance of the class it is bound to and not the case where - # done by the class type itself. We thus ignore _self_instance - # and use the __self__ attribute of the bound function instead. - # For a classmethod, this means instance will be the class type - # and for a staticmethod it will be None. This is probably the - # more useful thing we can pass through even though we loose - # knowledge of whether they were called on the instance vs the - # class type, as it reflects what they have available in the - # decoratored function. - - instance = getattr(self.__wrapped__, '__self__', None) - - return self._self_wrapper(self._self_module, - self._self_method, - self.__wrapped__, instance, args, - kwargs) - - -class OriginalNamesFunctionWrapper(wrapt.FunctionWrapper): - - __bound_function_wrapper__ = OriginalNamesBoundFunctionWrapper - - def __init__(self, wrapped, wrapper, module, method): - super(OriginalNamesFunctionWrapper, self).__init__(wrapped, wrapper) - self._self_module = module - self._self_method = method - - def __call__(self, *args, **kwargs): - # If enabled has been specified, then evaluate it at this point - # and if the wrapper is not to be executed, then simply return - # the bound function rather than a bound wrapper for the bound - # function. When evaluating enabled, if it is callable we call - # it, otherwise we evaluate it as a boolean. - - if self._self_enabled is not None: - if callable(self._self_enabled): - if not self._self_enabled(): - return self.__wrapped__(*args, **kwargs) - elif not self._self_enabled: - return self.__wrapped__(*args, **kwargs) - - # This can occur where initial function wrapper was applied to - # a function that was already bound to an instance. In that case - # we want to extract the instance from the function and use it. - - if self._self_binding == 'function': - if self._self_instance is None: - instance = getattr(self.__wrapped__, '__self__', None) - if instance is not None: - return self._self_wrapper(self._self_module, - self._self_method, - self.__wrapped__, instance, - args, kwargs) - - # This is generally invoked when the wrapped function is being - # called as a normal function and is not bound to a class as an - # instance method. This is also invoked in the case where the - # wrapped function was a method, but this wrapper was in turn - # wrapped using the staticmethod decorator. - - return self._self_wrapper(self._self_module, - self._self_method, - self.__wrapped__, self._self_instance, - args, kwargs) - - class AbstractInstrumentedModule(object): name = None @@ -188,11 +65,9 @@ def instrument(self): # `module`/`method` in the call to `call_if_sampling` parent, attribute, original = wrapt.resolve_path(module, method) self.originals[(module, method)] = original - wrapper = OriginalNamesFunctionWrapper( + wrapper = wrapt.FunctionWrapper( original, - self.call_if_sampling, - module, - method + functools.partial(self.call_if_sampling, module, method), ) wrapt.apply_patch(parent, attribute, wrapper) except ImportError: diff --git a/tests/instrumentation/base_tests.py b/tests/instrumentation/base_tests.py index c10e08f0d..ea401e21a 100644 --- a/tests/instrumentation/base_tests.py +++ b/tests/instrumentation/base_tests.py @@ -5,14 +5,13 @@ import pytest import elasticapm -from elasticapm.instrumentation.packages.base import (AbstractInstrumentedModule, - OriginalNamesBoundFunctionWrapper) -from elasticapm.utils import compat +from elasticapm.instrumentation.packages.base import AbstractInstrumentedModule +from elasticapm.utils import compat, wrapt class Dummy(object): - def dummy(self): - pass + def dummy(self, call_args=None): + return call_args class _TestInstrumentNonExistingFunctionOnModule(AbstractInstrumentedModule): @@ -36,6 +35,12 @@ class _TestDummyInstrumentation(AbstractInstrumentedModule): ("tests.instrumentation.base_tests", "Dummy.dummy"), ] + def call(self, module, method, wrapped, instance, args, kwargs): + kwargs = kwargs or {} + kwargs['call_args'] = (module, method) + return wrapped(*args, **kwargs) + + def test_instrument_nonexisting_method_on_module(): _TestInstrumentNonExistingFunctionOnModule().instrument() @@ -48,31 +53,47 @@ def test_instrument_nonexisting_method(): @pytest.mark.skipif(compat.PY3, reason="different object model") def test_uninstrument_py2(): assert isinstance(Dummy.dummy, types.MethodType) - assert not isinstance(Dummy.dummy, OriginalNamesBoundFunctionWrapper) + assert not isinstance(Dummy.dummy, wrapt.BoundFunctionWrapper) instrumentation = _TestDummyInstrumentation() instrumentation.instrument() - assert isinstance(Dummy.dummy, OriginalNamesBoundFunctionWrapper) + assert isinstance(Dummy.dummy, wrapt.BoundFunctionWrapper) instrumentation.uninstrument() assert isinstance(Dummy.dummy, types.MethodType) - assert not isinstance(Dummy.dummy, OriginalNamesBoundFunctionWrapper) + assert not isinstance(Dummy.dummy, wrapt.BoundFunctionWrapper) @pytest.mark.skipif(compat.PY2, reason="different object model") def test_uninstrument_py3(): original = Dummy.dummy - assert not isinstance(Dummy.dummy, OriginalNamesBoundFunctionWrapper) + assert not isinstance(Dummy.dummy, wrapt.BoundFunctionWrapper) instrumentation = _TestDummyInstrumentation() instrumentation.instrument() assert Dummy.dummy is not original - assert isinstance(Dummy.dummy, OriginalNamesBoundFunctionWrapper) + assert isinstance(Dummy.dummy, wrapt.BoundFunctionWrapper) instrumentation.uninstrument() assert Dummy.dummy is original - assert not isinstance(Dummy.dummy, OriginalNamesBoundFunctionWrapper) + assert not isinstance(Dummy.dummy, wrapt.BoundFunctionWrapper) + + +def test_module_method_args(elasticapm_client): + """ + Test that the module/method arguments are correctly passed to + the _TestDummyInstrumentation.call method + """ + instrumentation = _TestDummyInstrumentation() + instrumentation.instrument() + elasticapm_client.begin_transaction('test') + dummy = Dummy() + call_args = dummy.dummy() + elasticapm_client.end_transaction('test', 'test') + instrumentation.uninstrument() + + assert call_args == ('tests.instrumentation.base_tests', 'Dummy.dummy') def test_skip_instrument_env_var():