Skip to content

Commit

Permalink
fix(rpc): Fix more of the events tests (#81934)
Browse files Browse the repository at this point in the history
- This fixes a bug with searching on fields with contexts where we
weren't passing context
- eg. if you did project:some-slug, the context to transform wasn't
being passed
- This fixes a bug where we weren't searching sentry_tags correctly
  • Loading branch information
wmak authored Dec 19, 2024
1 parent 630a90f commit 1448a59
Show file tree
Hide file tree
Showing 5 changed files with 215 additions and 72 deletions.
17 changes: 16 additions & 1 deletion src/sentry/search/eap/columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from sentry.exceptions import InvalidSearchQuery
from sentry.search.eap import constants
from sentry.search.events.constants import SPAN_MODULE_CATEGORY_VALUES
from sentry.search.events.types import SnubaParams
from sentry.search.utils import DEVICE_CLASS
from sentry.utils.validators import is_event_id, is_span_id
Expand Down Expand Up @@ -246,6 +247,11 @@ def datetime_processor(datetime_string: str) -> str:
internal_name="sentry.status",
search_type="string",
),
ResolvedColumn(
public_alias="span.status_code",
internal_name="sentry.status_code",
search_type="string",
),
ResolvedColumn(
public_alias="trace",
internal_name="sentry.trace_id",
Expand Down Expand Up @@ -321,7 +327,6 @@ def datetime_processor(datetime_string: str) -> str:
simple_sentry_field("release"),
simple_sentry_field("sdk.name"),
simple_sentry_field("sdk.version"),
simple_sentry_field("span.status_code"),
simple_sentry_field("span_id"),
simple_sentry_field("trace.status"),
simple_sentry_field("transaction.method"),
Expand Down Expand Up @@ -430,11 +435,21 @@ def device_class_context_constructor(params: SnubaParams) -> VirtualColumnContex
)


def module_context_constructor(params: SnubaParams) -> VirtualColumnContext:
value_map = {key: key for key in SPAN_MODULE_CATEGORY_VALUES}
return VirtualColumnContext(
from_column_name="sentry.category",
to_column_name="span.module",
value_map=value_map,
)


VIRTUAL_CONTEXTS = {
"project": project_context_constructor("project"),
"project.slug": project_context_constructor("project.slug"),
"project.name": project_context_constructor("project.name"),
"device.class": device_class_context_constructor,
"span.module": module_context_constructor,
}


Expand Down
105 changes: 68 additions & 37 deletions src/sentry/search/eap/spans.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ def resolve_meta(self, referrer: str) -> RequestMeta:
)

@sentry_sdk.trace
def resolve_query(self, querystring: str | None) -> TraceItemFilter | None:
def resolve_query(
self, querystring: str | None
) -> tuple[TraceItemFilter | None, list[VirtualColumnContext | None]]:
"""Given a query string in the public search syntax eg. `span.description:foo` construct the TraceItemFilter"""
environment_query = self.__resolve_environment_query()
query = self.__resolve_query(querystring)
query, contexts = self.__resolve_query(querystring)
span = sentry_sdk.get_current_span()
if span:
span.set_tag("SearchResolver.query_string", querystring)
Expand All @@ -89,18 +91,21 @@ def resolve_query(self, querystring: str | None) -> TraceItemFilter | None:
# But if both are defined, we AND them together.

if not environment_query:
return query
return query, contexts

if not query:
return environment_query

return TraceItemFilter(
and_filter=AndFilter(
filters=[
environment_query,
query,
]
)
return environment_query, []

return (
TraceItemFilter(
and_filter=AndFilter(
filters=[
environment_query,
query,
]
)
),
contexts,
)

def __resolve_environment_query(self) -> TraceItemFilter | None:
Expand All @@ -126,9 +131,11 @@ def __resolve_environment_query(self) -> TraceItemFilter | None:

return TraceItemFilter(and_filter=AndFilter(filters=filters))

def __resolve_query(self, querystring: str | None) -> TraceItemFilter | None:
def __resolve_query(
self, querystring: str | None
) -> tuple[TraceItemFilter | None, list[VirtualColumnContext | None]]:
if querystring is None:
return None
return None, []
try:
parsed_terms = event_search.parse_search_query(
querystring,
Expand All @@ -153,8 +160,10 @@ def __resolve_query(self, querystring: str | None) -> TraceItemFilter | None:

def _resolve_boolean_conditions(
self, terms: event_filter.ParsedTerms
) -> TraceItemFilter | None:
if len(terms) == 1:
) -> tuple[TraceItemFilter | None, list[VirtualColumnContext | None]]:
if len(terms) == 0:
return None, []
elif len(terms) == 1:
if isinstance(terms[0], event_search.ParenExpression):
return self._resolve_boolean_conditions(terms[0].children)
elif isinstance(terms[0], event_search.SearchFilter):
Expand Down Expand Up @@ -207,38 +216,54 @@ def _resolve_boolean_conditions(
lhs, rhs = terms[:1], terms[1:]
operator = AndFilter

resolved_lhs = self._resolve_boolean_conditions(lhs) if lhs else None
resolved_rhs = self._resolve_boolean_conditions(rhs) if rhs else None
resolved_lhs, contexts_lhs = self._resolve_boolean_conditions(lhs)
resolved_rhs, contexts_rhs = self._resolve_boolean_conditions(rhs)
contexts = contexts_lhs + contexts_rhs

if resolved_lhs is not None and resolved_rhs is not None:
if operator == AndFilter:
return TraceItemFilter(and_filter=AndFilter(filters=[resolved_lhs, resolved_rhs]))
return (
TraceItemFilter(and_filter=AndFilter(filters=[resolved_lhs, resolved_rhs])),
contexts,
)
else:
return TraceItemFilter(or_filter=OrFilter(filters=[resolved_lhs, resolved_rhs]))
return (
TraceItemFilter(or_filter=OrFilter(filters=[resolved_lhs, resolved_rhs])),
contexts,
)
elif resolved_lhs is None and resolved_rhs is not None:
return resolved_rhs
return resolved_rhs, contexts
elif resolved_lhs is not None and resolved_rhs is None:
return resolved_lhs
return resolved_lhs, contexts
else:
return None
return None, contexts

def _resolve_terms(self, terms: event_filter.ParsedTerms) -> TraceItemFilter | None:
def _resolve_terms(
self, terms: event_filter.ParsedTerms
) -> tuple[TraceItemFilter | None, list[VirtualColumnContext | None]]:
parsed_terms = []
resolved_contexts = []
for item in terms:
if isinstance(item, event_search.SearchFilter):
parsed_terms.append(self.resolve_term(cast(event_search.SearchFilter, item)))
resolved_term, resolved_context = self.resolve_term(
cast(event_search.SearchFilter, item)
)
parsed_terms.append(resolved_term)
resolved_contexts.append(resolved_context)
else:
if self.config.use_aggregate_conditions:
raise NotImplementedError("Can't filter on aggregates yet")

if len(parsed_terms) > 1:
return TraceItemFilter(and_filter=AndFilter(filters=parsed_terms))
return TraceItemFilter(and_filter=AndFilter(filters=parsed_terms)), resolved_contexts
elif len(parsed_terms) == 1:
return parsed_terms[0]
return parsed_terms[0], resolved_contexts
else:
return None
return None, []

def resolve_term(self, term: event_search.SearchFilter) -> TraceItemFilter:
def resolve_term(
self, term: event_search.SearchFilter
) -> tuple[TraceItemFilter, VirtualColumnContext | None]:
resolved_column, context = self.resolve_column(term.key.name)
raw_value = term.value.raw_value
if term.value.is_wildcard():
Expand All @@ -262,12 +287,15 @@ def resolve_term(self, term: event_search.SearchFilter) -> TraceItemFilter:
else:
raise InvalidSearchQuery(f"Unknown operator: {term.operator}")
if isinstance(resolved_column.proto_definition, AttributeKey):
return TraceItemFilter(
comparison_filter=ComparisonFilter(
key=resolved_column.proto_definition,
op=operator,
value=self._resolve_search_value(resolved_column, term.operator, raw_value),
)
return (
TraceItemFilter(
comparison_filter=ComparisonFilter(
key=resolved_column.proto_definition,
op=operator,
value=self._resolve_search_value(resolved_column, term.operator, raw_value),
)
),
context,
)
else:
raise NotImplementedError("Can't filter on aggregates yet")
Expand Down Expand Up @@ -339,7 +367,7 @@ def clean_contexts(
@sentry_sdk.trace
def resolve_columns(
self, selected_columns: list[str]
) -> tuple[list[ResolvedColumn | ResolvedFunction], list[VirtualColumnContext]]:
) -> tuple[list[ResolvedColumn | ResolvedFunction], list[VirtualColumnContext | 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
Expand Down Expand Up @@ -370,7 +398,7 @@ def resolve_columns(
resolved_columns.append(project_column)
resolved_contexts.append(project_context)

return resolved_columns, self.clean_contexts(resolved_contexts)
return resolved_columns, resolved_contexts

def resolve_column(
self, column: str, match: Match | None = None
Expand Down Expand Up @@ -436,6 +464,9 @@ def resolve_attribute(self, column: str) -> tuple[ResolvedColumn, VirtualColumnC
if field_type not in constants.TYPE_MAP:
raise InvalidSearchQuery(f"Unsupported type {field_type} in {column}")

if column.startswith("sentry_tags"):
field = f"sentry.{field}"

search_type = cast(constants.SearchType, field_type)
column_definition = ResolvedColumn(
public_alias=column, internal_name=field, search_type=search_type
Expand Down
13 changes: 8 additions & 5 deletions src/sentry/snuba/spans_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ def run_table_query(
SearchResolver(params=params, config=config) if search_resolver is None else search_resolver
)
meta = resolver.resolve_meta(referrer=referrer)
query = resolver.resolve_query(query_string)
columns, contexts = resolver.resolve_columns(selected_columns)
query, query_contexts = resolver.resolve_query(query_string)
columns, column_contexts = resolver.resolve_columns(selected_columns)
contexts = resolver.clean_contexts(query_contexts + column_contexts)
# We allow orderby function_aliases if they're a selected_column
# eg. can orderby sum_span_self_time, assuming sum(span.self_time) is selected
orderby_aliases = {
Expand Down Expand Up @@ -155,7 +156,7 @@ def get_timeseries_query(
) -> TimeSeriesRequest:
resolver = SearchResolver(params=params, config=config)
meta = resolver.resolve_meta(referrer=referrer)
query = resolver.resolve_query(query_string)
query, query_contexts = resolver.resolve_query(query_string)
(aggregations, _) = resolver.resolve_aggregates(y_axes)
(groupbys, _) = resolver.resolve_columns(groupby)
if extra_conditions is not None:
Expand All @@ -178,6 +179,8 @@ def get_timeseries_query(
if isinstance(groupby.proto_definition, AttributeKey)
],
granularity_secs=granularity_secs,
# TODO: need to add this once the RPC supports it
# virtual_column_contexts=[context for context in resolver.clean_contexts(query_contexts) if context is not None],
)


Expand Down Expand Up @@ -285,7 +288,7 @@ def build_top_event_conditions(
]
else:
value = event[key]
resolved_term = resolver.resolve_term(
resolved_term, context = resolver.resolve_term(
SearchFilter(
key=SearchKey(name=key),
operator="=",
Expand All @@ -294,7 +297,7 @@ def build_top_event_conditions(
)
if resolved_term is not None:
row_conditions.append(resolved_term)
other_term = resolver.resolve_term(
other_term, context = resolver.resolve_term(
SearchFilter(
key=SearchKey(name=key),
operator="!=",
Expand Down
Loading

0 comments on commit 1448a59

Please sign in to comment.