Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions snuba/web/rpc/v1/trace_item_attribute_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
from snuba.datasets.entities.entity_key import EntityKey
from snuba.datasets.entities.factory import get_entity
from snuba.datasets.pluggable_dataset import PluggableDataset
from snuba.protos.common import ATTRIBUTES_TO_COALESCE
from snuba.query import OrderBy, OrderByDirection, SelectedExpression
from snuba.query.composite import CompositeQuery
from snuba.query.conditions import combine_or_conditions
from snuba.query.data_source.simple import Entity, LogicalDataSource
from snuba.query.dsl import Functions as f
from snuba.query.dsl import column
Expand All @@ -30,6 +32,7 @@
from snuba.web.query import run_query
from snuba.web.rpc import RPCEndpoint
from snuba.web.rpc.common.common import (
add_existence_check_to_subscriptable_references,
attribute_key_to_expression,
base_conditions_and,
treeify_or_and_conditions,
Expand All @@ -40,12 +43,26 @@
)


def _map_key_names_for_existence_check(request_key: AttributeKey) -> list[str]:
"""Map key names that may hold values (canonical plus deprecated/alias names)."""
names = [request_key.name]
for alt in ATTRIBUTES_TO_COALESCE.get(request_key.name, ()):
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ATTRIBUTES_TO_COALESCE values are set[str], so iterating them makes the generated OR condition order nondeterministic across runs (and across Python hash seeds). This can make query serialization/caching/metrics noisier and can complicate debugging. Consider iterating a sorted list of alternates (e.g. for alt in sorted(...)) to keep the query AST stable.

Suggested change
for alt in ATTRIBUTES_TO_COALESCE.get(request_key.name, ()):
for alt in sorted(ATTRIBUTES_TO_COALESCE.get(request_key.name, ())):

Copilot uses AI. Check for mistakes.
if alt not in names:
names.append(alt)
return names


def _build_conditions(request: TraceItemAttributeValuesRequest) -> Expression:
attribute_key = attribute_key_to_expression(request.key)

conditions: list[Expression] = [
f.has(column("attributes_string"), getattr(attribute_key, "key", request.key.name)),
]
key_existence = combine_or_conditions(
[
f.has(column("attributes_string"), name)
for name in _map_key_names_for_existence_check(request.key)
]
)

conditions: list[Expression] = [key_existence]
if request.meta.trace_item_type:
conditions.append(f.equals(column("item_type"), request.meta.trace_item_type))

Expand Down Expand Up @@ -103,6 +120,7 @@ def _build_query(
limit=10000,
)
treeify_or_and_conditions(inner_query)
add_existence_check_to_subscriptable_references(inner_query)
res = CompositeQuery(
from_clause=inner_query,
selected_columns=[
Expand Down
39 changes: 38 additions & 1 deletion tests/web/rpc/v1/test_trace_item_attribute_values_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@

import pytest
from google.protobuf.timestamp_pb2 import Timestamp
from sentry_protos.snuba.v1.downsampled_storage_pb2 import DownsampledStorageConfig
from sentry_protos.snuba.v1.endpoint_trace_item_attributes_pb2 import (
TraceItemAttributeValuesRequest,
)
from sentry_protos.snuba.v1.request_common_pb2 import RequestMeta
from sentry_protos.snuba.v1.request_common_pb2 import RequestMeta, TraceItemType
from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey
from sentry_protos.snuba.v1.trace_item_pb2 import AnyValue
from sentry_protos.snuba.v1.trace_item_pb2 import TraceItem as TraceItemMessage
Expand Down Expand Up @@ -181,3 +182,39 @@ def test_item_id_substring_match(self, setup_teardown: List[bytes]) -> None:
)
res = AttributeValuesRequest().execute(req)
assert res.values == [item_id]

def test_deprecated_alias_attribute(self) -> None:
"""db.system.name request returns values stored only under deprecated key db.system."""

items_storage = get_storage(StorageKey("eap_items"))
write_raw_unprocessed_events(
items_storage, # type: ignore
[
gen_item_message(
start_timestamp=BASE_TIME,
attributes={"db.system": AnyValue(string_value="redis")},
),
gen_item_message(
start_timestamp=BASE_TIME,
attributes={"db.system": AnyValue(string_value="postgresql")},
),
],
)
message = TraceItemAttributeValuesRequest(
meta=RequestMeta(
organization_id=1,
project_ids=[1],
cogs_category="something",
referrer="api.spans.tags-values.rpc",
start_timestamp=Timestamp(seconds=int(BASE_TIME.timestamp())),
end_timestamp=Timestamp(seconds=int((BASE_TIME + timedelta(days=1)).timestamp())),
trace_item_type=TraceItemType.TRACE_ITEM_TYPE_SPAN,
downsampled_storage_config=DownsampledStorageConfig(
mode=DownsampledStorageConfig.MODE_NORMAL
),
),
key=AttributeKey(name="db.system.name", type=AttributeKey.TYPE_STRING),
limit=1001,
)
response = AttributeValuesRequest().execute(message)
assert sorted(response.values) == ["postgresql", "redis"]
Loading