diff --git a/snuba/admin/views.py b/snuba/admin/views.py index 49ba5f29b96..09bdbe46c81 100644 --- a/snuba/admin/views.py +++ b/snuba/admin/views.py @@ -70,6 +70,11 @@ from snuba.datasets.factory import InvalidDatasetError, get_enabled_dataset_names from snuba.datasets.storages.factory import get_storage, get_writable_storage from snuba.datasets.storages.storage_key import StorageKey +from snuba.lw_deletions.delete_query import ( + DeletesNotEnabledError, + delete_from_storage, + deletes_are_enabled, +) from snuba.manual_jobs.runner import ( list_job_specs, list_job_specs_with_status, @@ -90,11 +95,6 @@ from snuba.state.explain_meta import explain_cleanup, get_explain_meta from snuba.utils.metrics.timer import Timer from snuba.utils.registered_class import InvalidConfigKeyError -from snuba.web.delete_query import ( - DeletesNotEnabledError, - delete_from_storage, - deletes_are_enabled, -) from snuba.web.rpc import RPCEndpoint, list_all_endpoint_names, run_rpc_handler from snuba.web.rpc.storage_routing.routing_strategies.storage_routing import ( BaseRoutingStrategy, diff --git a/snuba/cli/lw_deletions_consumer.py b/snuba/cli/lw_deletions_consumer.py index 228c5198187..080b1b9abcb 100644 --- a/snuba/cli/lw_deletions_consumer.py +++ b/snuba/cli/lw_deletions_consumer.py @@ -19,11 +19,11 @@ from snuba.datasets.storages.factory import get_writable_storage from snuba.datasets.storages.storage_key import StorageKey from snuba.environment import setup_logging, setup_sentry +from snuba.lw_deletions.bulk_delete_query import STORAGE_TOPIC from snuba.lw_deletions.formatters import STORAGE_FORMATTER from snuba.lw_deletions.strategy import LWDeletionsConsumerStrategyFactory from snuba.utils.metrics.wrapper import MetricsWrapper from snuba.utils.streams.metrics_adapter import StreamMetricsAdapter -from snuba.web.bulk_delete_query import STORAGE_TOPIC # A longer batch time for deletes is reasonable # since we want fewer mutations @@ -126,9 +126,7 @@ def handler(signum: int, frame: Any) -> None: "consumer_group": consumer_group, "storage": storage, } - metrics = MetricsWrapper( - environment.metrics, "lw_deletions_consumer", tags=metrics_tags - ) + metrics = MetricsWrapper(environment.metrics, "lw_deletions_consumer", tags=metrics_tags) configure_metrics(StreamMetricsAdapter(metrics), force=True) consumer_config = resolve_consumer_config( storage_names=[storage], diff --git a/snuba/web/bulk_delete_query.py b/snuba/lw_deletions/bulk_delete_query.py similarity index 97% rename from snuba/web/bulk_delete_query.py rename to snuba/lw_deletions/bulk_delete_query.py index c1618c97404..b1847a491ca 100644 --- a/snuba/web/bulk_delete_query.py +++ b/snuba/lw_deletions/bulk_delete_query.py @@ -15,6 +15,13 @@ from snuba.datasets.deletion_settings import DeletionSettings, get_trace_item_type_name from snuba.datasets.storage import WritableTableStorage from snuba.datasets.storages.storage_key import StorageKey +from snuba.lw_deletions.delete_query import ( + DeletesNotEnabledError, + _construct_condition, + _enforce_max_rows, + _get_attribution_info, + deletes_are_enabled, +) from snuba.lw_deletions.types import AttributeConditions, ConditionsBag, ConditionsType from snuba.query.conditions import combine_or_conditions from snuba.query.data_source.simple import Table @@ -28,13 +35,6 @@ from snuba.utils.schemas import ColumnValidator, InvalidColumnType from snuba.utils.streams.configuration_builder import build_kafka_producer_configuration from snuba.utils.streams.topics import Topic -from snuba.web.delete_query import ( - DeletesNotEnabledError, - _construct_condition, - _enforce_max_rows, - _get_attribution_info, - deletes_are_enabled, -) metrics = MetricsWrapper(environment.metrics, "snuba.delete") logger = logging.getLogger(__name__) @@ -226,7 +226,10 @@ def delete_from_storage( _validate_attribute_conditions(attribute_conditions, delete_settings) if not get_int_config("permit_delete_by_attribute", default=0): - metrics.increment("delete_query.delete_ignored") + logger.error( + "valid attribute_conditions passed to delete_from_storage, but delete will be ignored " + "as functionality is not yet launched (permit_delete_by_attribute=0)" + ) return {} attr_info = _get_attribution_info(attribution_info) diff --git a/snuba/web/delete_query.py b/snuba/lw_deletions/delete_query.py similarity index 100% rename from snuba/web/delete_query.py rename to snuba/lw_deletions/delete_query.py diff --git a/snuba/lw_deletions/formatters.py b/snuba/lw_deletions/formatters.py index ab14f4ecdea..4946580f661 100644 --- a/snuba/lw_deletions/formatters.py +++ b/snuba/lw_deletions/formatters.py @@ -15,8 +15,11 @@ from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey from snuba.datasets.storages.storage_key import StorageKey +from snuba.lw_deletions.bulk_delete_query import ( + DeleteQueryMessage, + WireAttributeCondition, +) from snuba.lw_deletions.types import AttributeConditions, ConditionsBag -from snuba.web.bulk_delete_query import DeleteQueryMessage, WireAttributeCondition class Formatter(ABC): diff --git a/snuba/lw_deletions/strategy.py b/snuba/lw_deletions/strategy.py index 9426072f274..9b4a46b8398 100644 --- a/snuba/lw_deletions/strategy.py +++ b/snuba/lw_deletions/strategy.py @@ -19,6 +19,15 @@ from snuba.datasets.storage import WritableTableStorage from snuba.datasets.storages.storage_key import StorageKey from snuba.lw_deletions.batching import BatchStepCustom, ValuesBatch +from snuba.lw_deletions.bulk_delete_query import ( + construct_or_conditions, + construct_query, +) +from snuba.lw_deletions.delete_query import ( + TooManyOngoingMutationsError, + _execute_query, + _num_ongoing_mutations, +) from snuba.lw_deletions.formatters import Formatter from snuba.lw_deletions.types import ConditionsBag from snuba.query.allocation_policies import AllocationPolicyViolations @@ -26,13 +35,7 @@ from snuba.state import get_int_config, get_str_config from snuba.utils.metrics import MetricsBackend from snuba.web import QueryException -from snuba.web.bulk_delete_query import construct_or_conditions, construct_query from snuba.web.constants import LW_DELETE_NON_RETRYABLE_CLICKHOUSE_ERROR_CODES -from snuba.web.delete_query import ( - TooManyOngoingMutationsError, - _execute_query, - _num_ongoing_mutations, -) TPayload = TypeVar("TPayload") diff --git a/snuba/web/rpc/v1/endpoint_delete_trace_items.py b/snuba/web/rpc/v1/endpoint_delete_trace_items.py index 5f1958fca5f..7cd96afb950 100644 --- a/snuba/web/rpc/v1/endpoint_delete_trace_items.py +++ b/snuba/web/rpc/v1/endpoint_delete_trace_items.py @@ -11,8 +11,8 @@ from snuba.attribution.appid import AppID from snuba.datasets.storages.factory import get_writable_storage from snuba.datasets.storages.storage_key import StorageKey +from snuba.lw_deletions.bulk_delete_query import delete_from_storage from snuba.lw_deletions.types import AttributeConditions -from snuba.web.bulk_delete_query import delete_from_storage from snuba.web.rpc import RPCEndpoint from snuba.web.rpc.common.exceptions import BadSnubaRPCRequestException diff --git a/snuba/web/views.py b/snuba/web/views.py index b5b28f6e6af..85ca6cdbc19 100644 --- a/snuba/web/views.py +++ b/snuba/web/views.py @@ -52,6 +52,13 @@ from snuba.datasets.factory import InvalidDatasetError, get_dataset_name from snuba.datasets.schemas.tables import TableSchema from snuba.datasets.storage import StorageNotAvailable, WritableTableStorage +from snuba.lw_deletions.bulk_delete_query import ( + delete_from_storage as bulk_delete_from_storage, +) +from snuba.lw_deletions.delete_query import ( + DeletesNotEnabledError, + TooManyOngoingMutationsError, +) from snuba.query.allocation_policies import AllocationPolicyViolations from snuba.query.exceptions import InvalidQueryException, QueryPlanException from snuba.query.query_settings import HTTPQuerySettings @@ -72,10 +79,8 @@ from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.util import with_span from snuba.web import QueryException, QueryTooLongException -from snuba.web.bulk_delete_query import delete_from_storage as bulk_delete_from_storage from snuba.web.constants import get_http_status_for_clickhouse_error from snuba.web.converters import DatasetConverter, EntityConverter, StorageConverter -from snuba.web.delete_query import DeletesNotEnabledError, TooManyOngoingMutationsError from snuba.web.query import parse_and_run_query from snuba.web.rpc import run_rpc_handler from snuba.writer import BatchWriterEncoderWrapper, WriterTableRow diff --git a/tests/web/test_bulk_delete_query.py b/tests/lw_deletions/test_bulk_delete_query.py similarity index 92% rename from tests/web/test_bulk_delete_query.py rename to tests/lw_deletions/test_bulk_delete_query.py index 0a07b0f75ab..f4d5132c37a 100644 --- a/tests/web/test_bulk_delete_query.py +++ b/tests/lw_deletions/test_bulk_delete_query.py @@ -13,14 +13,14 @@ from snuba import settings from snuba.datasets.storages.factory import get_writable_storage from snuba.datasets.storages.storage_key import StorageKey +from snuba.lw_deletions.bulk_delete_query import delete_from_storage +from snuba.lw_deletions.delete_query import DeletesNotEnabledError from snuba.lw_deletions.types import AttributeConditions from snuba.query.exceptions import InvalidQueryException from snuba.state import set_config from snuba.utils.manage_topics import create_topics from snuba.utils.streams.configuration_builder import get_default_kafka_configuration from snuba.utils.streams.topics import Topic -from snuba.web.bulk_delete_query import delete_from_storage -from snuba.web.delete_query import DeletesNotEnabledError # TraceItemType values from sentry_protos TRACE_ITEM_TYPE_SPAN = 1 @@ -48,7 +48,7 @@ def get_attribution_info(tenant_ids: Optional[Mapping[str, int | str]] = None) - } -@patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10) +@patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10) def test_delete_success(mock_enforce_max_row: Mock) -> None: admin_client = AdminClient(get_default_kafka_configuration()) create_topics(admin_client, [Topic.LW_DELETIONS_GENERIC_EVENTS]) @@ -107,8 +107,8 @@ def test_deletes_not_enabled_runtime_config() -> None: @pytest.mark.redis_db -@patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10) -@patch("snuba.web.bulk_delete_query.produce_delete_query") +@patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10) +@patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query") def test_deletes_killswitch(mock_produce_query: Mock, mock_enforce_rows: Mock) -> None: storage = get_writable_storage(StorageKey("search_issues")) conditions = {"project_id": [1], "group_id": [1, 2, 3, 4]} @@ -177,8 +177,8 @@ def test_attribute_conditions_valid_occurrence() -> None: attr_info = get_attribution_info() # Mock out _enforce_max_rows to avoid needing actual data - with patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10): - with patch("snuba.web.bulk_delete_query.produce_delete_query") as mock_produce: + with patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10): + with patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query") as mock_produce: # Should not raise an exception, but should return empty dict since # functionality is not yet launched (permit_delete_by_attribute=0 by default) result = delete_from_storage(storage, conditions, attr_info, attribute_conditions) @@ -225,8 +225,8 @@ def test_attribute_conditions_missing_item_type() -> None: # Since item_type is now in AttributeConditions, we need to test a different scenario # The validation now should pass, but we need to ensure item_type is also in conditions - with patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10): - with patch("snuba.web.bulk_delete_query.produce_delete_query"): + with patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10): + with patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query"): # This should now succeed since we're no longer checking conditions dict delete_from_storage(storage, conditions, attr_info, attribute_conditions) @@ -268,8 +268,8 @@ def test_attribute_conditions_feature_flag_enabled() -> None: try: # Mock out _enforce_max_rows to avoid needing actual data - with patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10): - with patch("snuba.web.bulk_delete_query.produce_delete_query") as mock_produce: + with patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10): + with patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query") as mock_produce: # Should process normally and produce a message result = delete_from_storage(storage, conditions, attr_info, attribute_conditions) diff --git a/tests/web/test_delete_query.py b/tests/lw_deletions/test_delete_query.py similarity index 97% rename from tests/web/test_delete_query.py rename to tests/lw_deletions/test_delete_query.py index ba6d10a6a9c..6d164896abd 100644 --- a/tests/web/test_delete_query.py +++ b/tests/lw_deletions/test_delete_query.py @@ -11,6 +11,7 @@ from snuba.configs.configuration import Configuration, ResourceIdentifier from snuba.datasets.storages.factory import get_writable_storage from snuba.datasets.storages.storage_key import StorageKey +from snuba.lw_deletions.delete_query import _execute_query from snuba.query.allocation_policies import ( MAX_THRESHOLD, NO_SUGGESTION, @@ -24,7 +25,6 @@ from snuba.query.dsl import and_cond, column, equals, literal from snuba.query.query_settings import HTTPQuerySettings from snuba.web import QueryException -from snuba.web.delete_query import _execute_query @pytest.mark.clickhouse_db @@ -113,7 +113,7 @@ def _update_quota_balance( return with mock.patch( - "snuba.web.delete_query._get_delete_allocation_policies", + "snuba.lw_deletions.delete_query._get_delete_allocation_policies", return_value=[ RejectPolicy(ResourceIdentifier(StorageKey("doesntmatter")), ["a", "b", "c"], {}) ], diff --git a/tests/lw_deletions/test_formatters.py b/tests/lw_deletions/test_formatters.py index aae5ae5089a..20e5774b259 100644 --- a/tests/lw_deletions/test_formatters.py +++ b/tests/lw_deletions/test_formatters.py @@ -3,13 +3,16 @@ import pytest from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey +from snuba.lw_deletions.bulk_delete_query import ( + DeleteQueryMessage, + WireAttributeCondition, +) from snuba.lw_deletions.formatters import ( EAPItemsFormatter, Formatter, SearchIssuesFormatter, ) from snuba.lw_deletions.types import ConditionsBag, ConditionsType -from snuba.web.bulk_delete_query import DeleteQueryMessage, WireAttributeCondition def create_delete_query_message( diff --git a/tests/lw_deletions/test_lw_deletions.py b/tests/lw_deletions/test_lw_deletions.py index f87661e4a6d..e107446f8b6 100644 --- a/tests/lw_deletions/test_lw_deletions.py +++ b/tests/lw_deletions/test_lw_deletions.py @@ -13,11 +13,11 @@ from snuba.datasets.storages.factory import get_writable_storage from snuba.datasets.storages.storage_key import StorageKey from snuba.lw_deletions.batching import BatchStepCustom +from snuba.lw_deletions.bulk_delete_query import DeleteQueryMessage from snuba.lw_deletions.formatters import SearchIssuesFormatter from snuba.lw_deletions.strategy import FormatQuery, increment_by from snuba.lw_deletions.types import ConditionsType from snuba.utils.streams.topics import Topic as SnubaTopic -from snuba.web.bulk_delete_query import DeleteQueryMessage ROWS_CONDITIONS = { 5: {"project_id": [1], "group_id": [1, 2, 3, 4]}, diff --git a/tests/web/test_max_rows_enforcer.py b/tests/lw_deletions/test_max_rows_enforcer.py similarity index 98% rename from tests/web/test_max_rows_enforcer.py rename to tests/lw_deletions/test_max_rows_enforcer.py index afa590644b5..d33cc505522 100644 --- a/tests/web/test_max_rows_enforcer.py +++ b/tests/lw_deletions/test_max_rows_enforcer.py @@ -12,11 +12,11 @@ from snuba.datasets.entities.entity_key import EntityKey from snuba.datasets.entities.factory import get_entity from snuba.datasets.storages.storage_key import StorageKey +from snuba.lw_deletions.delete_query import _enforce_max_rows from snuba.query.data_source.simple import Table from snuba.query.dsl import and_cond, column, equals, literal from snuba.query.exceptions import TooManyDeleteRowsException from snuba.state import set_config -from snuba.web.delete_query import _enforce_max_rows from tests.base import BaseApiTest from tests.datasets.configuration.utils import ConfigurationTest from tests.helpers import write_unprocessed_events diff --git a/tests/test_search_issues_api.py b/tests/test_search_issues_api.py index 804afa5de24..eab13f51426 100644 --- a/tests/test_search_issues_api.py +++ b/tests/test_search_issues_api.py @@ -104,7 +104,7 @@ def delete_query( headers={"referer": "test"}, ) - @patch("snuba.web.bulk_delete_query.produce_delete_query") + @patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query") def test_simple_delete(self, mock_produce_delete: Mock) -> None: set_config("read_through_cache.short_circuit", 1) now = datetime.now().replace(minute=0, second=0, microsecond=0) diff --git a/tests/web/rpc/v1/test_endpoint_delete_trace_items.py b/tests/web/rpc/v1/test_endpoint_delete_trace_items.py index 437775a9870..6ff7070e5ac 100644 --- a/tests/web/rpc/v1/test_endpoint_delete_trace_items.py +++ b/tests/web/rpc/v1/test_endpoint_delete_trace_items.py @@ -119,7 +119,7 @@ def test_valid_trace_id_returns_success_response(self, setup_teardown: Any) -> N assert MessageToDict(response) == MessageToDict(expected_response) - @patch("snuba.web.bulk_delete_query.produce_delete_query") + @patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query") def test_valid_trace_id_produces_bulk_delete_message( self, produce_delete_query_mock: Mock, setup_teardown: Any ) -> None: @@ -183,8 +183,8 @@ def test_filters_with_equals_operation_accepted(self) -> None: ], ) - with patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10): - with patch("snuba.web.bulk_delete_query.produce_delete_query") as mock_produce: + with patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10): + with patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query") as mock_produce: EndpointDeleteTraceItems().execute(message) # Verify produce_delete_query was called with attribute_conditions @@ -221,8 +221,8 @@ def test_filters_with_in_operation_accepted(self) -> None: ], ) - with patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10): - with patch("snuba.web.bulk_delete_query.produce_delete_query"): + with patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10): + with patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query"): assert isinstance( EndpointDeleteTraceItems().execute(message), DeleteTraceItemsResponse )