Skip to content

Commit

Permalink
ignore frames from frame collection logic
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
beniwohli committed Feb 20, 2018
1 parent c99dd94 commit 3345df1
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 19 deletions.
13 changes: 10 additions & 3 deletions elasticapm/base.py
Expand Up @@ -129,8 +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(
def frames_collector_func():
__traceback_hide__ = True # noqa F841
return self._get_stack_info_for_trace(
stacks.iter_stack_frames(),
library_frame_context_lines=self.config.source_lines_span_library_frames,
in_app_frame_context_lines=self.config.source_lines_span_app_frames,
Expand All @@ -140,7 +141,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,
Expand Down Expand Up @@ -538,6 +542,9 @@ def _get_stack_info_for_trace(self, frames,
with_locals=True,
locals_processor_func=None):
"""Overrideable in derived clients to add frames/info, e.g. templates"""

__traceback_hide__ = True # noqa F841

return stacks.get_stack_info(
frames,
library_frame_context_lines=library_frame_context_lines,
Expand Down
14 changes: 4 additions & 10 deletions elasticapm/contrib/django/client.py
Expand Up @@ -179,7 +179,10 @@ 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(

__traceback_hide__ = True # noqa F841

return list(iterate_with_template_sources(
frames,
with_locals=with_locals,
library_frame_context_lines=library_frame_context_lines,
Expand All @@ -188,15 +191,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):
"""
Expand Down
2 changes: 2 additions & 0 deletions elasticapm/contrib/django/utils.py
Expand Up @@ -12,6 +12,8 @@ class Template(object):
def iterate_with_template_sources(frames, with_locals=True,
library_frame_context_lines=None, in_app_frame_context_lines=None,
include_paths_re=None, exclude_paths_re=None, locals_processor_func=None):
__traceback_hide__ = True # noqa F841

template = None
for frame, lineno in frames:
f_code = getattr(frame, 'f_code', None)
Expand Down
5 changes: 3 additions & 2 deletions elasticapm/instrumentation/packages/base.py
Expand Up @@ -25,7 +25,7 @@ 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.

__traceback_hide__ = True # noqa F841
if self._self_enabled is not None:
if callable(self._self_enabled):
if not self._self_enabled():
Expand Down Expand Up @@ -98,7 +98,7 @@ 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.

__traceback_hide__ = True # noqa F841
if self._self_enabled is not None:
if callable(self._self_enabled):
if not self._self_enabled():
Expand Down Expand Up @@ -225,6 +225,7 @@ def uninstrument(self):
self.originals = {}

def call_if_sampling(self, module, method, wrapped, instance, args, kwargs):
__traceback_hide__ = True # noqa F841
transaction = get_transaction()
if not transaction or not transaction.is_sampled:
return wrapped(*args, **kwargs)
Expand Down
4 changes: 4 additions & 0 deletions elasticapm/instrumentation/packages/dbapi2.py
Expand Up @@ -165,13 +165,16 @@ class CursorProxy(wrapt.ObjectProxy):
provider_name = None

def callproc(self, procname, params=None):
__traceback_hide__ = True # noqa F841
return self._trace_sql(self.__wrapped__.callproc, procname,
params)

def execute(self, sql, params=None):
__traceback_hide__ = True # noqa F841
return self._trace_sql(self.__wrapped__.execute, sql, params)

def executemany(self, sql, param_list):
__traceback_hide__ = True # noqa F841
return self._trace_sql(self.__wrapped__.executemany, sql,
param_list)

Expand All @@ -183,6 +186,7 @@ def _bake_sql(self, sql):
return sql

def _trace_sql(self, method, sql, params):
__traceback_hide__ = True # noqa F841
sql_string = self._bake_sql(sql)
signature = self.extract_signature(sql_string)
kind = "db.{0}.sql".format(self.provider_name)
Expand Down
2 changes: 2 additions & 0 deletions elasticapm/instrumentation/packages/django/template.py
Expand Up @@ -10,6 +10,7 @@ class DjangoTemplateInstrumentation(AbstractInstrumentedModule):
]

def call(self, module, method, wrapped, instance, args, kwargs):
__traceback_hide__ = True # noqa F841
name = getattr(instance, 'name', None)

if not name:
Expand All @@ -25,6 +26,7 @@ class DjangoTemplateSourceInstrumentation(AbstractInstrumentedModule):
]

def call(self, module, method, wrapped, instance, args, kwargs):
__traceback_hide__ = True # noqa F841
ret = wrapped(*args, **kwargs)

if len(args) > 1:
Expand Down
3 changes: 3 additions & 0 deletions elasticapm/traces.py
Expand Up @@ -89,6 +89,8 @@ def begin_span(self, name, span_type, context=None, leaf=False):
return span

def end_span(self, skip_frames):
__traceback_hide__ = True # noqa F841

span = self.span_stack.pop()
if span is IGNORED_SPAN:
return None
Expand Down Expand Up @@ -261,6 +263,7 @@ def __enter__(self):
def __exit__(self, exc_type, exc_val, exc_tb):
transaction = get_transaction()
if transaction and transaction.is_sampled:
__traceback_hide__ = True # noqa F841
transaction.end_span(self.skip_frames)


Expand Down
3 changes: 3 additions & 0 deletions elasticapm/utils/stacks.py
Expand Up @@ -253,6 +253,9 @@ def get_stack_info(frames, with_locals=True,
:param locals_processor_func: a function to call on all local variables
:return:
"""

__traceback_hide__ = True # noqa F841

results = []
for frame, lineno in frames:
result = get_frame_info(
Expand Down
6 changes: 3 additions & 3 deletions tests/client/client_tests.py
Expand Up @@ -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
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/contrib/django/django_tests.py
Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions tests/instrumentation/base_tests.py
Expand Up @@ -4,6 +4,7 @@
import mock
import pytest

import elasticapm
from elasticapm.instrumentation.packages.base import (AbstractInstrumentedModule,
OriginalNamesBoundFunctionWrapper)
from elasticapm.utils import compat
Expand Down Expand Up @@ -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')

0 comments on commit 3345df1

Please sign in to comment.