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 all 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
70 changes: 55 additions & 15 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,11 +386,19 @@ 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 by default
if hub.client and hub.client.options["_experiments"].get(
"record_sql_params", False
):
if not params_list or params_list == [None]:
params_list = None

if paramstyle == "pyformat":
paramstyle = "format"
else:
params_list = None
paramstyle = None

query = _format_sql(cursor, query)

Expand Down
26 changes: 22 additions & 4 deletions tests/integrations/django/test_basic.py
Expand Up @@ -170,7 +170,9 @@ def test_sql_queries(sentry_init, capture_events, with_integration):
sentry_init(
integrations=[DjangoIntegration()] if with_integration else [],
send_default_pii=True,
_experiments={"record_sql_params": True},
)

from django.db import connection

sql = connection.cursor()
Expand All @@ -193,7 +195,11 @@ def test_sql_queries(sentry_init, capture_events, with_integration):

@pytest.mark.django_db
def test_sql_dict_query_params(sentry_init, capture_events):
sentry_init(integrations=[DjangoIntegration()], send_default_pii=True)
sentry_init(
integrations=[DjangoIntegration()],
send_default_pii=True,
_experiments={"record_sql_params": True},
)

from django.db import connections

Expand Down Expand Up @@ -230,7 +236,11 @@ def test_sql_dict_query_params(sentry_init, capture_events):
)
@pytest.mark.django_db
def test_sql_psycopg2_string_composition(sentry_init, capture_events, query):
sentry_init(integrations=[DjangoIntegration()], send_default_pii=True)
sentry_init(
integrations=[DjangoIntegration()],
send_default_pii=True,
_experiments={"record_sql_params": True},
)
from django.db import connections

if "postgres" not in connections:
Expand All @@ -254,7 +264,11 @@ def test_sql_psycopg2_string_composition(sentry_init, capture_events, query):

@pytest.mark.django_db
def test_sql_psycopg2_placeholders(sentry_init, capture_events):
sentry_init(integrations=[DjangoIntegration()], send_default_pii=True)
sentry_init(
integrations=[DjangoIntegration()],
send_default_pii=True,
_experiments={"record_sql_params": True},
)
from django.db import connections

if "postgres" not in connections:
Expand Down Expand Up @@ -499,7 +513,11 @@ def test_does_not_capture_403(sentry_init, client, capture_events, endpoint):


def test_middleware_spans(sentry_init, client, capture_events):
sentry_init(integrations=[DjangoIntegration()], traces_sample_rate=1.0)
sentry_init(
integrations=[DjangoIntegration()],
traces_sample_rate=1.0,
_experiments={"record_sql_params": True},
)
events = capture_events()

_content, status, _headers = client.get(reverse("message"))
Expand Down
4 changes: 3 additions & 1 deletion tests/integrations/sqlalchemy/test_sqlalchemy.py
Expand Up @@ -8,7 +8,9 @@


def test_orm_queries(sentry_init, capture_events):
sentry_init(integrations=[SqlalchemyIntegration()])
sentry_init(
integrations=[SqlalchemyIntegration()], _experiments={"record_sql_params": True}
)
events = capture_events()

Base = declarative_base()
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"