feat(rpc): Allow filtering when we can't use VirtualColumnContexts #83663
Conversation
wmak
commented
Jan 17, 2025
wmak
commented
- VirtualColumnContexts aren't available on timeseries, so if we filter by a vcc field (project, device.class) we need custom logic
- From a discussion in slack going to drop device.module support, will do that in a follow up PR
github-actions
bot
added
the
Scope: Backend
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #83663 +/- ##
==========================================
+ Coverage 87.23% 87.64% +0.41%
==========================================
Files 9530 9530
Lines 542683 545711 +3028
Branches 21259 21259
==========================================
+ Hits 473390 478296 +4906
+ Misses 68926 67048 -1878
Partials 367 367 |
src/sentry/search/eap/spans.py
Outdated
| # Convert the resolved_column to the underlying column | ||
| if term.key.name in constants.PROJECT_FIELDS: | ||
| resolved_column, _ = self.resolve_attribute("project.id") | ||
| if isinstance(final_raw_value, list): | ||
| final_raw_value = [int(val) for val in final_raw_value] | ||
| else: | ||
| final_raw_value = int(final_raw_value) | ||
| elif term.key.name == "device.class": | ||
| resolved_column, _ = self.resolve_attribute("sentry.device.class") |
There was a problem hiding this comment.
This will conflict with #83714 where you're trying to make the search resolver applicable to different datasets.
There was a problem hiding this comment.
😬 ugh right... I'll move this logic to the VCC definitions instead then and merge 83714 first
| if context.default_value and context.default_value == raw_iterable: | ||
| # Avoiding this for now, while this could work with the Unknown:"" mapping | ||
| # But that won't work once we use the VirtualColumnContext.default_value | ||
| raise InvalidSearchQuery( | ||
| f"Using {raw_iterable} in an IN filter is not currently supported" | ||
| ) |
There was a problem hiding this comment.
Why is the default value special when used in an IN filter?
There was a problem hiding this comment.
good catch, i'll disable in both sides for now and re-enable when i fix
github-actions
bot
added
the
Scope: Frontend
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
- VirtualColumnContexts aren't available on timeseries, so if we filter by a vcc field (project, device.class) we need custom logic - From a discussion in slack going to drop device.module support, will do that in a follow up PR
- TODO: move all of this into VCC definition so that we can be dataset agnostic within the SearchResolver
wmak
force-pushed
the
wmak/feat/filter-without-vcc
branch
from
c2f2f7c to
995a413
Compare
…83663) - VirtualColumnContexts aren't available on timeseries, so if we filter by a vcc field (project, device.class) we need custom logic - From a discussion in slack going to drop device.module support, will do that in a follow up PR
