Skip to content

Commit

Permalink
feat(starfish): Add total.sum_transaction_duration to query builder (#…
Browse files Browse the repository at this point in the history
…47335)

Adding this field to allow us to prototype queries like this one that
will allow us to find "percentage of time spent by each endpoint in your
web service"
  • Loading branch information
shruthilayaj authored Apr 13, 2023
1 parent d5e23e0 commit a5f44d6
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 3 deletions.
8 changes: 6 additions & 2 deletions src/sentry/discover/arithmetic.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from parsimonious.grammar import Grammar, NodeVisitor

from sentry.exceptions import InvalidSearchQuery
from sentry.search.events.constants import TOTAL_COUNT_ALIAS
from sentry.search.events.constants import TOTAL_COUNT_ALIAS, TOTAL_TRANSACTION_DURATION_ALIAS

# prefix on fields so we know they're equations
EQUATION_PREFIX = "equation|"
Expand Down Expand Up @@ -161,6 +161,7 @@ class ArithmeticVisitor(NodeVisitor):
"measurements.ttfb",
"measurements.ttfb.requesttime",
TOTAL_COUNT_ALIAS,
TOTAL_TRANSACTION_DURATION_ALIAS,
}
function_allowlist = {
"count",
Expand Down Expand Up @@ -297,7 +298,10 @@ def parse_arithmetic(
visitor = ArithmeticVisitor(max_operators, custom_measurements)
result = visitor.visit(tree)
# total count is the exception to the no mixing rule
if visitor.fields == {TOTAL_COUNT_ALIAS} and len(visitor.functions) > 0:
if (
visitor.fields.intersection({TOTAL_COUNT_ALIAS, TOTAL_TRANSACTION_DURATION_ALIAS})
and len(visitor.functions) > 0
):
return result, list(visitor.fields), list(visitor.functions)
if len(visitor.fields) > 0 and len(visitor.functions) > 0:
raise ArithmeticValidationError("Cannot mix functions and fields in arithmetic")
Expand Down
3 changes: 2 additions & 1 deletion src/sentry/search/events/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
TIMESTAMP_TO_DAY_ALIAS = "timestamp.to_day"
# Named this way in case we want to eventually do stuff like total.p50
TOTAL_COUNT_ALIAS = "total.count"
TOTAL_TRANSACTION_DURATION_ALIAS = "total.transaction_duration"
TRANSACTION_STATUS_ALIAS = "transaction.status"
MEASUREMENTS_FRAMES_SLOW_RATE = "measurements.frames_slow_rate"
MEASUREMENTS_FRAMES_FROZEN_RATE = "measurements.frames_frozen_rate"
Expand Down Expand Up @@ -130,7 +131,7 @@ class ThresholdDict(TypedDict):
NO_CONVERSION_FIELDS = {"start", "end"}
# Skip total_count_alias since it queries the total count and therefore doesn't make sense in a filter
# In these cases we should instead treat it as a tag instead
SKIP_FILTER_RESOLUTION = {TOTAL_COUNT_ALIAS}
SKIP_FILTER_RESOLUTION = {TOTAL_COUNT_ALIAS, TOTAL_TRANSACTION_DURATION_ALIAS}
EQUALITY_OPERATORS = frozenset(["=", "IN"])
INEQUALITY_OPERATORS = frozenset(["!=", "NOT IN"])
ARRAY_FIELDS = {
Expand Down
27 changes: 27 additions & 0 deletions src/sentry/search/events/datasets/discover.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
TIMESTAMP_TO_DAY_ALIAS,
TIMESTAMP_TO_HOUR_ALIAS,
TOTAL_COUNT_ALIAS,
TOTAL_TRANSACTION_DURATION_ALIAS,
TRACE_PARENT_SPAN_ALIAS,
TRACE_PARENT_SPAN_CONTEXT,
TRANSACTION_STATUS_ALIAS,
Expand Down Expand Up @@ -106,6 +107,7 @@ class DiscoverDatasetConfig(DatasetConfig):
def __init__(self, builder: builder.QueryBuilder):
self.builder = builder
self.total_count: Optional[int] = None
self.total_sum_transaction_duration: Optional[float] = None

@property
def search_filter_converter(
Expand Down Expand Up @@ -152,6 +154,7 @@ def field_alias_converter(self) -> Mapping[str, Callable[[str], SelectType]]:
MEASUREMENTS_STALL_PERCENTAGE: self._resolve_measurements_stall_percentage,
HTTP_STATUS_CODE_ALIAS: self._resolve_http_status_code,
TOTAL_COUNT_ALIAS: self._resolve_total_count,
TOTAL_TRANSACTION_DURATION_ALIAS: self._resolve_total_sum_transaction_duration,
DEVICE_CLASS_ALIAS: self._resolve_device_class,
}

Expand Down Expand Up @@ -1275,6 +1278,30 @@ def _resolve_total_count(self, alias: str) -> SelectType:
self.total_count = results["data"][0]["count"]
return Function("toUInt64", [self.total_count], alias)

def _resolve_total_sum_transaction_duration(self, alias: str) -> SelectType:
"""This must be cached since it runs another query"""
self.builder.requires_other_aggregates = True
if self.total_sum_transaction_duration is not None:
return Function("toFloat64", [self.total_sum_transaction_duration], alias)
# TODO[Shruthi]: Figure out parametrization of the args to sum()
total_query = builder.QueryBuilder(
dataset=self.builder.dataset,
params={},
snuba_params=self.builder.params,
selected_columns=["sum(transaction.duration)"],
)
total_query.columns += self.builder.resolve_groupby()
total_query.where = self.builder.where
total_results = total_query.run_query(
Referrer.API_DISCOVER_TOTAL_SUM_TRANSACTION_DURATION_FIELD.value
)
results = total_query.process_results(total_results)
if len(results["data"]) != 1:
self.total_sum_transaction_duration = 0
return Function("toFloat64", [0], alias)
self.total_sum_transaction_duration = results["data"][0]["sum_transaction_duration"]
return Function("toFloat64", [self.total_sum_transaction_duration], alias)

def _resolve_device_class(self, _: str) -> SelectType:
return Function(
"multiIf",
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/snuba/referrer.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class ReferrerBase(Enum):
API_DASHBOARDS_WIDGET_LINE_CHART = "api.dashboards.widget.line-chart"
API_DASHBOARDS_WORLDMAPWIDGET = "api.dashboards.worldmapwidget"
API_DISCOVER_TOTAL_COUNT_FIELD = "api.discover.total-events-field"
API_DISCOVER_TOTAL_SUM_TRANSACTION_DURATION_FIELD = (
"api.discover.total-sum-transaction-duration-field"
)
API_DISCOVER_DAILY_CHART = "api.discover.daily-chart"
API_DISCOVER_DAILYTOP5_CHART_FIND_TOPN = "api.discover.dailytop5-chart.find-topn"
API_DISCOVER_DAILYTOP5_CHART = "api.discover.dailytop5-chart"
Expand Down
36 changes: 36 additions & 0 deletions tests/snuba/api/endpoints/test_organization_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -5981,6 +5981,42 @@ def test_total_count_filter(self):
assert len(data) == 1
assert data[0]["total.count"] == 1

def test_total_sum_transaction_duration_equation(self):
for i in range(3):
data = self.load_data(
timestamp=self.eleven_mins_ago,
duration=timedelta(seconds=5),
)
data["transaction"] = "/endpoint/1"
self.store_event(data, project_id=self.project.id)

data = self.load_data(
timestamp=self.ten_mins_ago,
duration=timedelta(seconds=5),
)
data["transaction"] = "/endpoint/2"
self.store_event(data, project_id=self.project.id)

features = {"organizations:discover-basic": True, "organizations:global-views": True}

query = {
"field": [
"transaction",
"sum(transaction.duration)",
"total.transaction_duration",
"equation|sum(transaction.duration)/total.transaction_duration",
],
"query": "",
"orderby": "-equation|sum(transaction.duration)/total.transaction_duration",
"statsPeriod": "24h",
}
response = self.do_request(query, features=features)
assert response.status_code == 200, response.content
data = response.data["data"]
assert len(data) == 2
assert data[0]["equation|sum(transaction.duration)/total.transaction_duration"] == 0.75
assert data[1]["equation|sum(transaction.duration)/total.transaction_duration"] == 0.25

def test_device_class(self):
project1 = self.create_project()
for i in range(3):
Expand Down

0 comments on commit a5f44d6

Please sign in to comment.