feat(deletes): support item attribute conditions in API and consumer
This PR enables (gated behind a runtime-config) deletions of EAP items by attributes.
Changes:
- add support for attribute_conditions in the lightweight deletions consumer
- this means the consumer does resolution of attribute_conditions to "columns" internally
- changed the query creation to support true columns or column-like subscript expressions (e.g.
attributes_string_x['key_name'] - the wire format of
DeleteQueryMessagehas two new fields:attribute_conditionsandattribute_conditions_item_type
- fixed the unimplemented API validation of item types, and the respective allowed attributes
- actually passing attribute_conditions queries through is gated behind a new run-time config
permit_delete_by_attribute, defaulting to off
End-to-end tested locally, verification walk-through: https://www.notion.so/sentry/e2e-attribute-testing-2ab8b10e4b5d80cea5cfdc255cff5305
:x: 1 Tests Failed:
| Tests completed | Failed | Passed | Skipped |
|---|---|---|---|
| 2960 | 1 | 2959 | 16 |
View the top 1 failed test(s) by shortest run time
tests.web.test_bulk_delete_query::test_attribute_conditions_feature_flag_enabledStack Traces | 0.053s run time
Traceback (most recent call last): File "/.venv/lib/python3.11........./site-packages/_pytest/runner.py", line 341, in from_call result: TResult | None = func() ^^^^^^ File "/.venv/lib/python3.11........./site-packages/_pytest/runner.py", line 242, in <lambda> lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/.venv/lib/python3.11....../site-packages/pluggy/_hooks.py", line 513, in __call__ return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/.venv/lib/python3.11....../site-packages/pluggy/_manager.py", line 120, in _hookexec return self._inner_hookexec(hook_name, methods, kwargs, firstresult) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/.venv/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 182, in _multicall return outcome.get_result() ^^^^^^^^^^^^^^^^^^^^ File "/.venv/lib/python3.11.../site-packages/pluggy/_result.py", line 100, in get_result raise exc.with_traceback(exc.__traceback__) File "/.venv/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 167, in _multicall teardown.throw(outcome._exception) File "/.venv/lib/python3.11....../site-packages/_pytest/threadexception.py", line 92, in pytest_runtest_call yield from thread_exception_runtest_hook() File "/.venv/lib/python3.11....../site-packages/_pytest/threadexception.py", line 68, in thread_exception_runtest_hook yield File "/.venv/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 167, in _multicall teardown.throw(outcome._exception) File "/.venv/lib/python3.11....../site-packages/_pytest/unraisableexception.py", line 95, in pytest_runtest_call yield from unraisable_exception_runtest_hook() File "/.venv/lib/python3.11....../site-packages/_pytest/unraisableexception.py", line 70, in unraisable_exception_runtest_hook yield File "/.venv/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 167, in _multicall teardown.throw(outcome._exception) File "/.venv/lib/python3.11....../site-packages/_pytest/logging.py", line 846, in pytest_runtest_call yield from self._runtest_for(item, "call") File "/.venv/lib/python3.11....../site-packages/_pytest/logging.py", line 829, in _runtest_for yield File "/.venv/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 167, in _multicall teardown.throw(outcome._exception) File "/.venv/lib/python3.11.../site-packages/_pytest/capture.py", line 880, in pytest_runtest_call return (yield) ^^^^^ File "/.venv/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 167, in _multicall teardown.throw(outcome._exception) File "/.venv/lib/python3.11.../site-packages/_pytest/skipping.py", line 257, in pytest_runtest_call return (yield) ^^^^^ File "/.venv/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 103, in _multicall res = hook_impl.function(*args) ^^^^^^^^^^^^^^^^^^^^^^^^^ File "/.venv/lib/python3.11........./site-packages/_pytest/runner.py", line 174, in pytest_runtest_call item.runtest() File "/.venv/lib/python3.11....../site-packages/_pytest/python.py", line 1627, in runtest self.ihook.pytest_pyfunc_call(pyfuncitem=self) File "/.venv/lib/python3.11....../site-packages/pluggy/_hooks.py", line 513, in __call__ return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/.venv/lib/python3.11....../site-packages/pluggy/_manager.py", line 120, in _hookexec return self._inner_hookexec(hook_name, methods, kwargs, firstresult) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/.venv/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 139, in _multicall raise exception.with_traceback(exception.__traceback__) File "/.venv/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 103, in _multicall res = hook_impl.function(*args) ^^^^^^^^^^^^^^^^^^^^^^^^^ File "/.venv/lib/python3.11....../site-packages/_pytest/python.py", line 159, in pytest_pyfunc_call result = testfunction(**testargs) ^^^^^^^^^^^^^^^^^^^^^^^^ File ".../tests/web/test_bulk_delete_query.py", line 221, in test_attribute_conditions_feature_flag_enabled assert mock_produce.call_count == 1 AssertionError: assert 0 == 1 + where 0 = <MagicMock name='produce_delete_query' id='140033197111312'>.call_count
To view more test analytics, go to the Test Analytics Dashboard 📋 Got 3 mins? Take this short survey to help us improve Test Analytics.
Overall this seems good to me, but Im wondering what happens if a request has an attr value that doesn't match the type, like
{"group_id": ["hello"]}?Also, do we need to be updating https://github.com/getsentry/sentry-kafka-schemas/blob/main/schemas/snuba-lw-deletions-eap-items.v1.json ?
I'm not sure I care about that case. The API should return that 0 rows are expected to be deleted if the attribute doesn't exist.
I'd be more concerned with the inversion of that e.g. if we support "group_id" != "hello" (which would then match all the items in the project)
Overall this seems good to me, but Im wondering what happens if a request has an attr value that doesn't match the type, like
{"group_id": ["hello"]}? Also, do we need to be updating https://github.com/getsentry/sentry-kafka-schemas/blob/main/schemas/snuba-lw-deletions-eap-items.v1.json ?I'm not sure I care about that case. The API should return that 0 rows are expected to be deleted if the attribute doesn't exist.
I'd be more concerned with the inversion of that e.g. if we support "group_id" != "hello" (which would then match all the items in the project)
Gotcha, yeah I don't actually think we want to support using non-equative (not sure thats a word) conditions, cause of what you are saying
working on a different approach in https://github.com/getsentry/snuba/pull/7570/files, will report back when that's ready for review