Skip to content

feat(eap): Read TraceItemAttributeNames exclusively from co-occurring v2 #8314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
phacops wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(eap): Read TraceItemAttributeNames exclusively from co-occurring v2 #8314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 0 additions & 10 deletions sentry-options/schemas/snuba/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,16 +688,6 @@
"default": "jittered",
"description": "TaskBuilderMode for the subscription scheduler: one of immediate, jittered, transition_jitter, transition_immediate."
},
"use_co_occurring_attrs_v2": {
"type": "boolean",
"default": false,
"description": "When true, the TraceItemAttributeNames RPC may read the v2 co-occurring-attributes storage (eap_item_co_occurring_attrs_v2) instead of v1. v2 is a SummingMergeTree carrying an occurrence count per attribute set (so count ordering sums that column rather than counting rows), a last_seen timestamp (which v1 lacks entirely, so ordering by COLUMN_LAST_SEEN is only honoured on v2 and degrades to COLUMN_COUNT otherwise), and one attribute-key array per type, which lets int and array-typed keys be returned with their real AttributeKey type. Enabling this is safe at any time: a request whose time range reaches back before co_occurring_attrs_v2_start_timestamp still reads v1, since v2 has no data there."
},
"co_occurring_attrs_v2_start_timestamp": {
"type": "integer",
"default": 1785110400,
"description": "Unix timestamp of the earliest weekly date bucket the v2 co-occurring-attributes tables hold data for; TraceItemAttributeNames only reads v2 for requests whose range starts at or after it, and reads v1 otherwise. Defaults to 2026-07-27 00:00 UTC, the Monday of the week the v2 tables and their materialized view were created. Must be a Monday: the date column is bucketed weekly with toMonday() and the query rounds its lower bound down to the previous Monday, so a mid-week value would let a request read a bucket v2 never populated. Lower it only to a bucket v2 has been backfilled to."
},
"eap_items_use_indexed_name_organization_ids": {
"type": "array",
"items": {
Expand Down
70 changes: 13 additions & 57 deletions snuba/web/rpc/v1/endpoint_trace_item_attribute_names.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@
)
from snuba.web.rpc.common.debug_info import extract_response_meta
from snuba.web.rpc.proto_visitor import ProtoVisitor, TraceItemFilterWrapper
from snuba.web.rpc.v1.resolvers.R_eap_items import co_occurring_attrs
from snuba.web.rpc.v1.resolvers.R_eap_items.co_occurring_attrs import CoOccurringAttrsSource
from snuba.web.rpc.v1.resolvers.R_eap_items.co_occurring_attrs import (
V2,
CoOccurringAttrsSource,
)

# max value the user can provide for 'limit' in their request
MAX_REQUEST_LIMIT = 1000
Expand Down Expand Up @@ -71,29 +73,6 @@ def _order_by_count(request: TraceItemAttributeNamesRequest) -> bool:
return request.order_by.column == TraceItemAttributeNamesRequest.OrderBy.Column.COLUMN_COUNT


def _effective_order_by_column(
request: TraceItemAttributeNamesRequest,
source: CoOccurringAttrsSource,
) -> TraceItemAttributeNamesRequest.OrderBy.Column.ValueType:
"""The ordering the query will apply, which may differ from the one requested.

Only v2 records ``last_seen``, so a recency request that lands on v1 degrades to frequency
ordering rather than failing: both rank "attributes worth showing first", so an
autocomplete caller still gets a useful answer. It stays detectable because ``last_seen``
is then absent from the response, and via a metric.

Everything downstream keys off this rather than ``request.order_by.column``, so the
ClickHouse ORDER BY and the Python re-sort cannot disagree about which ordering was used.
"""
column = request.order_by.column
if (
column == TraceItemAttributeNamesRequest.OrderBy.Column.COLUMN_LAST_SEEN
and not source.has_last_seen
):
return TraceItemAttributeNamesRequest.OrderBy.Column.COLUMN_COUNT
return column


def _aggregates_attributes(
order_by_column: TraceItemAttributeNamesRequest.OrderBy.Column.ValueType,
) -> bool:
Expand Down Expand Up @@ -254,7 +233,6 @@ def _add_substring_match_optimization(

def get_co_occurring_attributes(
request: TraceItemAttributeNamesRequest,
source: CoOccurringAttrsSource | None = None,
) -> SnubaRequest:
"""Constructs the clickhouse query for co-occurring attributes:

Expand All @@ -263,7 +241,7 @@ def get_co_occurring_attributes(

-- Default ordering (order_by unset or COLUMN_NAME): distinct keys by name
SELECT distinct(arrayJoin(arrayFilter(attr -> ((NOT has(['test_tag_1_0'], attr.2)) AND startsWith(attr.2, 'test_')), arrayMap(x -> ('TYPE_STRING', x), attributes_string)))) AS attr_key
FROM eap_item_co_occurring_attrs_1_local
FROM eap_item_co_occurring_attrs_2_local
WHERE (item_type = 1) AND (project_id IN [1]) AND (organization_id = 1) AND (date < toDateTime(toDate('2025-03-17', 'Universal'))) AND (date >= toDateTime(toDate('2025-03-10', 'Universal')))

-- This is a faster way of looking up whether all attributes co-exist, it uses an array of hashes. This avoids string equality comparisons
Comment on lines 241 to 247

@sentry sentry Bot Aug 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: Queries for co-occurring attributes before 2026-07-29 will return incomplete data because the fallback to the v1 table was removed, and the v2 table was not backfilled.
Severity: CRITICAL

Suggested Fix

Reinstate the date-gated fallback logic. Queries with a start date before the co_occurring_attrs_v2_start_timestamp should be directed to the v1 table to ensure historical data remains accessible. Alternatively, perform a one-time backfill of the v2 table with data from the v1 table.

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/web/rpc/v1/endpoint_trace_item_attribute_names.py#L241-L247

Potential issue: The code removes the fallback logic that queries an older `v1` table
for historical data. It now exclusively queries a `v2` table
(`eap_item_co_occurring_attrs_v2`) which was created on 2026-07-29 and was not
backfilled. As a result, any query for co-occurring attribute names with a date range
before this creation date will target a table that has no data for that period. This
will cause the endpoint to silently return empty or incomplete results for historical
queries, even though the data exists in the now-inaccessible `v1` table.

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

Expand All @@ -279,9 +257,8 @@ def get_co_occurring_attributes(
-- max(last_seen) AS last_seen
-- ... GROUP BY attr_key ORDER BY <count|last_seen> DESC, attr_key ASC

**Storage:** the roll-up this reads and the parts of the query shape that differ between
the two (per-type key arrays, the aggregates) come from the `CoOccurringAttrsSource`
returned by `resolvers.R_eap_items.co_occurring_attrs.for_request`.
**Storage:** always ``eap_item_co_occurring_attrs_v2``. Per-type key arrays and
aggregates come from ``CoOccurringAttrsV2``.

**Explanation:**

Expand Down Expand Up @@ -318,12 +295,8 @@ def get_co_occurring_attributes(
- The attribute keys are deduplicated, resulting in less data to scan (~95% row reduction rate)
- there is a bloom filter index on all key values
"""
# Callers that need the source themselves must pass the one they resolved: resolving reads
# runtime options, and an option flipping between two reads would build the query for one
# storage while the response is processed as if it were the other.
if source is None:
source = co_occurring_attrs.for_request(request)
order_by_column = _effective_order_by_column(request, source)
source = V2
order_by_column = request.order_by.column

# get all attribute keys from the filter
collector = AttributeKeyCollector()
Expand Down Expand Up @@ -497,9 +470,8 @@ def convert_co_occurring_results_to_attributes(
) -> list[TraceItemAttributeNamesResponse.Attribute]:
"""Build the response attributes, re-sorting to match the ClickHouse ORDER BY.

``order_by_column`` must be the value the query was built with (see
``_effective_order_by_column``), or the merge below re-sorts into a different order than
ClickHouse used. Defaults to the requested column.
``order_by_column`` must match the query's ORDER BY, or the merge re-sorts into a
different order than ClickHouse used. Defaults to the requested column.
"""
if order_by_column is None:
order_by_column = request.order_by.column
Expand Down Expand Up @@ -607,24 +579,8 @@ def _build_response(
)

def _execute(self, in_msg: TraceItemAttributeNamesRequest) -> TraceItemAttributeNamesResponse:
# Resolved once and shared, so the query and the response re-sort cannot disagree.
source = co_occurring_attrs.for_request(in_msg)
order_by_column = _effective_order_by_column(in_msg, source)
if order_by_column != in_msg.order_by.column:
# Keeps an otherwise silent downgrade visible during the v2 rollout.
self.metrics.increment(
"attribute_names_order_by_degraded",
1,
tags={
"requested": TraceItemAttributeNamesRequest.OrderBy.Column.Name(
in_msg.order_by.column
),
"applied": TraceItemAttributeNamesRequest.OrderBy.Column.Name(order_by_column),
"storage": source.storage_key.value,
},
)

snuba_request = get_co_occurring_attributes(in_msg, source)
order_by_column = in_msg.order_by.column
snuba_request = get_co_occurring_attributes(in_msg)
res = run_query(
dataset=PluggableDataset(name="eap", all_entities=[]),
request=snuba_request,
Expand Down
17 changes: 3 additions & 14 deletions snuba/web/rpc/v1/resolvers/R_eap_items/co_occurring_attrs/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,16 @@
"""Pre-aggregated roll-ups of ``eap_items`` that ``TraceItemAttributeNames`` reads from.

The two roll-ups have different schemas, so choosing one also decides part of the query's
shape. ``CoOccurringAttrsSource`` is that choice, one implementation per table:
``TraceItemAttributeNames`` reads ``eap_item_co_occurring_attrs_v2`` exclusively.
``CoOccurringAttrsV1`` remains as the query shape for the original table, which is
still populated but no longer served.

v1.py eap_item_co_occurring_attrs ReplacingMergeTree, scalar key arrays only
v2.py eap_item_co_occurring_attrs_v2 SummingMergeTree, one key array per type

``for_request`` picks the source for a request.
"""

from snuba.web.rpc.v1.resolvers.R_eap_items.co_occurring_attrs.base import (
CoOccurringAttrsSource,
)
from snuba.web.rpc.v1.resolvers.R_eap_items.co_occurring_attrs.selection import (
CO_OCCURRING_ATTRS_V2_OPTION,
CO_OCCURRING_ATTRS_V2_START_TIMESTAMP_DEFAULT,
CO_OCCURRING_ATTRS_V2_START_TIMESTAMP_OPTION,
for_request,
)
from snuba.web.rpc.v1.resolvers.R_eap_items.co_occurring_attrs.v1 import (
CO_OCCURRING_ATTRS_STORAGE_KEY,
V1,
Expand All @@ -31,14 +24,10 @@

__all__ = [
"CO_OCCURRING_ATTRS_STORAGE_KEY",
"CO_OCCURRING_ATTRS_V2_OPTION",
"CO_OCCURRING_ATTRS_V2_START_TIMESTAMP_DEFAULT",
"CO_OCCURRING_ATTRS_V2_START_TIMESTAMP_OPTION",
"CO_OCCURRING_ATTRS_V2_STORAGE_KEY",
"V1",
"V2",
"CoOccurringAttrsSource",
"CoOccurringAttrsV1",
"CoOccurringAttrsV2",
"for_request",
]
55 changes: 0 additions & 55 deletions snuba/web/rpc/v1/resolvers/R_eap_items/co_occurring_attrs/selection.py
View file
Open in desktop

This file was deleted.

86 changes: 0 additions & 86 deletions tests/web/rpc/v1/resolvers/R_eap_items/co_occurring_attrs/test_selection.py
View file
Open in desktop

This file was deleted.

Loading
Loading