Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Do not capture SQL params for now #503

Merged
merged 5 commits into from Sep 19, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions sentry_sdk/consts.py
Expand Up @@ -49,6 +49,7 @@ def __init__(
# DO NOT ENABLE THIS RIGHT NOW UNLESS YOU WANT TO EXCEED YOUR EVENT QUOTA IMMEDIATELY
traces_sample_rate=0.0, # type: float
traceparent_v2=False, # type: bool
_experiments={}, # type: Dict[str, Any]
):
# type: (...) -> None
pass
Expand Down
5 changes: 4 additions & 1 deletion sentry_sdk/hub.py
Expand Up @@ -450,7 +450,10 @@ def start_span(
span.sampled = random.random() < sample_rate

if span.sampled:
span.init_finished_spans()
max_spans = (
client and client.options["_experiments"].get("max_spans") or 1000
)
span.init_finished_spans(maxlen=max_spans)

return span

Expand Down
63 changes: 45 additions & 18 deletions sentry_sdk/tracing.py
Expand Up @@ -64,6 +64,31 @@ def __iter__(self):
yield k[len(self.prefix) :]


class _SpanRecorder(object):
__slots__ = ("maxlen", "finished_spans", "open_span_count")

def __init__(self, maxlen):
# type: (int) -> None
self.maxlen = maxlen
self.open_span_count = 0 # type: int
self.finished_spans = [] # type: List[Span]

def start_span(self, span):
# type: (Span) -> None

# This is just so that we don't run out of memory while recording a lot
# of spans. At some point we just stop and flush out the start of the
# trace tree (i.e. the first n spans with the smallest
# start_timestamp).
self.open_span_count += 1
if self.open_span_count > self.maxlen:
span._span_recorder = None

def finish_span(self, span):
# type: (Span) -> None
self.finished_spans.append(span)


class Span(object):
__slots__ = (
"trace_id",
Expand All @@ -78,7 +103,7 @@ class Span(object):
"timestamp",
"_tags",
"_data",
"_finished_spans",
"_span_recorder",
"hub",
"_context_manager_state",
)
Expand Down Expand Up @@ -107,16 +132,18 @@ def __init__(
self.hub = hub
self._tags = {} # type: Dict[str, str]
self._data = {} # type: Dict[str, Any]
self._finished_spans = None # type: Optional[List[Span]]
self.start_timestamp = datetime.now()

#: End timestamp of span
self.timestamp = None # type: Optional[datetime]

def init_finished_spans(self):
# type: () -> None
if self._finished_spans is None:
self._finished_spans = []
self._span_recorder = None # type: Optional[_SpanRecorder]

def init_finished_spans(self, maxlen):
# type: (int) -> None
if self._span_recorder is None:
self._span_recorder = _SpanRecorder(maxlen)
self._span_recorder.start_span(self)

def __repr__(self):
# type: () -> str
Expand Down Expand Up @@ -162,7 +189,8 @@ def new_span(self, **kwargs):
sampled=self.sampled,
**kwargs
)
rv._finished_spans = self._finished_spans

rv._span_recorder = self._span_recorder
return rv

@classmethod
Expand Down Expand Up @@ -252,11 +280,13 @@ def finish(self, hub=None):

self.timestamp = datetime.now()

if self._finished_spans is not None:
self._finished_spans.append(self)

_maybe_create_breadcrumbs_from_span(hub, self)

if self._span_recorder is None:
return None

self._span_recorder.finish_span(self)

if self.transaction is None:
# If this has no transaction set we assume there's a parent
# transaction for this span that would be flushed out eventually.
Expand Down Expand Up @@ -285,7 +315,9 @@ def finish(self, hub=None):
"timestamp": self.timestamp,
"start_timestamp": self.start_timestamp,
"spans": [
s.to_json() for s in (self._finished_spans or ()) if s is not self
s.to_json()
for s in self._span_recorder.finished_spans
if s is not self
],
}
)
Expand Down Expand Up @@ -354,15 +386,10 @@ def record_sql_queries(
executemany, # type: bool
):
# type: (...) -> Generator[Span, None, None]
if not params_list or params_list == [None]:
params_list = None

if paramstyle == "pyformat":
paramstyle = "format"

# TODO: Bring back capturing of params
query = _format_sql(cursor, query)

data = {"db.params": params_list, "db.paramstyle": paramstyle}
data = {}
if executemany:
data["db.executemany"] = True

Expand Down
10 changes: 2 additions & 8 deletions tests/integrations/django/test_basic.py
Expand Up @@ -188,7 +188,6 @@ def test_sql_queries(sentry_init, capture_events, with_integration):
crumb = event["breadcrumbs"][-1]

assert crumb["message"] == "SELECT count(*) FROM people_person WHERE foo = %s"
assert crumb["data"]["db.params"] == [123]


@pytest.mark.django_db
Expand Down Expand Up @@ -216,7 +215,6 @@ def test_sql_dict_query_params(sentry_init, capture_events):
assert crumb["message"] == (
"SELECT count(*) FROM people_person WHERE foo = %(my_foo)s"
)
assert crumb["data"]["db.params"] == {"my_foo": 10}


@pytest.mark.parametrize(
Expand Down Expand Up @@ -249,7 +247,6 @@ def test_sql_psycopg2_string_composition(sentry_init, capture_events, query):
event, = events
crumb = event["breadcrumbs"][-1]
assert crumb["message"] == ('SELECT %(my_param)s FROM "foobar"')
assert crumb["data"]["db.params"] == {"my_param": 10}


@pytest.mark.django_db
Expand Down Expand Up @@ -288,16 +285,13 @@ def test_sql_psycopg2_placeholders(sentry_init, capture_events):
assert event["breadcrumbs"][-2:] == [
{
"category": "query",
"data": {"db.paramstyle": "format"},
"data": {},
"message": "create table my_test_table (foo text, bar date)",
"type": "default",
},
{
"category": "query",
"data": {
"db.params": {"first_var": "fizz", "second_var": "not a date"},
"db.paramstyle": "format",
},
"data": {},
"message": 'insert into my_test_table ("foo", "bar") values (%(first_var)s, '
"%(second_var)s)",
"type": "default",
Expand Down
4 changes: 2 additions & 2 deletions tests/integrations/sqlalchemy/test_sqlalchemy.py
Expand Up @@ -48,13 +48,13 @@ class Address(Base):
assert event["breadcrumbs"][-2:] == [
{
"category": "query",
"data": {"db.params": ["Bob"], "db.paramstyle": "qmark"},
"data": {},
"message": "INSERT INTO person (name) VALUES (?)",
"type": "default",
},
{
"category": "query",
"data": {"db.params": [1, 0], "db.paramstyle": "qmark"},
"data": {},
"message": "SELECT person.id AS person_id, person.name AS person_name \n"
"FROM person\n"
" LIMIT ? OFFSET ?",
Expand Down
15 changes: 15 additions & 0 deletions tests/test_tracing.py
Expand Up @@ -127,3 +127,18 @@ def foo():
gc.collect()

assert len(references) == expected_refcount


def test_span_trimming(sentry_init, capture_events):
sentry_init(traces_sample_rate=1.0, _experiments={"max_spans": 3})
events = capture_events()

with Hub.current.start_span(transaction="hi"):
for i in range(10):
with Hub.current.start_span(op="foo{}".format(i)):
pass

event, = events
span1, span2 = event["spans"]
assert span1["op"] == "foo0"
assert span2["op"] == "foo1"