ignore elasticapm frames from frame collection logic for spans (elast…

…ic#167)

this should clean up the collected stack traces quite a bit

Note, for Django this has been done a bit differently, this
specialization has been removed.

closes elastic#167

elasticapm/base.py

@@ -129,9 +129,9 @@ def __init__(self, config=None, **defaults):
 
         self.processors = [import_string(p) for p in self.config.processors] if self.config.processors else []
 
-        self.instrumentation_store = TransactionsStore(
-            lambda: self._get_stack_info_for_trace(
-                stacks.iter_stack_frames(),
+        def frames_collector_func():
+            return self._get_stack_info_for_trace(
+                stacks.iter_stack_frames(ignore_modules=('elasticapm.',)),
                 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'),
@@ -140,7 +140,10 @@ def __init__(self, config=None, **defaults):
                     list_length=self.config.local_var_list_max_length,
                     string_length=self.config.local_var_max_length,
                 ), local_var)
-            ),
+            )
+
+        self.instrumentation_store = TransactionsStore(
+            frames_collector_func=frames_collector_func,
             collect_frequency=self.config.flush_interval,
             sample_rate=self.config.transaction_sample_rate,
             max_spans=self.config.transaction_max_spans,

elasticapm/contrib/django/client.py

@@ -179,7 +179,7 @@ def _get_stack_info_for_trace(self, frames,
                                   locals_processor_func=None):
         """If the stacktrace originates within the elasticapm module, it will skip
         frames until some other module comes up."""
-        frames = list(iterate_with_template_sources(
+        return list(iterate_with_template_sources(
             frames,
             with_locals=with_locals,
             library_frame_context_lines=library_frame_context_lines,
@@ -188,15 +188,6 @@ def _get_stack_info_for_trace(self, frames,
             exclude_paths_re=self.exclude_paths_re,
             locals_processor_func=locals_processor_func,
         ))
-        i = 0
-        while len(frames) > i:
-            if 'module' in frames[i] and not (
-                    frames[i]['module'].startswith('elasticapm.') or
-                    frames[i]['module'] == 'contextlib'
-            ):
-                return frames[i:]
-            i += 1
-        return frames
 
     def send(self, url, **kwargs):
         """

elasticapm/instrumentation/packages/base.py

@@ -25,7 +25,6 @@ def __call__(self, *args, **kwargs):
         # 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():
@@ -98,7 +97,6 @@ def __call__(self, *args, **kwargs):
         # 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():

elasticapm/utils/stacks.py

@@ -138,7 +138,7 @@ def to_dict(dictish):
     return dict((k, dictish[k]) for k in m())
 
 
-def iter_traceback_frames(tb):
+def iter_traceback_frames(tb, ignore_modules=()):
     """
     Given a traceback object, it will iterate over all
     frames that do not contain the ``__traceback_hide__``
@@ -147,13 +147,16 @@ def iter_traceback_frames(tb):
     while tb:
         # support for __traceback_hide__ which is used by a few libraries
         # to hide internal frames.
-        f_locals = getattr(tb.tb_frame, 'f_locals', {})
-        if not _getitem_from_frame(f_locals, '__traceback_hide__'):
-            yield tb.tb_frame, getattr(tb, 'tb_lineno', None)
+        frame = tb.tb_frame
+        f_locals = getattr(frame, 'f_locals', {})
+        f_globals = getattr(frame, 'f_globals', {})
+        if not (f_globals.get('__name__', '').startswith(ignore_modules) or
+                _getitem_from_frame(f_locals, '__traceback_hide__')):
+            yield frame, getattr(tb, 'tb_lineno', None)
         tb = tb.tb_next
 
 
-def iter_stack_frames(frames=None, skip=0):
+def iter_stack_frames(frames=None, skip=0, ignore_modules=()):
     """
     Given an optional list of frames (defaults to current stack),
     iterates over all frames that do not contain the ``__traceback_hide__``
@@ -166,7 +169,9 @@ def iter_stack_frames(frames=None, skip=0):
         if i < skip:
             continue
         f_locals = getattr(frame, 'f_locals', {})
-        if not _getitem_from_frame(f_locals, '__traceback_hide__'):
+        f_globals = getattr(frame, 'f_globals', {})
+        if not (f_globals.get('__name__', '').startswith(ignore_modules) or
+                _getitem_from_frame(f_locals, '__traceback_hide__')):
             yield frame, frame.f_lineno
 
 

tests/client/client_tests.py

@@ -552,7 +552,7 @@ def test_collect_local_variables_transactions(should_collect, elasticapm_client)
         pass
     elasticapm_client.end_transaction('test', 'ok')
     transaction = elasticapm_client.instrumentation_store.get_all()[0]
-    frame = transaction['spans'][0]['stacktrace'][5]
+    frame = transaction['spans'][0]['stacktrace'][0]
     if mode in ('transactions', 'all'):
         assert 'vars' in frame, mode
         assert frame['vars']['a_local_var'] == 1
@@ -578,8 +578,8 @@ def test_collect_source_transactions(should_collect, elasticapm_client):
         pass
     elasticapm_client.end_transaction('test', 'ok')
     transaction = elasticapm_client.instrumentation_store.get_all()[0]
-    in_app_frame = transaction['spans'][0]['stacktrace'][5]
-    library_frame = transaction['spans'][0]['stacktrace'][6]
+    in_app_frame = transaction['spans'][0]['stacktrace'][0]
+    library_frame = transaction['spans'][0]['stacktrace'][1]
     assert not in_app_frame['library_frame']
     assert library_frame['library_frame']
     if library_frame_context:

tests/contrib/django/django_tests.py

@@ -966,7 +966,7 @@ def test_stacktrace_filtered_for_elasticapm(client, django_elasticapm_client):
     assert spans[1]['name'] == 'list_users.html'
 
     # Top frame should be inside django rendering
-    assert spans[1]['stacktrace'][0]['module'].startswith('django.template')
+    assert spans[1]['stacktrace'][0]['module'].startswith('django.template'), spans[1]['stacktrace'][0]['function']
 
 
 @pytest.mark.parametrize('django_elasticapm_client', [{'_wait_to_first_send': 100}], indirect=True)

tests/instrumentation/base_tests.py

@@ -4,6 +4,7 @@
 import mock
 import pytest
 
+import elasticapm
 from elasticapm.instrumentation.packages.base import (AbstractInstrumentedModule,
                                                       OriginalNamesBoundFunctionWrapper)
 from elasticapm.utils import compat
@@ -79,3 +80,12 @@ def test_skip_instrument_env_var():
     with mock.patch.dict('os.environ', {'SKIP_INSTRUMENT_TEST_DUMMY_INSTRUMENT': 'foo'}):
         instrumentation.instrument()
     assert not instrumentation.instrumented
+
+
+def test_skip_ignored_frames(elasticapm_client):
+    elasticapm_client.begin_transaction('test')
+    with elasticapm.capture_span('test'):
+        pass
+    transaction = elasticapm_client.end_transaction('test', 'test')
+    for frame in transaction.spans[0].frames:
+        assert not frame['module'].startswith('elasticapm')