From 3089bb32155c3d9a7d6d064bbbc9f508ffe4ad6f Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Fri, 21 Feb 2025 13:06:24 -0500 Subject: [PATCH 1/9] implement response status code 5 --- src/sentry/search/eap/columns.py | 20 +++++ src/sentry/search/eap/resolver.py | 20 +++-- src/sentry/search/eap/span_columns.py | 81 ++++++++++++++++++- src/sentry/snuba/rpc_dataset_common.py | 6 +- .../test_organization_events_span_indexed.py | 32 ++++++++ 5 files changed, 148 insertions(+), 11 deletions(-) diff --git a/src/sentry/search/eap/columns.py b/src/sentry/search/eap/columns.py index af86b8b26b5771..2041a5ebad6dc2 100644 --- a/src/sentry/search/eap/columns.py +++ b/src/sentry/search/eap/columns.py @@ -17,6 +17,8 @@ from sentry.search.eap import constants from sentry.search.events.types import SnubaParams +# from sentry.snuba.rpc_dataset_common import Column + @dataclass(frozen=True, kw_only=True) class ResolvedAttribute: @@ -117,6 +119,24 @@ class VirtualColumnDefinition: default_value: str | None = None +@dataclass(frozen=True, kw_only=True) +class CustomFunction(ResolvedAttribute): + formula: Any + + @property + def proto_definition(self) -> Any: + """The definition of this function as needed by the RPC""" + return self.formula + + @property + def proto_type(self) -> AttributeKey.Type.ValueType: + """The rpc always returns functions as floats, especially count() even though it should be an integer + + see: https://www.notion.so/sentry/Should-count-return-an-int-in-the-v1-RPC-API-1348b10e4b5d80498bfdead194cc304e + """ + return constants.DOUBLE + + @dataclass(frozen=True, kw_only=True) class ResolvedFunction(ResolvedAttribute): # The internal rpc alias for this column diff --git a/src/sentry/search/eap/resolver.py b/src/sentry/search/eap/resolver.py index ecfab2dcc747a0..a732b6a4bb794d 100644 --- a/src/sentry/search/eap/resolver.py +++ b/src/sentry/search/eap/resolver.py @@ -36,10 +36,12 @@ from sentry.search.eap import constants from sentry.search.eap.columns import ( ColumnDefinitions, + CustomFunction, ResolvedColumn, ResolvedFunction, VirtualColumnDefinition, ) +from sentry.search.eap.span_columns import CUSTOM_FUNCTIONS from sentry.search.eap.types import SearchResolverConfig from sentry.search.events import constants as qb_constants from sentry.search.events import fields @@ -547,9 +549,10 @@ def resolve_contexts( return final_contexts @sentry_sdk.trace - def resolve_columns( - self, selected_columns: list[str] - ) -> tuple[list[ResolvedColumn | ResolvedFunction], list[VirtualColumnDefinition | None]]: + def resolve_columns(self, selected_columns: list[str]) -> tuple[ + list[ResolvedColumn | ResolvedFunction | CustomFunction], + list[VirtualColumnDefinition | None], + ]: """Given a list of columns resolve them and get their context if applicable This function will also dedupe the virtual column contexts if necessary @@ -584,7 +587,7 @@ def resolve_columns( def resolve_column( self, column: str, match: Match | None = None - ) -> tuple[ResolvedColumn | ResolvedFunction, VirtualColumnDefinition | None]: + ) -> tuple[ResolvedColumn | ResolvedFunction | CustomFunction, VirtualColumnDefinition | None]: """Column is either an attribute or an aggregate, this function will determine which it is and call the relevant resolve function""" match = fields.is_function(column) @@ -666,7 +669,7 @@ def resolve_attribute( @sentry_sdk.trace def resolve_aggregates( self, columns: list[str] - ) -> tuple[list[ResolvedFunction], list[VirtualColumnDefinition | None]]: + ) -> tuple[list[ResolvedFunction | CustomFunction], list[VirtualColumnDefinition | None]]: """Helper function to resolve a list of aggregates instead of 1 attribute at a time""" resolved_aggregates, resolved_contexts = [], [] for column in columns: @@ -677,10 +680,10 @@ def resolve_aggregates( def resolve_aggregate( self, column: str, match: Match | None = None - ) -> tuple[ResolvedFunction, VirtualColumnDefinition | None]: + ) -> tuple[ResolvedFunction | CustomFunction, VirtualColumnDefinition | None]: if column in self._resolved_function_cache: return self._resolved_function_cache[column] - # Check if this is a valid function, parse the function name and args out + # Check if the column looks like a function (matches a pattern), parse the function name and args out if match is None: match = fields.is_function(column) if match is None: @@ -691,6 +694,9 @@ def resolve_aggregate( # Alias defaults to the name of the function alias = match.group("alias") or column + if function in CUSTOM_FUNCTIONS: + return (CUSTOM_FUNCTIONS.get(function), None) + # Get the function definition if function not in self.definitions.functions: raise InvalidSearchQuery(f"Unknown function {function}") diff --git a/src/sentry/search/eap/span_columns.py b/src/sentry/search/eap/span_columns.py index adc220da0e576c..7c659787a6e3eb 100644 --- a/src/sentry/search/eap/span_columns.py +++ b/src/sentry/search/eap/span_columns.py @@ -1,12 +1,25 @@ -from typing import Literal +from typing import Any, Literal +from sentry_protos.snuba.v1.attribute_conditional_aggregation_pb2 import ( + AttributeConditionalAggregation, +) +from sentry_protos.snuba.v1.endpoint_trace_item_table_pb2 import Column from sentry_protos.snuba.v1.request_common_pb2 import TraceItemType -from sentry_protos.snuba.v1.trace_item_attribute_pb2 import Function, VirtualColumnContext +from sentry_protos.snuba.v1.trace_item_attribute_pb2 import ( + AttributeKey, + AttributeValue, + ExtrapolationMode, + Function, + StrArray, + VirtualColumnContext, +) +from sentry_protos.snuba.v1.trace_item_filter_pb2 import ComparisonFilter, TraceItemFilter from sentry.search.eap import constants from sentry.search.eap.columns import ( ArgumentDefinition, ColumnDefinitions, + CustomFunction, FunctionDefinition, ResolvedColumn, VirtualColumnDefinition, @@ -627,3 +640,67 @@ def count_processor(count_value: int | None) -> int: contexts=SPAN_VIRTUAL_CONTEXTS, trace_item_type=TraceItemType.TRACE_ITEM_TYPE_SPAN, ) + +CUSTOM_FUNCTIONS_FORMULAS: dict[Any, Column.BinaryFormula] = { + "http_response_rate": Column.BinaryFormula( + left=Column( + conditional_aggregation=AttributeConditionalAggregation( + aggregate=Function.FUNCTION_COUNT, + key=AttributeKey( + name="sentry.status_code", + type=AttributeKey.TYPE_STRING, + ), + filter=TraceItemFilter( + comparison_filter=ComparisonFilter( + key=AttributeKey( + name="sentry.status_code", + type=AttributeKey.TYPE_STRING, + ), + op=ComparisonFilter.OP_IN, + value=AttributeValue( + val_str_array=StrArray( + values=[ + "500", + "501", + "502", + "503", + "504", + "505", + "506", + "507", + "508", + "509", + "510", + "511", + ], + ), + ), + ) + ), + label="error_request_count", + extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, + ), + ), + op=Column.BinaryFormula.OP_DIVIDE, + right=Column( + conditional_aggregation=AttributeConditionalAggregation( + aggregate=Function.FUNCTION_COUNT, + key=AttributeKey( + name="sentry.status_code", + type=AttributeKey.TYPE_STRING, + ), + label="total_request_count", + extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, + ), + ), + ), +} + + +CUSTOM_FUNCTIONS: dict[Any, CustomFunction] = { + "http_response_rate": CustomFunction( + public_alias="http_response_rate_5", + formula=CUSTOM_FUNCTIONS_FORMULAS["http_response_rate"], + search_type="percentage", + ) +} diff --git a/src/sentry/snuba/rpc_dataset_common.py b/src/sentry/snuba/rpc_dataset_common.py index c82c730d3ac888..c7030164a3f601 100644 --- a/src/sentry/snuba/rpc_dataset_common.py +++ b/src/sentry/snuba/rpc_dataset_common.py @@ -5,7 +5,7 @@ from sentry_protos.snuba.v1.request_common_pb2 import PageToken from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeAggregation, AttributeKey -from sentry.search.eap.columns import ResolvedColumn, ResolvedFunction +from sentry.search.eap.columns import CustomFunction, ResolvedColumn, ResolvedFunction from sentry.search.eap.resolver import SearchResolver from sentry.search.eap.types import CONFIDENCES, ConfidenceData, EAPResponse from sentry.search.events.fields import get_function_alias @@ -16,7 +16,9 @@ logger = logging.getLogger("sentry.snuba.spans_rpc") -def categorize_column(column: ResolvedColumn | ResolvedFunction) -> Column: +def categorize_column(column: ResolvedColumn | ResolvedFunction | CustomFunction) -> Column: + if isinstance(column, CustomFunction): + return Column(formula=column.proto_definition, label=column.public_alias) if isinstance(column, ResolvedFunction): return Column(aggregation=column.proto_definition, label=column.public_alias) else: diff --git a/tests/snuba/api/endpoints/test_organization_events_span_indexed.py b/tests/snuba/api/endpoints/test_organization_events_span_indexed.py index 9e861c4b3cd12c..c005bce28a1c25 100644 --- a/tests/snuba/api/endpoints/test_organization_events_span_indexed.py +++ b/tests/snuba/api/endpoints/test_organization_events_span_indexed.py @@ -2537,6 +2537,38 @@ def test_device_class_filter_unknown(self): assert data[0]["device.class"] == "Unknown" assert meta["dataset"] == self.dataset + def test_http_response_rate(self): + self.store_spans( + [ + self.create_span( + {"sentry_tags": {"status_code": "500"}}, start_ts=self.ten_mins_ago + ), + ], + is_eap=self.is_eap, + ) + self.store_spans( + [ + self.create_span( + {"sentry_tags": {"status_code": "200"}}, start_ts=self.ten_mins_ago + ), + ], + is_eap=self.is_eap, + ) + response = self.do_request( + { + "field": ["http_response_rate(5)"], + "project": self.project.id, + "dataset": self.dataset, + } + ) + + assert response.status_code == 200, response.content + data = response.data["data"] + meta = response.data["meta"] + assert len(data) == 1 + assert data[0]["http_response_rate_5"] == 0.5 + assert meta["dataset"] == self.dataset + @pytest.mark.skip(reason="replay id alias not migrated over") def test_replay_id(self): super().test_replay_id() From 2b8f6db0bf107797c522fe72d2024be553d095c2 Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Mon, 24 Feb 2025 14:17:10 -0500 Subject: [PATCH 2/9] working function with args --- src/sentry/search/eap/columns.py | 71 ++++++++---- src/sentry/search/eap/resolver.py | 55 +++++---- src/sentry/search/eap/span_columns.py | 93 +++------------ src/sentry/search/eap/span_formulas.py | 107 ++++++++++++++++++ src/sentry/snuba/rpc_dataset_common.py | 6 +- .../test_organization_events_span_indexed.py | 26 ++++- 6 files changed, 230 insertions(+), 128 deletions(-) create mode 100644 src/sentry/search/eap/span_formulas.py diff --git a/src/sentry/search/eap/columns.py b/src/sentry/search/eap/columns.py index 2041a5ebad6dc2..ddfeb6febb4614 100644 --- a/src/sentry/search/eap/columns.py +++ b/src/sentry/search/eap/columns.py @@ -83,27 +83,6 @@ class ArgumentDefinition: ignored: bool = False -@dataclass -class FunctionDefinition: - internal_function: Function.ValueType - # The list of arguments for this function - arguments: list[ArgumentDefinition] - # The search_type the argument should be the default type for this column - default_search_type: constants.SearchType - # Try to infer the search type from the function arguments - infer_search_type_from_arguments: bool = True - # The internal rpc type for this function, optional as it can mostly be inferred from search_type - internal_type: AttributeKey.Type.ValueType | None = None - # Processor is the function run in the post process step to transform a row into the final result - processor: Callable[[Any], Any] | None = None - # Whether to request extrapolation or not, should be true for all functions except for _sample functions for debugging - extrapolation: bool = True - - @property - def required_arguments(self) -> list[ArgumentDefinition]: - return [arg for arg in self.arguments if arg.default_arg is None and not arg.ignored] - - @dataclass class VirtualColumnDefinition: constructor: Callable[[SnubaParams], VirtualColumnContext] @@ -120,7 +99,7 @@ class VirtualColumnDefinition: @dataclass(frozen=True, kw_only=True) -class CustomFunction(ResolvedAttribute): +class ResolvedFormula(ResolvedAttribute): formula: Any @property @@ -167,6 +146,54 @@ def proto_type(self) -> AttributeKey.Type.ValueType: return constants.DOUBLE +@dataclass +class FunctionDefinition: + internal_function: Function.ValueType + # The list of arguments for this function + arguments: list[ArgumentDefinition] + # The search_type the argument should be the default type for this column + default_search_type: constants.SearchType + # Try to infer the search type from the function arguments + infer_search_type_from_arguments: bool = True + # The internal rpc type for this function, optional as it can mostly be inferred from search_type + internal_type: AttributeKey.Type.ValueType | None = None + # Processor is the function run in the post process step to transform a row into the final result + processor: Callable[[Any], Any] | None = None + # Whether to request extrapolation or not, should be true for all functions except for _sample functions for debugging + extrapolation: bool = True + # For more complex functions, a formula can be specified + formula_resolver: Callable[[Any], Any] | None = None + + @property + def required_arguments(self) -> list[ArgumentDefinition]: + return [arg for arg in self.arguments if arg.default_arg is None and not arg.ignored] + + def resolve( + self, alias: str, search_type: constants.SearchType, resolved_argument: AttributeKey | None + ) -> ResolvedFormula | ResolvedFunction: + if self.formula_resolver: + return ResolvedFormula( + public_alias=alias, + search_type=search_type, + formula=self.formula_resolver( + resolved_argument.name if resolved_argument else None + ), + argument=resolved_argument, + internal_type=self.internal_type, + processor=self.processor, + ) + else: + return ResolvedFunction( + public_alias=alias, + internal_name=self.internal_function, + search_type=search_type, + internal_type=self.internal_type, + processor=self.processor, + extrapolation=self.extrapolation, + argument=resolved_argument, + ) + + def simple_sentry_field(field) -> ResolvedColumn: """For a good number of fields, the public alias matches the internal alias without the `sentry.` suffix. This helper functions makes defining them easier""" diff --git a/src/sentry/search/eap/resolver.py b/src/sentry/search/eap/resolver.py index a732b6a4bb794d..8dfcc44258edcb 100644 --- a/src/sentry/search/eap/resolver.py +++ b/src/sentry/search/eap/resolver.py @@ -36,12 +36,11 @@ from sentry.search.eap import constants from sentry.search.eap.columns import ( ColumnDefinitions, - CustomFunction, ResolvedColumn, + ResolvedFormula, ResolvedFunction, VirtualColumnDefinition, ) -from sentry.search.eap.span_columns import CUSTOM_FUNCTIONS from sentry.search.eap.types import SearchResolverConfig from sentry.search.events import constants as qb_constants from sentry.search.events import fields @@ -62,9 +61,9 @@ class SearchResolver: _resolved_attribute_cache: dict[str, tuple[ResolvedColumn, VirtualColumnDefinition | None]] = ( field(default_factory=dict) ) - _resolved_function_cache: dict[str, tuple[ResolvedFunction, VirtualColumnDefinition | None]] = ( - field(default_factory=dict) - ) + _resolved_function_cache: dict[ + str, tuple[ResolvedFunction | ResolvedFormula, VirtualColumnDefinition | None] + ] = field(default_factory=dict) @sentry_sdk.trace def resolve_meta(self, referrer: str) -> RequestMeta: @@ -550,7 +549,7 @@ def resolve_contexts( @sentry_sdk.trace def resolve_columns(self, selected_columns: list[str]) -> tuple[ - list[ResolvedColumn | ResolvedFunction | CustomFunction], + list[ResolvedColumn | ResolvedFunction | ResolvedFormula], list[VirtualColumnDefinition | None], ]: """Given a list of columns resolve them and get their context if applicable @@ -587,7 +586,7 @@ def resolve_columns(self, selected_columns: list[str]) -> tuple[ def resolve_column( self, column: str, match: Match | None = None - ) -> tuple[ResolvedColumn | ResolvedFunction | CustomFunction, VirtualColumnDefinition | None]: + ) -> tuple[ResolvedColumn | ResolvedFunction | ResolvedFormula, VirtualColumnDefinition | None]: """Column is either an attribute or an aggregate, this function will determine which it is and call the relevant resolve function""" match = fields.is_function(column) @@ -669,7 +668,7 @@ def resolve_attribute( @sentry_sdk.trace def resolve_aggregates( self, columns: list[str] - ) -> tuple[list[ResolvedFunction | CustomFunction], list[VirtualColumnDefinition | None]]: + ) -> tuple[list[ResolvedFunction | ResolvedFormula], list[VirtualColumnDefinition | None]]: """Helper function to resolve a list of aggregates instead of 1 attribute at a time""" resolved_aggregates, resolved_contexts = [], [] for column in columns: @@ -680,7 +679,7 @@ def resolve_aggregates( def resolve_aggregate( self, column: str, match: Match | None = None - ) -> tuple[ResolvedFunction | CustomFunction, VirtualColumnDefinition | None]: + ) -> tuple[ResolvedFunction | ResolvedFormula, VirtualColumnDefinition | None]: if column in self._resolved_function_cache: return self._resolved_function_cache[column] # Check if the column looks like a function (matches a pattern), parse the function name and args out @@ -694,9 +693,6 @@ def resolve_aggregate( # Alias defaults to the name of the function alias = match.group("alias") or column - if function in CUSTOM_FUNCTIONS: - return (CUSTOM_FUNCTIONS.get(function), None) - # Get the function definition if function not in self.definitions.functions: raise InvalidSearchQuery(f"Unknown function {function}") @@ -714,7 +710,29 @@ def resolve_aggregate( for index, argument in enumerate(function_definition.arguments): if argument.ignored: continue - if index < len(attribute_args): + + # if a function only accepts one type of argument, we can resolve it directly + if ( + argument.argument_types is not None + and len(argument.argument_types) == 1 + and index < len(attribute_args) + ): + arg = attribute_args[index] + arg_type = next(iter(argument.argument_types)) + if ( + arg_type == constants.TYPE_TO_STRING_MAP.get(AttributeKey.TYPE_INT) + and arg.isdigit() + ): + parsed_argument = ResolvedColumn( + public_alias=arg, + internal_name=arg, + search_type="integer", + ) + else: + parsed_argument = ResolvedColumn( + public_alias=arg, internal_name=arg, search_type="string" + ) + elif index < len(attribute_args): parsed_argument, _ = self.resolve_attribute(attribute_args[index]) elif argument.default_arg: parsed_argument, _ = self.resolve_attribute(argument.default_arg) @@ -749,15 +767,8 @@ def resolve_aggregate( resolved_argument = None search_type = function_definition.default_search_type - resolved_function = ResolvedFunction( - public_alias=alias, - internal_name=function_definition.internal_function, - search_type=search_type, - internal_type=function_definition.internal_type, - processor=function_definition.processor, - extrapolation=function_definition.extrapolation, - argument=resolved_argument, - ) + resolved_function = function_definition.resolve(alias, search_type, resolved_argument) + resolved_context = None self._resolved_function_cache[column] = (resolved_function, resolved_context) return self._resolved_function_cache[column] diff --git a/src/sentry/search/eap/span_columns.py b/src/sentry/search/eap/span_columns.py index 7c659787a6e3eb..e553aa7085f576 100644 --- a/src/sentry/search/eap/span_columns.py +++ b/src/sentry/search/eap/span_columns.py @@ -1,25 +1,12 @@ -from typing import Any, Literal +from typing import Literal -from sentry_protos.snuba.v1.attribute_conditional_aggregation_pb2 import ( - AttributeConditionalAggregation, -) -from sentry_protos.snuba.v1.endpoint_trace_item_table_pb2 import Column from sentry_protos.snuba.v1.request_common_pb2 import TraceItemType -from sentry_protos.snuba.v1.trace_item_attribute_pb2 import ( - AttributeKey, - AttributeValue, - ExtrapolationMode, - Function, - StrArray, - VirtualColumnContext, -) -from sentry_protos.snuba.v1.trace_item_filter_pb2 import ComparisonFilter, TraceItemFilter +from sentry_protos.snuba.v1.trace_item_attribute_pb2 import Function, VirtualColumnContext from sentry.search.eap import constants from sentry.search.eap.columns import ( ArgumentDefinition, ColumnDefinitions, - CustomFunction, FunctionDefinition, ResolvedColumn, VirtualColumnDefinition, @@ -30,6 +17,7 @@ simple_sentry_field, ) from sentry.search.eap.common_columns import COMMON_COLUMNS +from sentry.search.eap.span_formulas import CUSTOM_FUNCTION_RESOLVER from sentry.search.events.constants import ( PRECISE_FINISH_TS, PRECISE_START_TS, @@ -632,6 +620,17 @@ def count_processor(count_value: int | None) -> int: ) ], ), + "http_response_rate": FunctionDefinition( + internal_function=Function.FUNCTION_UNSPECIFIED, + default_search_type="percentage", + arguments=[ + ArgumentDefinition( + argument_types={"integer"}, + default_arg=None, # TODO - this should only accept 2,3,4,5 + ) + ], + formula_resolver=CUSTOM_FUNCTION_RESOLVER["http_response_rate"], + ), } SPAN_DEFINITIONS = ColumnDefinitions( @@ -640,67 +639,3 @@ def count_processor(count_value: int | None) -> int: contexts=SPAN_VIRTUAL_CONTEXTS, trace_item_type=TraceItemType.TRACE_ITEM_TYPE_SPAN, ) - -CUSTOM_FUNCTIONS_FORMULAS: dict[Any, Column.BinaryFormula] = { - "http_response_rate": Column.BinaryFormula( - left=Column( - conditional_aggregation=AttributeConditionalAggregation( - aggregate=Function.FUNCTION_COUNT, - key=AttributeKey( - name="sentry.status_code", - type=AttributeKey.TYPE_STRING, - ), - filter=TraceItemFilter( - comparison_filter=ComparisonFilter( - key=AttributeKey( - name="sentry.status_code", - type=AttributeKey.TYPE_STRING, - ), - op=ComparisonFilter.OP_IN, - value=AttributeValue( - val_str_array=StrArray( - values=[ - "500", - "501", - "502", - "503", - "504", - "505", - "506", - "507", - "508", - "509", - "510", - "511", - ], - ), - ), - ) - ), - label="error_request_count", - extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, - ), - ), - op=Column.BinaryFormula.OP_DIVIDE, - right=Column( - conditional_aggregation=AttributeConditionalAggregation( - aggregate=Function.FUNCTION_COUNT, - key=AttributeKey( - name="sentry.status_code", - type=AttributeKey.TYPE_STRING, - ), - label="total_request_count", - extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, - ), - ), - ), -} - - -CUSTOM_FUNCTIONS: dict[Any, CustomFunction] = { - "http_response_rate": CustomFunction( - public_alias="http_response_rate_5", - formula=CUSTOM_FUNCTIONS_FORMULAS["http_response_rate"], - search_type="percentage", - ) -} diff --git a/src/sentry/search/eap/span_formulas.py b/src/sentry/search/eap/span_formulas.py new file mode 100644 index 00000000000000..d588355f533053 --- /dev/null +++ b/src/sentry/search/eap/span_formulas.py @@ -0,0 +1,107 @@ +from collections.abc import Callable +from typing import Any + +from sentry_protos.snuba.v1.attribute_conditional_aggregation_pb2 import ( + AttributeConditionalAggregation, +) +from sentry_protos.snuba.v1.endpoint_trace_item_table_pb2 import Column +from sentry_protos.snuba.v1.trace_item_attribute_pb2 import ( + AttributeKey, + AttributeValue, + ExtrapolationMode, + Function, + StrArray, +) +from sentry_protos.snuba.v1.trace_item_filter_pb2 import ComparisonFilter, TraceItemFilter + + +def http_response_rate(arg: str) -> Column.BinaryFormula: + code = int( + arg + ) # TODO - this is a bit of a hack, we should pass in the int directly if the arg type is int + # TODO - this should be handled in the function_definitions (span_columns.py) + if code not in [1, 2, 3, 4, 5]: + raise Exception("http_response_rate takes in a single digit (1,2,3,4,5)") + response_code_map = { + 1: ["100", "101", "102"], + 2: ["200", "201", "202", "203", "204", "205", "206", "207", "208", "226"], + 3: ["300", "301", "302", "303", "304", "305", "306", "307", "308"], + 4: [ + "400", + "401", + "402", + "403", + "404", + "405", + "406", + "407", + "408", + "409", + "410", + "411", + "412", + "413", + "414", + "415", + "416", + "417", + "418", + "421", + "422", + "423", + "424", + "425", + "426", + "428", + "429", + "431", + "451", + ], + 5: ["500", "501", "502", "503", "504", "505", "506", "507", "508", "509", "510", "511"], + } + + response_codes = response_code_map[code] + return Column.BinaryFormula( + left=Column( + conditional_aggregation=AttributeConditionalAggregation( + aggregate=Function.FUNCTION_COUNT, + key=AttributeKey( + name="sentry.status_code", + type=AttributeKey.TYPE_STRING, + ), + filter=TraceItemFilter( + comparison_filter=ComparisonFilter( + key=AttributeKey( + name="sentry.status_code", + type=AttributeKey.TYPE_STRING, + ), + op=ComparisonFilter.OP_IN, + value=AttributeValue( + val_str_array=StrArray( + values=response_codes, + ), + ), + ) + ), + label="error_request_count", + extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, + ), + ), + op=Column.BinaryFormula.OP_DIVIDE, + right=Column( + conditional_aggregation=AttributeConditionalAggregation( + aggregate=Function.FUNCTION_COUNT, + key=AttributeKey( + name="sentry.status_code", + type=AttributeKey.TYPE_STRING, + ), + label="total_request_count", + extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, + ), + ), + ) + + +CUSTOM_FUNCTION_RESOLVER: dict[str, Callable[[Any], Column.BinaryFormula]] = { + "http_response_rate": http_response_rate +} diff --git a/src/sentry/snuba/rpc_dataset_common.py b/src/sentry/snuba/rpc_dataset_common.py index c7030164a3f601..fb787ffed1841b 100644 --- a/src/sentry/snuba/rpc_dataset_common.py +++ b/src/sentry/snuba/rpc_dataset_common.py @@ -5,7 +5,7 @@ from sentry_protos.snuba.v1.request_common_pb2 import PageToken from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeAggregation, AttributeKey -from sentry.search.eap.columns import CustomFunction, ResolvedColumn, ResolvedFunction +from sentry.search.eap.columns import ResolvedColumn, ResolvedFormula, ResolvedFunction from sentry.search.eap.resolver import SearchResolver from sentry.search.eap.types import CONFIDENCES, ConfidenceData, EAPResponse from sentry.search.events.fields import get_function_alias @@ -16,8 +16,8 @@ logger = logging.getLogger("sentry.snuba.spans_rpc") -def categorize_column(column: ResolvedColumn | ResolvedFunction | CustomFunction) -> Column: - if isinstance(column, CustomFunction): +def categorize_column(column: ResolvedColumn | ResolvedFunction | ResolvedFormula) -> Column: + if isinstance(column, ResolvedFormula): return Column(formula=column.proto_definition, label=column.public_alias) if isinstance(column, ResolvedFunction): return Column(aggregation=column.proto_definition, label=column.public_alias) diff --git a/tests/snuba/api/endpoints/test_organization_events_span_indexed.py b/tests/snuba/api/endpoints/test_organization_events_span_indexed.py index c005bce28a1c25..495acd311542d7 100644 --- a/tests/snuba/api/endpoints/test_organization_events_span_indexed.py +++ b/tests/snuba/api/endpoints/test_organization_events_span_indexed.py @@ -2546,6 +2546,22 @@ def test_http_response_rate(self): ], is_eap=self.is_eap, ) + self.store_spans( + [ + self.create_span( + {"sentry_tags": {"status_code": "500"}}, start_ts=self.ten_mins_ago + ), + ], + is_eap=self.is_eap, + ) + self.store_spans( + [ + self.create_span( + {"sentry_tags": {"status_code": "404"}}, start_ts=self.ten_mins_ago + ), + ], + is_eap=self.is_eap, + ) self.store_spans( [ self.create_span( @@ -2556,7 +2572,11 @@ def test_http_response_rate(self): ) response = self.do_request( { - "field": ["http_response_rate(5)"], + "field": [ + "http_response_rate(5)", + "http_response_rate(4)", + "http_response_rate(2)", + ], "project": self.project.id, "dataset": self.dataset, } @@ -2566,7 +2586,9 @@ def test_http_response_rate(self): data = response.data["data"] meta = response.data["meta"] assert len(data) == 1 - assert data[0]["http_response_rate_5"] == 0.5 + assert data[0]["http_response_rate(5)"] == 0.5 + assert data[0]["http_response_rate(4)"] == 0.25 + assert data[0]["http_response_rate(2)"] == 0.25 assert meta["dataset"] == self.dataset @pytest.mark.skip(reason="replay id alias not migrated over") From eb3a77786960ddd8ea3425e582ab9c13e0701baf Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Mon, 24 Feb 2025 14:23:04 -0500 Subject: [PATCH 3/9] working function with args --- src/sentry/search/eap/resolver.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/search/eap/resolver.py b/src/sentry/search/eap/resolver.py index 8dfcc44258edcb..1103e8e47e825e 100644 --- a/src/sentry/search/eap/resolver.py +++ b/src/sentry/search/eap/resolver.py @@ -325,7 +325,7 @@ def resolve_virtual_context_term( self, term: str, raw_value: str | list[str], - resolved_column: ResolvedColumn, + resolved_column: ResolvedColumn | ResolvedFormula | ResolvedFormula, context: VirtualColumnDefinition, ) -> list[str] | str: # Convert the term to the expected values @@ -469,7 +469,7 @@ def resolve_aggregate_term( def _resolve_search_value( self, - column: ResolvedColumn, + column: ResolvedColumn | ResolvedFormula, operator: str, value: str | float | datetime | Sequence[float] | Sequence[str], ) -> AttributeValue: From 40064c30061503d42ed359683c9f4da23143f317 Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Mon, 24 Feb 2025 14:43:41 -0500 Subject: [PATCH 4/9] fix logic --- src/sentry/search/eap/resolver.py | 22 ---------------------- src/sentry/search/eap/span_columns.py | 4 +++- 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/src/sentry/search/eap/resolver.py b/src/sentry/search/eap/resolver.py index 1103e8e47e825e..1e8002f1723095 100644 --- a/src/sentry/search/eap/resolver.py +++ b/src/sentry/search/eap/resolver.py @@ -710,28 +710,6 @@ def resolve_aggregate( for index, argument in enumerate(function_definition.arguments): if argument.ignored: continue - - # if a function only accepts one type of argument, we can resolve it directly - if ( - argument.argument_types is not None - and len(argument.argument_types) == 1 - and index < len(attribute_args) - ): - arg = attribute_args[index] - arg_type = next(iter(argument.argument_types)) - if ( - arg_type == constants.TYPE_TO_STRING_MAP.get(AttributeKey.TYPE_INT) - and arg.isdigit() - ): - parsed_argument = ResolvedColumn( - public_alias=arg, - internal_name=arg, - search_type="integer", - ) - else: - parsed_argument = ResolvedColumn( - public_alias=arg, internal_name=arg, search_type="string" - ) elif index < len(attribute_args): parsed_argument, _ = self.resolve_attribute(attribute_args[index]) elif argument.default_arg: diff --git a/src/sentry/search/eap/span_columns.py b/src/sentry/search/eap/span_columns.py index e553aa7085f576..8ac5b86b06a23e 100644 --- a/src/sentry/search/eap/span_columns.py +++ b/src/sentry/search/eap/span_columns.py @@ -625,7 +625,9 @@ def count_processor(count_value: int | None) -> int: default_search_type="percentage", arguments=[ ArgumentDefinition( - argument_types={"integer"}, + argument_types={ + "string" + }, # TODO - this should be an integer, but `resolve_attribute` returns a string default_arg=None, # TODO - this should only accept 2,3,4,5 ) ], From dc05a4f26ad1fae3d81cff522ac368a6e38d2bcc Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Mon, 24 Feb 2025 14:45:54 -0500 Subject: [PATCH 5/9] remove column --- src/sentry/search/eap/columns.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/sentry/search/eap/columns.py b/src/sentry/search/eap/columns.py index ddfeb6febb4614..468bf6249df840 100644 --- a/src/sentry/search/eap/columns.py +++ b/src/sentry/search/eap/columns.py @@ -17,8 +17,6 @@ from sentry.search.eap import constants from sentry.search.events.types import SnubaParams -# from sentry.snuba.rpc_dataset_common import Column - @dataclass(frozen=True, kw_only=True) class ResolvedAttribute: From 9b98b9d3c3933262623e4a945af4f5caa42a568e Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Mon, 24 Feb 2025 14:51:58 -0500 Subject: [PATCH 6/9] updates --- src/sentry/search/eap/resolver.py | 2 +- src/sentry/search/eap/span_formulas.py | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/sentry/search/eap/resolver.py b/src/sentry/search/eap/resolver.py index 1e8002f1723095..743e517317d28d 100644 --- a/src/sentry/search/eap/resolver.py +++ b/src/sentry/search/eap/resolver.py @@ -710,7 +710,7 @@ def resolve_aggregate( for index, argument in enumerate(function_definition.arguments): if argument.ignored: continue - elif index < len(attribute_args): + if index < len(attribute_args): parsed_argument, _ = self.resolve_attribute(attribute_args[index]) elif argument.default_arg: parsed_argument, _ = self.resolve_attribute(argument.default_arg) diff --git a/src/sentry/search/eap/span_formulas.py b/src/sentry/search/eap/span_formulas.py index d588355f533053..ff2e4be28682ad 100644 --- a/src/sentry/search/eap/span_formulas.py +++ b/src/sentry/search/eap/span_formulas.py @@ -17,11 +17,13 @@ def http_response_rate(arg: str) -> Column.BinaryFormula: code = int( - arg - ) # TODO - this is a bit of a hack, we should pass in the int directly if the arg type is int - # TODO - this should be handled in the function_definitions (span_columns.py) + arg # TODO - converting this arg is a bit of a hack, we should pass in the int directly if the arg type is int + ) + + # TODO - handling valid parameters should be handled in the function_definitions (span_columns.py) if code not in [1, 2, 3, 4, 5]: raise Exception("http_response_rate takes in a single digit (1,2,3,4,5)") + response_code_map = { 1: ["100", "101", "102"], 2: ["200", "201", "202", "203", "204", "205", "206", "207", "208", "226"], From c9116547453966f05d7556cae7fd18669dc5dc89 Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Mon, 24 Feb 2025 14:57:05 -0500 Subject: [PATCH 7/9] typing --- src/sentry/search/eap/columns.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sentry/search/eap/columns.py b/src/sentry/search/eap/columns.py index 468bf6249df840..adf2b0eb430494 100644 --- a/src/sentry/search/eap/columns.py +++ b/src/sentry/search/eap/columns.py @@ -4,6 +4,7 @@ from typing import Any from dateutil.tz import tz +from sentry_protos.snuba.v1.endpoint_trace_item_table_pb2 import Column from sentry_protos.snuba.v1.request_common_pb2 import TraceItemType from sentry_protos.snuba.v1.trace_item_attribute_pb2 import ( AttributeAggregation, @@ -98,10 +99,10 @@ class VirtualColumnDefinition: @dataclass(frozen=True, kw_only=True) class ResolvedFormula(ResolvedAttribute): - formula: Any + formula: Column.BinaryFormula @property - def proto_definition(self) -> Any: + def proto_definition(self) -> Column.BinaryFormula: """The definition of this function as needed by the RPC""" return self.formula From f2e0812810751b97a996b5caec597cf884a2ad42 Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Tue, 25 Feb 2025 15:31:48 -0500 Subject: [PATCH 8/9] code review --- src/sentry/search/eap/columns.py | 69 +++++++---- src/sentry/search/eap/constants.py | 38 ++++++ src/sentry/search/eap/ourlog_columns.py | 1 + src/sentry/search/eap/resolver.py | 10 +- src/sentry/search/eap/span_columns.py | 87 +++++++++++++- src/sentry/search/eap/span_formulas.py | 109 ------------------ src/sentry/search/eap/uptime_check_columns.py | 1 + .../test_organization_events_span_indexed.py | 25 ++++ 8 files changed, 200 insertions(+), 140 deletions(-) delete mode 100644 src/sentry/search/eap/span_formulas.py diff --git a/src/sentry/search/eap/columns.py b/src/sentry/search/eap/columns.py index adf2b0eb430494..4aa5992514862f 100644 --- a/src/sentry/search/eap/columns.py +++ b/src/sentry/search/eap/columns.py @@ -160,8 +160,6 @@ class FunctionDefinition: processor: Callable[[Any], Any] | None = None # Whether to request extrapolation or not, should be true for all functions except for _sample functions for debugging extrapolation: bool = True - # For more complex functions, a formula can be specified - formula_resolver: Callable[[Any], Any] | None = None @property def required_arguments(self) -> list[ArgumentDefinition]: @@ -169,28 +167,50 @@ def required_arguments(self) -> list[ArgumentDefinition]: def resolve( self, alias: str, search_type: constants.SearchType, resolved_argument: AttributeKey | None - ) -> ResolvedFormula | ResolvedFunction: - if self.formula_resolver: - return ResolvedFormula( - public_alias=alias, - search_type=search_type, - formula=self.formula_resolver( - resolved_argument.name if resolved_argument else None - ), - argument=resolved_argument, - internal_type=self.internal_type, - processor=self.processor, - ) - else: - return ResolvedFunction( - public_alias=alias, - internal_name=self.internal_function, - search_type=search_type, - internal_type=self.internal_type, - processor=self.processor, - extrapolation=self.extrapolation, - argument=resolved_argument, - ) + ) -> ResolvedFunction: + return ResolvedFunction( + public_alias=alias, + internal_name=self.internal_function, + search_type=search_type, + internal_type=self.internal_type, + processor=self.processor, + extrapolation=self.extrapolation, + argument=resolved_argument, + ) + + +@dataclass +class FormulaDefinition: + # The list of arguments for this function + arguments: list[ArgumentDefinition] + # A function that takes in the resolved argument and returns a Column.BinaryFormula + formula_resolver: Callable[[Any], Any] + # The search_type the argument should be the default type for this column + default_search_type: constants.SearchType + # Try to infer the search type from the function arguments + infer_search_type_from_arguments: bool = True + # The internal rpc type for this function, optional as it can mostly be inferred from search_type + internal_type: AttributeKey.Type.ValueType | None = None + # Processor is the function run in the post process step to transform a row into the final result + processor: Callable[[Any], Column.BinaryFormula] | None = None + # Whether to request extrapolation or not, should be true for all functions except for _sample functions for debugging + extrapolation: bool = True + + @property + def required_arguments(self) -> list[ArgumentDefinition]: + return [arg for arg in self.arguments if arg.default_arg is None and not arg.ignored] + + def resolve( + self, alias: str, search_type: constants.SearchType, resolved_argument: AttributeKey | None + ) -> ResolvedFormula: + return ResolvedFormula( + public_alias=alias, + search_type=search_type, + formula=self.formula_resolver(resolved_argument.name if resolved_argument else None), + argument=resolved_argument, + internal_type=self.internal_type, + processor=self.processor, + ) def simple_sentry_field(field) -> ResolvedColumn: @@ -244,6 +264,7 @@ def project_term_resolver( @dataclass(frozen=True) class ColumnDefinitions: functions: dict[str, FunctionDefinition] + formulas: dict[str, FormulaDefinition] columns: dict[str, ResolvedColumn] contexts: dict[str, VirtualColumnDefinition] trace_item_type: TraceItemType.ValueType diff --git a/src/sentry/search/eap/constants.py b/src/sentry/search/eap/constants.py index b012494e1fc57b..fb1ac78a77ffc1 100644 --- a/src/sentry/search/eap/constants.py +++ b/src/sentry/search/eap/constants.py @@ -116,3 +116,41 @@ PROJECT_FIELDS = {"project", "project.slug", "project.name"} REVERSE_CONTEXT_ERROR = "Unknown value {} for filter {}, expecting one of: {}" + +RESPONSE_CODE_MAP = { + 1: ["100", "101", "102"], + 2: ["200", "201", "202", "203", "204", "205", "206", "207", "208", "226"], + 3: ["300", "301", "302", "303", "304", "305", "306", "307", "308"], + 4: [ + "400", + "401", + "402", + "403", + "404", + "405", + "406", + "407", + "408", + "409", + "410", + "411", + "412", + "413", + "414", + "415", + "416", + "417", + "418", + "421", + "422", + "423", + "424", + "425", + "426", + "428", + "429", + "431", + "451", + ], + 5: ["500", "501", "502", "503", "504", "505", "506", "507", "508", "509", "510", "511"], +} diff --git a/src/sentry/search/eap/ourlog_columns.py b/src/sentry/search/eap/ourlog_columns.py index b117b346286302..be3a492d3a2c44 100644 --- a/src/sentry/search/eap/ourlog_columns.py +++ b/src/sentry/search/eap/ourlog_columns.py @@ -74,6 +74,7 @@ OURLOG_DEFINITIONS = ColumnDefinitions( functions={}, + formulas={}, columns=OURLOG_ATTRIBUTE_DEFINITIONS, contexts=OURLOG_VIRTUAL_CONTEXTS, trace_item_type=TraceItemType.TRACE_ITEM_TYPE_LOG, diff --git a/src/sentry/search/eap/resolver.py b/src/sentry/search/eap/resolver.py index 743e517317d28d..c1ab475035e929 100644 --- a/src/sentry/search/eap/resolver.py +++ b/src/sentry/search/eap/resolver.py @@ -36,6 +36,8 @@ from sentry.search.eap import constants from sentry.search.eap.columns import ( ColumnDefinitions, + FormulaDefinition, + FunctionDefinition, ResolvedColumn, ResolvedFormula, ResolvedFunction, @@ -694,9 +696,13 @@ def resolve_aggregate( alias = match.group("alias") or column # Get the function definition - if function not in self.definitions.functions: + function_definition: FunctionDefinition | FormulaDefinition + if function in self.definitions.functions: + function_definition = self.definitions.functions[function] + elif function in self.definitions.formulas: + function_definition = self.definitions.formulas[function] + else: raise InvalidSearchQuery(f"Unknown function {function}") - function_definition = self.definitions.functions[function] parsed_columns = [] diff --git a/src/sentry/search/eap/span_columns.py b/src/sentry/search/eap/span_columns.py index 8ac5b86b06a23e..9eaf39fd5c5ade 100644 --- a/src/sentry/search/eap/span_columns.py +++ b/src/sentry/search/eap/span_columns.py @@ -1,12 +1,27 @@ -from typing import Literal +from collections.abc import Callable +from typing import Any, Literal +from sentry_protos.snuba.v1.attribute_conditional_aggregation_pb2 import ( + AttributeConditionalAggregation, +) +from sentry_protos.snuba.v1.endpoint_trace_item_table_pb2 import Column from sentry_protos.snuba.v1.request_common_pb2 import TraceItemType -from sentry_protos.snuba.v1.trace_item_attribute_pb2 import Function, VirtualColumnContext +from sentry_protos.snuba.v1.trace_item_attribute_pb2 import ( + AttributeKey, + AttributeValue, + ExtrapolationMode, + Function, + StrArray, + VirtualColumnContext, +) +from sentry_protos.snuba.v1.trace_item_filter_pb2 import ComparisonFilter, TraceItemFilter +from sentry.exceptions import InvalidSearchQuery from sentry.search.eap import constants from sentry.search.eap.columns import ( ArgumentDefinition, ColumnDefinitions, + FormulaDefinition, FunctionDefinition, ResolvedColumn, VirtualColumnDefinition, @@ -17,7 +32,7 @@ simple_sentry_field, ) from sentry.search.eap.common_columns import COMMON_COLUMNS -from sentry.search.eap.span_formulas import CUSTOM_FUNCTION_RESOLVER +from sentry.search.eap.constants import RESPONSE_CODE_MAP from sentry.search.events.constants import ( PRECISE_FINISH_TS, PRECISE_START_TS, @@ -387,6 +402,65 @@ def count_processor(count_value: int | None) -> int: return count_value +def http_response_rate(arg: str) -> Column.BinaryFormula: + + if not arg.isdigit(): + raise InvalidSearchQuery("http_response_rate accepts a single digit (1,2,3,4,5)") + + code = int( + arg # TODO - converting this arg is a bit of a hack, we should pass in the int directly if the arg type is int + ) + + # TODO - handling valid parameters should be handled in the function_definitions (span_columns.py) + if code not in [1, 2, 3, 4, 5]: + raise InvalidSearchQuery("http_response_rate accepts a single digit (1,2,3,4,5)") + + response_codes = RESPONSE_CODE_MAP[code] + return Column.BinaryFormula( + left=Column( + conditional_aggregation=AttributeConditionalAggregation( + aggregate=Function.FUNCTION_COUNT, + key=AttributeKey( + name="sentry.status_code", + type=AttributeKey.TYPE_STRING, + ), + filter=TraceItemFilter( + comparison_filter=ComparisonFilter( + key=AttributeKey( + name="sentry.status_code", + type=AttributeKey.TYPE_STRING, + ), + op=ComparisonFilter.OP_IN, + value=AttributeValue( + val_str_array=StrArray( + values=response_codes, # + ), + ), + ) + ), + label="error_request_count", + extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, + ), + ), + op=Column.BinaryFormula.OP_DIVIDE, + right=Column( + conditional_aggregation=AttributeConditionalAggregation( + aggregate=Function.FUNCTION_COUNT, + key=AttributeKey( + name="sentry.status_code", + type=AttributeKey.TYPE_STRING, + ), + label="total_request_count", + extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, + ), + ), + ) + + +CUSTOM_FUNCTION_RESOLVER: dict[str, Callable[[Any], Column.BinaryFormula]] = { + "http_response_rate": http_response_rate +} + SPAN_FUNCTION_DEFINITIONS = { "sum": FunctionDefinition( internal_function=Function.FUNCTION_SUM, @@ -620,8 +694,10 @@ def count_processor(count_value: int | None) -> int: ) ], ), - "http_response_rate": FunctionDefinition( - internal_function=Function.FUNCTION_UNSPECIFIED, +} + +SPAN_FORMULA_DEFINITIONS = { + "http_response_rate": FormulaDefinition( default_search_type="percentage", arguments=[ ArgumentDefinition( @@ -637,6 +713,7 @@ def count_processor(count_value: int | None) -> int: SPAN_DEFINITIONS = ColumnDefinitions( functions=SPAN_FUNCTION_DEFINITIONS, + formulas=SPAN_FORMULA_DEFINITIONS, columns=SPAN_ATTRIBUTE_DEFINITIONS, contexts=SPAN_VIRTUAL_CONTEXTS, trace_item_type=TraceItemType.TRACE_ITEM_TYPE_SPAN, diff --git a/src/sentry/search/eap/span_formulas.py b/src/sentry/search/eap/span_formulas.py deleted file mode 100644 index ff2e4be28682ad..00000000000000 --- a/src/sentry/search/eap/span_formulas.py +++ /dev/null @@ -1,109 +0,0 @@ -from collections.abc import Callable -from typing import Any - -from sentry_protos.snuba.v1.attribute_conditional_aggregation_pb2 import ( - AttributeConditionalAggregation, -) -from sentry_protos.snuba.v1.endpoint_trace_item_table_pb2 import Column -from sentry_protos.snuba.v1.trace_item_attribute_pb2 import ( - AttributeKey, - AttributeValue, - ExtrapolationMode, - Function, - StrArray, -) -from sentry_protos.snuba.v1.trace_item_filter_pb2 import ComparisonFilter, TraceItemFilter - - -def http_response_rate(arg: str) -> Column.BinaryFormula: - code = int( - arg # TODO - converting this arg is a bit of a hack, we should pass in the int directly if the arg type is int - ) - - # TODO - handling valid parameters should be handled in the function_definitions (span_columns.py) - if code not in [1, 2, 3, 4, 5]: - raise Exception("http_response_rate takes in a single digit (1,2,3,4,5)") - - response_code_map = { - 1: ["100", "101", "102"], - 2: ["200", "201", "202", "203", "204", "205", "206", "207", "208", "226"], - 3: ["300", "301", "302", "303", "304", "305", "306", "307", "308"], - 4: [ - "400", - "401", - "402", - "403", - "404", - "405", - "406", - "407", - "408", - "409", - "410", - "411", - "412", - "413", - "414", - "415", - "416", - "417", - "418", - "421", - "422", - "423", - "424", - "425", - "426", - "428", - "429", - "431", - "451", - ], - 5: ["500", "501", "502", "503", "504", "505", "506", "507", "508", "509", "510", "511"], - } - - response_codes = response_code_map[code] - return Column.BinaryFormula( - left=Column( - conditional_aggregation=AttributeConditionalAggregation( - aggregate=Function.FUNCTION_COUNT, - key=AttributeKey( - name="sentry.status_code", - type=AttributeKey.TYPE_STRING, - ), - filter=TraceItemFilter( - comparison_filter=ComparisonFilter( - key=AttributeKey( - name="sentry.status_code", - type=AttributeKey.TYPE_STRING, - ), - op=ComparisonFilter.OP_IN, - value=AttributeValue( - val_str_array=StrArray( - values=response_codes, - ), - ), - ) - ), - label="error_request_count", - extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, - ), - ), - op=Column.BinaryFormula.OP_DIVIDE, - right=Column( - conditional_aggregation=AttributeConditionalAggregation( - aggregate=Function.FUNCTION_COUNT, - key=AttributeKey( - name="sentry.status_code", - type=AttributeKey.TYPE_STRING, - ), - label="total_request_count", - extrapolation_mode=ExtrapolationMode.EXTRAPOLATION_MODE_NONE, - ), - ), - ) - - -CUSTOM_FUNCTION_RESOLVER: dict[str, Callable[[Any], Column.BinaryFormula]] = { - "http_response_rate": http_response_rate -} diff --git a/src/sentry/search/eap/uptime_check_columns.py b/src/sentry/search/eap/uptime_check_columns.py index 52e7a29dde8e62..6e91643cdf87a0 100644 --- a/src/sentry/search/eap/uptime_check_columns.py +++ b/src/sentry/search/eap/uptime_check_columns.py @@ -87,6 +87,7 @@ UPTIME_CHECK_DEFINITIONS = ColumnDefinitions( functions={}, + formulas={}, columns=UPTIME_CHECK_ATTRIBUTE_DEFINITIONS, contexts=UPTIME_CHECK_VIRTUAL_CONTEXTS, trace_item_type=TraceItemType.TRACE_ITEM_TYPE_UPTIME_CHECK, diff --git a/tests/snuba/api/endpoints/test_organization_events_span_indexed.py b/tests/snuba/api/endpoints/test_organization_events_span_indexed.py index 495acd311542d7..39ff41f72a2472 100644 --- a/tests/snuba/api/endpoints/test_organization_events_span_indexed.py +++ b/tests/snuba/api/endpoints/test_organization_events_span_indexed.py @@ -2591,6 +2591,31 @@ def test_http_response_rate(self): assert data[0]["http_response_rate(2)"] == 0.25 assert meta["dataset"] == self.dataset + def test_http_response_rate_invalid_param(self): + self.store_spans( + [ + self.create_span( + {"sentry_tags": {"status_code": "500"}}, start_ts=self.ten_mins_ago + ), + ], + is_eap=self.is_eap, + ) + response = self.do_request( + { + "field": [ + "http_response_rate(a)", + ], + "project": self.project.id, + "dataset": self.dataset, + } + ) + + assert response.status_code == 400, response.content + assert ( + "Http_Response_Rate Accepts A Single Digit (1,2,3,4,5)" + == response.data["detail"].title() + ) + @pytest.mark.skip(reason="replay id alias not migrated over") def test_replay_id(self): super().test_replay_id() From 6956df8977dc6e16aa3ae005d50e4783f28959b4 Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Wed, 26 Feb 2025 11:03:56 -0500 Subject: [PATCH 9/9] typing --- src/sentry/snuba/spans_rpc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/snuba/spans_rpc.py b/src/sentry/snuba/spans_rpc.py index d621b707497ea9..db4ff8ea3996cc 100644 --- a/src/sentry/snuba/spans_rpc.py +++ b/src/sentry/snuba/spans_rpc.py @@ -11,7 +11,7 @@ from sentry.api.event_search import SearchFilter, SearchKey, SearchValue from sentry.exceptions import InvalidSearchQuery -from sentry.search.eap.columns import ResolvedColumn, ResolvedFunction +from sentry.search.eap.columns import ResolvedColumn, ResolvedFormula, ResolvedFunction from sentry.search.eap.constants import MAX_ROLLUP_POINTS, VALID_GRANULARITIES from sentry.search.eap.resolver import SearchResolver from sentry.search.eap.span_columns import SPAN_DEFINITIONS @@ -74,7 +74,7 @@ def get_timeseries_query( config: SearchResolverConfig, granularity_secs: int, extra_conditions: TraceItemFilter | None = None, -) -> tuple[TimeSeriesRequest, list[ResolvedFunction], list[ResolvedColumn]]: +) -> tuple[TimeSeriesRequest, list[ResolvedFunction | ResolvedFormula], list[ResolvedColumn]]: resolver = get_resolver(params=params, config=config) meta = resolver.resolve_meta(referrer=referrer) query, _, query_contexts = resolver.resolve_query(query_string)