Skip to content

fix(rpc): honor deprecated attribute keys in trace item attribute values#7853

Merged
xurui-c merged 4 commits intomasterfrom
rachel/attributevalues
Apr 1, 2026
Merged

fix(rpc): honor deprecated attribute keys in trace item attribute values#7853
xurui-c merged 4 commits intomasterfrom
rachel/attributevalues

Conversation

@xurui-c
Copy link
Copy Markdown
Member

@xurui-c xurui-c commented Mar 30, 2026

https://linear.app/getsentry/issue/EAP-459/attributevaluesrequest-returns-empty-results-for-attributes-with

Fix the Trace Item Attribute Values RPC when clients request the canonical attribute name but data is stored only under a deprecated or alias key (e.g. db.system.name vs db.system).

attribute_key_to_expression already coalesces those names via ATTRIBUTES_TO_COALESCE, but the endpoint only checked has(attributes_string, <request key>), so inner rows that held values exclusively on the legacy key never matched. We now OR existence checks across the canonical name and any coalesce targets, and run add_existence_check_to_subscriptable_references on the inner query so behavior stays aligned with other EAP RPC paths.

Adds an integration test that writes db.system and requests db.system.name, asserting the returned distinct values.

Refs:

@xurui-c xurui-c marked this pull request as ready for review March 30, 2026 21:54
@xurui-c xurui-c requested review from a team as code owners March 30, 2026 21:54
Copilot AI review requested due to automatic review settings March 30, 2026 21:54
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes the Trace Item Attribute Values v1 RPC to return values when clients request a canonical attribute name but events only contain the deprecated/alias key, aligning the endpoint’s filtering behavior with attribute_key_to_expression coalescing.

Changes:

  • Update the attribute existence filter to OR across the requested key and any coalesce/deprecated targets.
  • Apply add_existence_check_to_subscriptable_references to the inner query to keep missing-key behavior consistent with other EAP RPC queries.
  • Add an integration test that writes values under db.system and queries db.system.name.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
snuba/web/rpc/v1/trace_item_attribute_values.py Extends attribute existence checks to cover coalesced keys and adds subscriptable existence wrapping to the inner query.
tests/web/rpc/v1/test_trace_item_attribute_values_v1.py Adds coverage for canonical-vs-deprecated attribute key lookups returning distinct values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.
@xurui-c xurui-c changed the title Rachel/attributevalues fix(rpc): honor deprecated attribute keys in trace item attribute values Mar 30, 2026
@xurui-c xurui-c merged commit 834c9a7 into master Apr 1, 2026
49 of 50 checks passed
@xurui-c xurui-c deleted the rachel/attributevalues branch April 1, 2026 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants