Skip to content

Commit 6ee8140

Browse files
authored
fix(eap-span): n+1 on opportunity and perf score functions (#91808)
In both the `performance_score` and `opportunity_score` functions, we were doing something like ```python for vital in vitals vital_count = get_count_of_vital(vital) # this is a new query with each iteration if (vital_count > 0) # account for this vital in the score ``` This pr updates `get_count_of_vital` to query for all vitals at once, and then cache the result. This way we don't have to make so many queries and just reuse the result of any subsequent vital The cache is stored on the `resolver` instance, therefore it is only applied per request and won't bleed into other requests.
1 parent e984248 commit 6ee8140

File tree

3 files changed

+39
-19
lines changed

3 files changed

+39
-19
lines changed

src/sentry/search/eap/columns.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
from sentry.exceptions import InvalidSearchQuery
2222
from sentry.search.eap import constants
23+
from sentry.search.eap.types import EAPResponse
2324
from sentry.search.events.types import SnubaParams
2425

2526
ResolvedArgument: TypeAlias = AttributeKey | str | int | float
@@ -29,6 +30,7 @@
2930
class ResolverSettings(TypedDict):
3031
extrapolation_mode: ExtrapolationMode.ValueType
3132
snuba_params: SnubaParams
33+
query_result_cache: dict[str, EAPResponse]
3234

3335

3436
@dataclass(frozen=True, kw_only=True)
@@ -259,6 +261,7 @@ def resolve(
259261
search_type: constants.SearchType,
260262
resolved_arguments: ResolvedArguments,
261263
snuba_params: SnubaParams,
264+
query_result_cache: dict[str, EAPResponse],
262265
) -> ResolvedFormula | ResolvedAggregate | ResolvedConditionalAggregate:
263266
raise NotImplementedError()
264267

@@ -278,6 +281,7 @@ def resolve(
278281
search_type: constants.SearchType,
279282
resolved_arguments: ResolvedArguments,
280283
snuba_params: SnubaParams,
284+
query_result_cache: dict[str, EAPResponse],
281285
) -> ResolvedAggregate:
282286
if len(resolved_arguments) > 1:
283287
raise InvalidSearchQuery(
@@ -324,6 +328,7 @@ def resolve(
324328
search_type: constants.SearchType,
325329
resolved_arguments: ResolvedArguments,
326330
snuba_params: SnubaParams,
331+
query_result_cache: dict[str, EAPResponse],
327332
) -> ResolvedConditionalAggregate:
328333
key, filter = self.aggregate_resolver(resolved_arguments)
329334
return ResolvedConditionalAggregate(
@@ -354,6 +359,7 @@ def resolve(
354359
search_type: constants.SearchType,
355360
resolved_arguments: list[AttributeKey | Any],
356361
snuba_params: SnubaParams,
362+
query_result_cache: dict[str, EAPResponse],
357363
) -> ResolvedFormula:
358364
resolver_settings = ResolverSettings(
359365
extrapolation_mode=(
@@ -362,6 +368,7 @@ def resolve(
362368
else ExtrapolationMode.EXTRAPOLATION_MODE_NONE
363369
),
364370
snuba_params=snuba_params,
371+
query_result_cache=query_result_cache,
365372
)
366373

367374
return ResolvedFormula(

src/sentry/search/eap/resolver.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
VirtualColumnDefinition,
5151
)
5252
from sentry.search.eap.spans.attributes import SPANS_INTERNAL_TO_PUBLIC_ALIAS_MAPPINGS
53-
from sentry.search.eap.types import SearchResolverConfig
53+
from sentry.search.eap.types import EAPResponse, SearchResolverConfig
5454
from sentry.search.eap.utils import validate_sampling
5555
from sentry.search.events import constants as qb_constants
5656
from sentry.search.events import fields
@@ -69,6 +69,7 @@ class SearchResolver:
6969
config: SearchResolverConfig
7070
definitions: ColumnDefinitions
7171
granularity_secs: int | None = None
72+
_query_result_cache: dict[str, EAPResponse] = field(default_factory=dict)
7273
_resolved_attribute_cache: dict[
7374
str, tuple[ResolvedAttribute, VirtualColumnDefinition | None]
7475
] = field(default_factory=dict)
@@ -947,6 +948,7 @@ def resolve_function(self, column: str, match: Match | None = None) -> tuple[
947948
search_type=search_type,
948949
resolved_arguments=resolved_arguments,
949950
snuba_params=self.params,
951+
query_result_cache=self._query_result_cache,
950952
)
951953

952954
resolved_context = None

src/sentry/search/eap/spans/formulas.py

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -245,26 +245,37 @@ def failure_rate(_: ResolvedArguments, settings: ResolverSettings) -> Column.Bin
245245

246246

247247
def get_count_of_vital(vital: str, settings: ResolverSettings) -> float:
248+
cache_key = "totalvitalcounts"
249+
response = None
250+
vital_column = f"count_scores(measurements.score.{vital})"
251+
252+
if cache_key in settings["query_result_cache"]:
253+
response = settings["query_result_cache"][cache_key]
254+
255+
else:
256+
snuba_params = settings["snuba_params"]
257+
query_string = snuba_params.query_string
258+
259+
vital_columns = [f"count_scores({v})" for v in WEB_VITALS_MEASUREMENTS]
260+
261+
response = spans_rpc.run_table_query(
262+
snuba_params,
263+
query_string=query_string if query_string is not None else "",
264+
referrer=cache_key,
265+
selected_columns=vital_columns,
266+
orderby=None,
267+
offset=0,
268+
limit=1,
269+
sampling_mode=snuba_params.sampling_mode,
270+
config=SearchResolverConfig(
271+
auto_fields=True,
272+
),
273+
)
248274

249-
snuba_params = settings["snuba_params"]
250-
query_string = snuba_params.query_string
251-
252-
rpc_res = spans_rpc.run_table_query(
253-
snuba_params,
254-
query_string=query_string if query_string is not None else "",
255-
referrer=f"totalvitalcount_{vital}",
256-
selected_columns=[f"count_scores(measurements.score.{vital}) as count"],
257-
orderby=None,
258-
offset=0,
259-
limit=1,
260-
sampling_mode=snuba_params.sampling_mode,
261-
config=SearchResolverConfig(
262-
auto_fields=True,
263-
),
264-
)
275+
settings["query_result_cache"][cache_key] = response
265276

266-
if len(rpc_res["data"]) > 0 and rpc_res["data"][0]["count"] is not None:
267-
return rpc_res["data"][0]["count"]
277+
if len(response["data"]) > 0 and response["data"][0][vital_column] is not None:
278+
return response["data"][0][vital_column]
268279

269280
return 0
270281

0 commit comments

Comments
 (0)