Skip to content
Open
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
36 changes: 36 additions & 0 deletions snuba/datasets/configuration/issues/entities/search_issues.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,24 @@ schema:
[ { name: key, type: String }, { name: value, type: String } ],
},
},
{
name: flags,
type: Nested,
args:
{
subcolumns:
[ { name: key, type: String }, { name: value, type: String } ],
},
},
{
name: _flags_hash_map,
type: Array,
args:
{
schema_modifiers: [ readonly ],
inner_type: { type: UInt, args: { size: 64 } },
},
},
{ name: user, type: String, args: { schema_modifiers: [ nullable ] } },
{ name: user_hash, type: UInt, args: { size: 64, schema_modifiers: [ nullable ] } },
{ name: user_id, type: String, args: { schema_modifiers: [ nullable ] } },
Expand Down Expand Up @@ -173,6 +191,18 @@ storages:
from_col_name: tags_value
to_function_name: arrayJoin
to_function_column: tags.value
- mapper: ColumnToFunctionOnColumn
args:
from_table_name: null
from_col_name: flags_key
to_function_name: arrayJoin
to_function_column: flags.key
- mapper: ColumnToFunctionOnColumn
args:
from_table_name: null
from_col_name: flags_value
to_function_name: arrayJoin
to_function_column: flags.value
subscriptables:
- mapper: SubscriptableMapper
args:
Expand All @@ -186,6 +216,12 @@ storages:
from_column_name: contexts
to_nested_col_table:
to_nested_col_name: contexts
- mapper: SubscriptableMapper
args:
from_column_table:
from_column_name: flags
to_nested_col_table:
to_nested_col_name: flags

storage_selector:
selector: DefaultQueryStorageSelector
Expand Down
14 changes: 14 additions & 0 deletions snuba/datasets/configuration/issues/storages/search_issues.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ schema:
{ name: tags.key, type: Array, args: { inner_type: { type: String } } },
{ name: tags.value, type: Array, args: { inner_type: { type: String } } },
{ name: _tags_hash_map, type: Array, args: { inner_type: { type: UInt, args: { size: 64 } } } },
{ name: flags.key, type: Array, args: { inner_type: { type: String } } },
{ name: flags.value, type: Array, args: { inner_type: { type: String } } },
{ name: _flags_hash_map, type: Array, args: { inner_type: { type: UInt, args: { size: 64 } } } },
{ name: user, type: String, args: { schema_modifiers: [ nullable ] } },
{ name: user_hash, type: UInt, args: { size: 64, schema_modifiers: [ nullable ] } },
{ name: user_id, type: String, args: { schema_modifiers: [ nullable ] } },
Expand Down Expand Up @@ -126,6 +129,17 @@ query_processors:
- processor: ArrayJoinKeyValueOptimizer
args:
column_name: tags
- processor: MappingOptimizer
args:
column_name: flags
hash_map_name: _flags_hash_map
killswitch: search_issues_flags_hash_map_enabled
- processor: EmptyTagConditionProcessor
args:
column_name: flags.key
- processor: ArrayJoinKeyValueOptimizer
args:
column_name: flags
- processor: UUIDColumnProcessor
args:
columns: [occurrence_id, event_id, trace_id, profile_id, replay_id]
Expand Down
15 changes: 15 additions & 0 deletions snuba/datasets/processors/search_issues_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
extract_extra_contexts,
extract_extra_tags,
extract_http,
extract_nested,
extract_user,
)
from snuba.datasets.processors import DatasetMessageProcessor
Expand Down Expand Up @@ -66,6 +67,7 @@ class IssueEventData(TypedDict, total=False):
start_timestamp: float

tags: Mapping[str, Any]
flags: Mapping[str, Any]
user: Mapping[str, Any] # user_hash, user_id, user_name, user_email, ip_address
sdk: Mapping[str, Any] # sdk_name, sdk_version
contexts: Mapping[str, Any]
Expand Down Expand Up @@ -157,6 +159,18 @@ def _process_tags(
promoted_tags.get("sentry:dist", event_data.get("dist")),
)

def _process_flags(
self, event_data: IssueEventData, processed: MutableMapping[str, Any]
) -> None:
existing_flags = event_data.get("flags", None)
flags: Mapping[str, Any] = _as_dict_safe(cast(Dict[str, Any], existing_flags))
if not existing_flags:
processed["flags.key"], processed["flags.value"] = [], []
else:
processed["flags.key"], processed["flags.value"] = extract_nested(
flags, lambda s: _unicodify(s) or None
)
Copy link

Choose a reason for hiding this comment

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

Flags extracted from wrong location with wrong format

High Severity

The _process_flags method reads flags from event_data["flags"] as a flat dict, but Sentry's event payload stores feature flags inside data.contexts.flags.values as a list of {"flag": "name", "result": value} objects. This is confirmed by the Rust errors processor and its tests in test_errors_processor.py. Since there's no flags key at the top level of event data, event_data.get("flags", None) will always return None, and flags.key/flags.value will always be empty lists — making the flags feature silently non-functional for search issues.

Additional Locations (1)

Fix in Cursor Fix in Web


Comment on lines +163 to +173
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bug: The PR adds logic to process flags for search issues but is missing the required ClickHouse migration to add the flags and _flags_hash_map columns to the table.
Severity: CRITICAL

Suggested Fix

Add a new database migration for the search_issues storage. This migration should add the flags nested column and the _flags_hash_map materialized column with the FLAGS_HASH_MAP_COLUMN expression to the ClickHouse table schema.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: snuba/datasets/processors/search_issues_processor.py#L162-L173

Potential issue: The `_process_flags` method populates `flags.key` and `flags.value` for
search issues. However, the pull request is missing a corresponding database migration
to add the necessary columns to the `search_issues` ClickHouse table. Without the
migration, the `flags` nested column and the `_flags_hash_map` materialized column will
not exist. This will cause any attempt to insert an event with flags to fail due to a
column mismatch error, breaking data ingestion for those events.

Did we get this right? 👍 / 👎 to inform future reviews.

def _process_request_data(
self, event_data: IssueEventData, processed: MutableMapping[str, Any]
) -> None:
Expand Down Expand Up @@ -291,6 +305,7 @@ def process_insert_v1(
self._process_tags(
event_data, fields
) # environment, release, dist, user, tags.key, tags.value
self._process_flags(event_data, fields) # flags.key, flags.value
self._process_user(
event_data, fields
) # user_name, user_id, user_email, ip_address_v4/ip_address_v6
Expand Down
Loading