Skip to content

feat(eap): Make deprecated attribute coalescing bidirectional#7953

Merged
mjq merged 1 commit into
masterfrom
mjq/bidirectional-coalesce
May 21, 2026
Merged

feat(eap): Make deprecated attribute coalescing bidirectional#7953
mjq merged 1 commit into
masterfrom
mjq/bidirectional-coalesce

Conversation

@mjq
Copy link
Copy Markdown
Member

@mjq mjq commented May 20, 2026

Currently when EAP is queried via RPC for a particular attribute name, the underlying query will coalesce that attribute name with any of its deprecated aliases.

For example, given an attribute A that was deprecated in favour of B, that was then deprecated in favour of C, querying for C will in fact resolve to coalesce(C, B, A). This helps keeps queries working despite attribute deprecations.

(Actually, the ordering of B, A is currently non-deterministic, but this PR addresses that.)

However, this requires that the caller always use the latest name for an attribute - for example, a query for A would only search A, and a query for B would only search B. Only C triggers the coalescing. Our desired behaviour is that a search for any of A, B, or C triggers a coalesced query on all aliases.

There are two possible approaches to take:

  1. All RPC callers must use the sentry-conventions library to replace attribute names with their latest non-deprecated alias before calling EAP, or get inconsistent results.
  2. Snuba always performs coalescing, for deprecated as well as non-deprecated attributes.

Option 1 is challenging because of how Sentry's Alerts work, which store RPC calls. Even if these calls use the latest attribute at creation time, a later deprecation coming in would cause incorrect result until they were recreated, and we would need to create a mechanism to do so whenever Snuba's sentry-conventions library was updated. To avoid duplicating the deprecated attribute logic in every caller and dealing with this conventions/alerts coupling, I'm suggesting option 2, which this PR implements.

Note the subtleties around coalesce ordering:

  • The requested attribute is always first. This is the most consistent option for end users and gives callers a bit of control over behaviour for edge cases.
  • If the requested attribute is deprecated, then its non-deprecated alias is always second. We wouldn't want a deprecated alias taking precedence over the current, non-deprecated one.
  • Finally, all remaining deprecated aliases are listed in alphabetically sorted order. This ordering is arbitrary because we don't store metadata around the order in which fields are deprecated, but enforcing an ordering is nonetheless important so that we don't get non-deterministic results.

This PR also updates sentry-conventions to the latest version in order to get getsentry/sentry-conventions#391 (ensures that all deprecated attributes point to a non-deprecated replacement; ensures that _resolve_canonical only ever has to make 1 hop).


🤖: Claude Code (Opus 4.6) used to generate the code, with human editing + review. All words my own.

Currently when EAP is queried via RPC for a particular attribute name, the
underlying query will coalesce that attribute name with any of its deprecated
aliases.

For example, given an attribute `A` that was deprecated in favour of `B`, which
was then deprecated in favour of `C`, querying for `C` will in fact resolve to
`coalesce(C, B, A)`. This helps keeps queries working despite attribute deprecations.

However, this requires that the caller always use the latest name for an
attribute - for example, a query for `A` would only search `A`, and a query for
`B` would only search `B`. Only `C` triggers the coalescing. Our desired
behaviour is that a search for any of `A`, `B`, or `C` triggers a coalesced
query on all aliases.

There are two possible approaches to take:

1. All RPC callers must use the `sentry-conventions` library to replace attribute
names with their latest non-deprecated alias before calling EAP, or get
inconsistent results.

2. Snuba always performs coalescing, for deprecated as well as non-deprecated
attributes.

Option 1 is challenging because of how Sentry's Alerts work, which store RPC
calls. Even if these calls use the latest attribute at creation time, a later
deprecation coming in would cause incorrect result until they were recreated,
and we would need to create a mechanism to do so whenever Snuba's
`sentry-conventions` library was updated. To avoid duplicating the deprecated
attribute logic and dealing with this conventions/alerts coupling, this commit
implements option 2.

Note the subtleties around **coalesce ordering**:
- The requested attribute is always first. This is the most consistent option
  for end users and gives callers a bit of control over behaviour for edge
  cases.
- If the requested attribute is deprecated, then its non-deprecated alias is
  always second. We wouldn't want a deprecated alias taking precendence over
  the current, non-deprecated one.
- Finally, all remaining deprecated aliases are listed in alphabetically sorted
  order. This ordering is arbitrary because we don't store metadata around the
  order in which fields are deprecated, but enforcing an ordering is nonetheless
  important so that we don't get non-deterministic results.

---

🤖: Claude Code (Opus 4.6) used to generate the code, with human editing +
review. All words my own.
cursor[bot]

This comment was marked as resolved.

@mjq mjq marked this pull request as ready for review May 20, 2026 21:01
@mjq mjq requested a review from a team as a code owner May 20, 2026 21:01
@mjq mjq merged commit b4061ef into master May 21, 2026
52 checks passed
@mjq mjq deleted the mjq/bidirectional-coalesce branch May 21, 2026 18:01
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.

2 participants