Skip to content

fix(eap): Recognize normalize deprecations in attribute mapping#116509

Merged
mjq merged 2 commits into
masterfrom
mjq/fix-ci-from-depreactions-change
May 29, 2026
Merged

fix(eap): Recognize normalize deprecations in attribute mapping#116509
mjq merged 2 commits into
masterfrom
mjq/fix-ci-from-depreactions-change

Conversation

@mjq
Copy link
Copy Markdown
Member

@mjq mjq commented May 29, 2026

Okay, here goes.

#116399 broke 3 tests in tests/sentry/api/endpoints/test_project_trace_item_details.py, breaking CI:

  1. test_convert_rpc_attribute_to_json_serializes_known_string_array_without_array_flag
  2. test_convert_rpc_attribute_to_json_exposes_array_with_array_flag
  3. TestReplacementAttributeFiltering::test_replacement_array_shown_when_no_deprecated_source

These all test the very fiddly logic in ProjectTraceItemDetailsEndpoint around arrays. Dig through enough indirection and you'll find that said logic depends on attribute info (especially deprecation metadata) being included in the relevant <type>_ATTRIBUTE_DEFINITIONS list (e.g. SPAN_ATTRIBUTE_DEFINITIONS for spans).

When #116399 changed our data source for deprecations, it picked up a convention update that wasn't reflected in the old static deprecation JSON, changing 8 gen_ai.* spans from backfill deprecations to normalize deprecations. However, only backfill deprecations get included in SPAN_ATTRIBUTE_DEFINITIONS lists - so the definitions disappeared, breaking the logic the tests depended on.

Frankly: all this code needs a major rethink. The <type>_ATTRIBUTE_DEFINITIONS lists are now heavily overloaded, acting as public search aliases, type definitions (often duplicating conventions), conventional attributes themselves, and their deprecation aliases. On top of that, the whole backfill vs normalize thing is poorly defined between conventions, Relay and Sentry. This is going to be a constant source of bugs until this is untangled.

But, this has to get fixed now and can't wait for all that heavy lifting. So I made the change that seems least likely to have a blast radius: processing normalize deprecated attributes the same way we do backfill ones. This only affects the 8 gen_ai.* attributes plus 2 more (db.operation and db.statement), and fixes the failing tests.

Fixes BROWSE-536.


🤖 Claude Code (Opus 4.6) used heavily to understand WTF is going on. I wrote the code change, Claude wrote the tests, I reviewed. All words mine.

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 29, 2026
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 29, 2026

BROWSE-536

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 37ac7f9. Configure here.

@mjq mjq marked this pull request as ready for review May 29, 2026 19:27
@mjq mjq requested review from a team as code owners May 29, 2026 19:27
Copy link
Copy Markdown
Member

@mchen-sentry mchen-sentry left a comment

Choose a reason for hiding this comment

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

Should revisit as mentioned in pr desc, let's unblock

assert len(keys) == 4
assert "transaction.op" in keys
assert "span.op" in keys
# These two are unrelated, but happen to match.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

seems fragile?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Definitely, will fast follow with a less fragile version and tag whoever wrote/maintains this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@mjq mjq merged commit d14ccb5 into master May 29, 2026
63 checks passed
@mjq mjq deleted the mjq/fix-ci-from-depreactions-change branch May 29, 2026 20:59
mjq added a commit that referenced this pull request Jun 1, 2026
`OrganizationTraceItemAttributesEndpointSpansTest::test_aliased_attribute`
in
`tests/snuba/api/endpoints/test_organization_trace_item_attributes.py`
revealed itself to be fragile in #116509, when adding new attributes
suddenly broke its assertions around `op` aliases:


https://github.com/getsentry/sentry/blob/61bc60a3e716563ca3bdc1dc0cd0f17114666408/tests/snuba/api/endpoints/test_organization_trace_item_attributes.py#L904-L906

Update the test to be less fragile, focusing on the presence or absence
of `op` aliases rather than complete `keys` results.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants