Skip to content

Commit

Permalink
feat(observability): Use spans more accurately in snuba (#4480)
Browse files Browse the repository at this point in the history
The sentry performance product expects that db spans actually query the DB. Our code is not like that atm. Put a span close to db execution and make the span description the actual sql query. This will allow us to dogfood new performance features
  • Loading branch information
volokluev authored Jul 10, 2023
1 parent feb0c70 commit ed5342b
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 19 deletions.
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ sentry-arroyo==2.14.0
sentry-kafka-schemas==0.1.17
sentry-redis-tools==0.1.7
sentry-relay==0.8.27
sentry-sdk==1.26.0
sentry-sdk==1.28.0
simplejson==3.17.6
structlog==22.3.0
structlog-sentry==2.0.0
Expand Down
29 changes: 18 additions & 11 deletions snuba/clickhouse/native.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from contextlib import contextmanager
from dataclasses import dataclass, field
from datetime import date, datetime
from functools import partial
from io import StringIO
from typing import (
Any,
Expand All @@ -24,6 +23,7 @@
)
from uuid import UUID

import sentry_sdk
from clickhouse_driver import Client, errors
from dateutil.tz import tz

Expand Down Expand Up @@ -172,16 +172,23 @@ def execute(
else {"send_logs_level": "trace"}
)

query_execute = partial(
conn.execute,
query,
params=params,
with_column_types=with_column_types,
query_id=query_id,
settings=settings,
types_check=types_check,
columnar=columnar,
)
def query_execute() -> Any:
with sentry_sdk.start_span(
description=query, op="db.clickhouse"
) as span:
span.set_data(
sentry_sdk.consts.SPANDATA.DB_SYSTEM, "clickhouse"
)
return conn.execute( # type: ignore
query,
params=params,
with_column_types=with_column_types,
query_id=query_id,
settings=settings,
types_check=types_check,
columnar=columnar,
)

result_data: Sequence[Any]
trace_output = ""
if capture_trace:
Expand Down
8 changes: 4 additions & 4 deletions snuba/web/db_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def update_query_metadata_and_stats(
return stats


@with_span(op="db")
@with_span(op="function")
def execute_query(
# TODO: Passing the whole clickhouse query here is needed as long
# as the execute method depends on it. Otherwise we can make this
Expand Down Expand Up @@ -285,7 +285,7 @@ def _apply_thread_quota_to_clickhouse_query_settings(
clickhouse_query_settings["max_threads"] = maxt


@with_span(op="db")
@with_span(op="function")
def execute_query_with_rate_limits(
clickhouse_query: Union[Query, CompositeQuery[Table]],
query_settings: QuerySettings,
Expand Down Expand Up @@ -367,7 +367,7 @@ def _get_cache_partition(reader: Reader) -> Cache[Result]:
]


@with_span(op="db")
@with_span(op="function")
def execute_query_with_query_id(
clickhouse_query: Union[Query, CompositeQuery[Table]],
query_settings: QuerySettings,
Expand Down Expand Up @@ -419,7 +419,7 @@ def execute_query_with_query_id(
)


@with_span(op="db")
@with_span(op="function")
def execute_query_with_readthrough_caching(
clickhouse_query: Union[Query, CompositeQuery[Table]],
query_settings: QuerySettings,
Expand Down
8 changes: 5 additions & 3 deletions snuba/web/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ def _dry_run_query_runner(
# you will only see the sql reported that the first of the split queries ran. Since this returns
# no results, you won't see any others

with sentry_sdk.start_span(description="dryrun_create_query", op="db") as span:
with sentry_sdk.start_span(
description="dryrun_create_query", op="function"
) as span:
formatted_query = format_query(clickhouse_query)
span.set_data("query", formatted_query.structured())

Expand Down Expand Up @@ -313,7 +315,7 @@ def _format_storage_query_and_run(
visitor = TablesCollector()
visitor.visit(from_clause)
table_names = ",".join(sorted(visitor.get_tables()))
with sentry_sdk.start_span(description="create_query", op="db") as span:
with sentry_sdk.start_span(description="create_query", op="function") as span:
_apply_turbo_sampling_if_needed(clickhouse_query, query_settings)

formatted_query = format_query(clickhouse_query)
Expand Down Expand Up @@ -352,7 +354,7 @@ def _format_storage_query_and_run(
experiments=clickhouse_query.get_experiments(),
),
) from cause
with sentry_sdk.start_span(description=formatted_sql, op="db") as span:
with sentry_sdk.start_span(description=formatted_sql, op="function") as span:
span.set_tag("table", table_names)

def execute() -> QueryResult:
Expand Down

0 comments on commit ed5342b

Please sign in to comment.