Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 51 additions & 22 deletions sentry_sdk/integrations/sqlalchemy.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
ensure_integration_enabled,
parse_version,
)
from sentry_sdk.traces import StreamedSpan, SpanStatus

try:
from sqlalchemy.engine import Engine # type: ignore
Expand All @@ -20,6 +21,7 @@
from typing import Any
from typing import ContextManager
from typing import Optional
from typing import Union

from sentry_sdk.tracing import Span

Expand Down Expand Up @@ -96,7 +98,10 @@ def _handle_error(context: "Any", *args: "Any") -> None:
span: "Optional[Span]" = getattr(execution_context, "_sentry_sql_span", None)

if span is not None:
span.set_status(SPANSTATUS.INTERNAL_ERROR)
if isinstance(span, StreamedSpan):
span.status = SpanStatus.ERROR
else:
span.set_status(SPANSTATUS.INTERNAL_ERROR)
Comment thread
alexander-alderman-webb marked this conversation as resolved.

# _after_cursor_execute does not get called for crashing SQL stmts. Judging
# from SQLAlchemy codebase it does seem like any error coming into this
Comment thread
alexander-alderman-webb marked this conversation as resolved.
Expand Down Expand Up @@ -132,29 +137,53 @@ def _get_db_system(name: str) -> "Optional[str]":
return None


def _set_db_data(span: "Span", conn: "Any") -> None:
def _set_db_data(span: "Union[Span, StreamedSpan]", conn: "Any") -> None:
db_system = _get_db_system(conn.engine.name)
if db_system is not None:
span.set_data(SPANDATA.DB_SYSTEM, db_system)

try:
driver = conn.dialect.driver
if driver:
span.set_data(SPANDATA.DB_DRIVER_NAME, driver)
except Exception:
pass
if isinstance(span, StreamedSpan):
if db_system is not None:
span.set_attribute(SPANDATA.DB_SYSTEM, db_system)

try:
driver = conn.dialect.driver
if driver:
span.set_attribute(SPANDATA.DB_DRIVER_NAME, driver)
except Exception:
pass
else:
if db_system is not None:
span.set_data(SPANDATA.DB_SYSTEM, db_system)

try:
driver = conn.dialect.driver
if driver:
span.set_data(SPANDATA.DB_DRIVER_NAME, driver)
except Exception:
pass

if conn.engine.url is None:
return

db_name = conn.engine.url.database
if db_name is not None:
span.set_data(SPANDATA.DB_NAME, db_name)

server_address = conn.engine.url.host
if server_address is not None:
span.set_data(SPANDATA.SERVER_ADDRESS, server_address)

server_port = conn.engine.url.port
if server_port is not None:
span.set_data(SPANDATA.SERVER_PORT, server_port)
if isinstance(span, StreamedSpan):
db_name = conn.engine.url.database
if db_name is not None:
span.set_attribute(SPANDATA.DB_NAME, db_name)

server_address = conn.engine.url.host
if server_address is not None:
span.set_attribute(SPANDATA.SERVER_ADDRESS, server_address)

server_port = conn.engine.url.port
if server_port is not None:
span.set_attribute(SPANDATA.SERVER_PORT, server_port)
else:
db_name = conn.engine.url.database
if db_name is not None:
span.set_data(SPANDATA.DB_NAME, db_name)

server_address = conn.engine.url.host
if server_address is not None:
span.set_data(SPANDATA.SERVER_ADDRESS, server_address)

server_port = conn.engine.url.port
if server_port is not None:
span.set_data(SPANDATA.SERVER_PORT, server_port)
31 changes: 22 additions & 9 deletions sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ def record_sql_queries(
span_origin: str = "manual",
) -> "Generator[sentry_sdk.tracing.Span, None, None]":
# TODO: Bring back capturing of params by default
if sentry_sdk.get_client().options["_experiments"].get("record_sql_params", False):
client = sentry_sdk.get_client()
if client.options["_experiments"].get("record_sql_params", False):
if not params_list or params_list == [None]:
params_list = None

Expand All @@ -160,14 +161,26 @@ def record_sql_queries(
with capture_internal_exceptions():
sentry_sdk.add_breadcrumb(message=query, category="query", data=data)

with sentry_sdk.start_span(
op=OP.DB,
name=query,
origin=span_origin,
) as span:
for k, v in data.items():
span.set_data(k, v)
yield span
if has_span_streaming_enabled(client.options):
with sentry_sdk.traces.start_span(
name=query,
attributes={
"sentry.origin": span_origin,
"sentry.op": OP.DB,
},
) as span:
for k, v in data.items():
span.set_attribute(k, v)
yield span
Comment thread
alexander-alderman-webb marked this conversation as resolved.
else:
with sentry_sdk.start_span(
op=OP.DB,
name=query,
Comment thread
alexander-alderman-webb marked this conversation as resolved.
origin=span_origin,
) as span:
for k, v in data.items():
span.set_data(k, v)
yield span


def maybe_create_breadcrumbs_from_span(
Expand Down
28 changes: 20 additions & 8 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,23 +476,35 @@ def maybe_monkeypatched_threading(request):

@pytest.fixture
def render_span_tree():
def inner(event):
assert event["type"] == "transaction"
def inner(spans, root_span=None):
streamed_spans = False
if root_span is None:
streamed_spans = True

by_parent = {}
for span in event["spans"]:
for span in spans:
print(span)
Comment thread
alexander-alderman-webb marked this conversation as resolved.
Outdated
if "parent_span_id" not in span:
root_span = span
continue
Comment thread
alexander-alderman-webb marked this conversation as resolved.

Comment thread
alexander-alderman-webb marked this conversation as resolved.
by_parent.setdefault(span["parent_span_id"], []).append(span)

def render_span(span):
yield "- op={}: description={}".format(
json.dumps(span.get("op")), json.dumps(span.get("description"))
)
if streamed_spans:
yield "- sentry.op={}: name={}".format(
json.dumps(span["attributes"].get("sentry.op")),
json.dumps(span["name"]),
)
else:
yield "- op={}: description={}".format(
json.dumps(span.get("op")), json.dumps(span.get("description"))
)

for subspan in by_parent.get(span["span_id"]) or ():
for line in render_span(subspan):
yield " {}".format(line)

root_span = event["contexts"]["trace"]

return "\n".join(render_span(root_span))

return inner
Expand Down
3 changes: 2 additions & 1 deletion tests/integrations/django/asgi/test_asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,9 @@ async def test_async_middleware_spans(

(transaction,) = events

assert transaction["type"] == "transaction"
assert (
render_span_tree(transaction)
render_span_tree(transaction["spans"], transaction["contexts"]["trace"])
== """\
- op="http.server": description=null
- op="event.django": description="django.db.reset_queries"
Expand Down
31 changes: 24 additions & 7 deletions tests/integrations/django/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,9 @@ def test_django_connect_trace(sentry_init, client, capture_events, render_span_t
data = span.get("data")
assert data.get(SPANDATA.DB_SYSTEM) == "postgresql"

assert '- op="db": description="connect"' in render_span_tree(event)
assert '- op="db": description="connect"' in render_span_tree(
event["spans"], event["contexts"]["trace"]
)


@pytest.mark.forked
Expand Down Expand Up @@ -954,7 +956,9 @@ def test_render_spans(sentry_init, client, capture_events, render_span_tree):
events = capture_events()
client.get(url)
transaction = events[0]
assert expected_line in render_span_tree(transaction)
assert expected_line in render_span_tree(
transaction["spans"], transaction["contexts"]["trace"]
)


@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9")
Expand Down Expand Up @@ -1034,7 +1038,10 @@ def test_middleware_spans(sentry_init, client, capture_events, render_span_tree)
message, transaction = events

assert message["message"] == "hi"
assert render_span_tree(transaction) == EXPECTED_MIDDLEWARE_SPANS
assert (
render_span_tree(transaction["spans"], transaction["contexts"]["trace"])
== EXPECTED_MIDDLEWARE_SPANS
)


def test_middleware_spans_disabled(sentry_init, client, capture_events):
Expand Down Expand Up @@ -1075,7 +1082,10 @@ def test_signals_spans(sentry_init, client, capture_events, render_span_tree):
message, transaction = events

assert message["message"] == "hi"
assert render_span_tree(transaction) == EXPECTED_SIGNALS_SPANS
assert (
render_span_tree(transaction["spans"], transaction["contexts"]["trace"])
== EXPECTED_SIGNALS_SPANS
)

assert transaction["spans"][0]["op"] == "event.django"
assert transaction["spans"][0]["description"] == "django.db.reset_queries"
Expand Down Expand Up @@ -1127,7 +1137,10 @@ def test_signals_spans_filtering(sentry_init, client, capture_events, render_spa

(transaction,) = events

assert render_span_tree(transaction) == EXPECTED_SIGNALS_SPANS_FILTERED
assert (
render_span_tree(transaction["spans"], transaction["contexts"]["trace"])
== EXPECTED_SIGNALS_SPANS_FILTERED
)

assert transaction["spans"][0]["op"] == "event.django"
assert transaction["spans"][0]["description"] == "django.db.reset_queries"
Expand Down Expand Up @@ -1206,7 +1219,9 @@ def test_custom_urlconf_middleware(
event = events.pop(0)
assert event["transaction"] == "/custom/ok"
if middleware_spans:
assert "custom_urlconf_middleware" in render_span_tree(event)
assert "custom_urlconf_middleware" in render_span_tree(
event["spans"], event["contexts"]["trace"]
)

_content, status, _headers = unpack_werkzeug_response(client.get("/custom/exc"))
assert status.lower() == "500 internal server error"
Expand All @@ -1216,7 +1231,9 @@ def test_custom_urlconf_middleware(
assert error_event["exception"]["values"][-1]["mechanism"]["type"] == "django"
assert transaction_event["transaction"] == "/custom/exc"
if middleware_spans:
assert "custom_urlconf_middleware" in render_span_tree(transaction_event)
assert "custom_urlconf_middleware" in render_span_tree(
transaction_event["spans"], transaction_event["contexts"]["trace"]
)

settings.MIDDLEWARE.pop(0)

Expand Down
Loading
Loading