snuba icon indicating copy to clipboard operation
snuba copied to clipboard

feat(deletes): support item attribute conditions in API and consumer

Open onewland opened this issue 2 months ago • 2 comments

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 DeleteQueryMessage has two new fields: attribute_conditions and attribute_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

onewland avatar Nov 14 '25 15:11 onewland

: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_enabled
Stack 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.

codecov[bot] avatar Nov 14 '25 16:11 codecov[bot]

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)

onewland avatar Nov 25 '25 19:11 onewland

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

MeredithAnya avatar Dec 02 '25 18:12 MeredithAnya

working on a different approach in https://github.com/getsentry/snuba/pull/7570/files, will report back when that's ready for review

onewland avatar Dec 04 '25 00:12 onewland