diff --git a/pyproject.toml b/pyproject.toml index 5e089e788d7..bf2c0623ed6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,6 +31,7 @@ dependencies = [ "sentry-arroyo>=2.39.2", "sentry-conventions>=0.12.0", "sentry-kafka-schemas>=2.1.35", + "sentry-options>=1.1.1", "sentry-protos>=0.32.0", "sentry-redis-tools>=0.5.1", "sentry-relay>=0.9.25", diff --git a/rust_snuba/src/processors/generic_metrics.rs b/rust_snuba/src/processors/generic_metrics.rs index 8fcac903d2c..e2d675ea9da 100644 --- a/rust_snuba/src/processors/generic_metrics.rs +++ b/rust_snuba/src/processors/generic_metrics.rs @@ -1,14 +1,14 @@ use adler::Adler32; -use anyhow::{anyhow, Context, Error}; +use anyhow::{anyhow, Context}; use chrono::DateTime; use serde::{ de::value::{MapAccessDeserializer, SeqAccessDeserializer}, Deserialize, Deserializer, Serialize, }; +use sentry_options::options; use std::{collections::BTreeMap, marker::PhantomData, vec}; use crate::{ - runtime_config::get_str_config, types::{CogsData, InsertBatch, RowData}, KafkaMessageMetadata, ProcessorConfig, }; @@ -339,8 +339,8 @@ impl Parse for CountersRawRow { } } -fn should_use_killswitch(config: Result, Error>, use_case: &MessageUseCase) -> bool { - if let Some(killswitch) = config.ok().flatten() { +fn should_use_killswitch(config: Option, use_case: &MessageUseCase) -> bool { + if let Some(killswitch) = config { return killswitch.contains(use_case.use_case_id.as_str()); } @@ -355,7 +355,10 @@ where T: Parse + Serialize, { let payload_bytes = payload.payload().context("Expected payload")?; - let killswitch_config = get_str_config("generic_metrics_use_case_killswitch"); + let killswitch_config = options("snuba") + .ok() + .and_then(|o| o.get("generic_metrics_use_case_killswitch").ok()) + .and_then(|v| v.as_str().map(String::from)); let use_case: MessageUseCase = serde_json::from_slice(payload_bytes)?; if should_use_killswitch(killswitch_config, &use_case) { @@ -1358,7 +1361,7 @@ mod tests { #[test] fn test_shouldnt_killswitch() { - let fake_config = Ok(Some("[custom]".to_string())); + let fake_config = Some("[custom]".to_string()); let use_case = MessageUseCase { use_case_id: "transactions".to_string(), }; @@ -1371,7 +1374,7 @@ mod tests { let use_case = MessageUseCase { use_case_id: "transactions".to_string(), }; - let fake_config = Ok(Some("[transactions]".to_string())); + let fake_config = Some("[transactions]".to_string()); assert!(should_use_killswitch(fake_config, &use_case)); } @@ -1381,7 +1384,7 @@ mod tests { let use_case = MessageUseCase { use_case_id: "transactions".to_string(), }; - let fake_config = Ok(Some("[transactions, custom]".to_string())); + let fake_config = Some("[transactions, custom]".to_string()); assert!(should_use_killswitch(fake_config, &use_case)); } @@ -1391,7 +1394,7 @@ mod tests { let use_case = MessageUseCase { use_case_id: "transactions".to_string(), }; - let fake_config = Ok(Some("[]".to_string())); + let fake_config = Some("[]".to_string()); assert!(!should_use_killswitch(fake_config, &use_case)); } @@ -1401,7 +1404,7 @@ mod tests { let use_case = MessageUseCase { use_case_id: "transactions".to_string(), }; - let fake_config = Ok(Some("".to_string())); + let fake_config = Some("".to_string()); assert!(!should_use_killswitch(fake_config, &use_case)); } @@ -1411,7 +1414,7 @@ mod tests { let use_case = MessageUseCase { use_case_id: "transactions".to_string(), }; - let fake_config = Ok(None); + let fake_config: Option = None; assert!(!should_use_killswitch(fake_config, &use_case)); } diff --git a/rust_snuba/src/processors/utils.rs b/rust_snuba/src/processors/utils.rs index 1b19356d4c9..0a2545ecfad 100644 --- a/rust_snuba/src/processors/utils.rs +++ b/rust_snuba/src/processors/utils.rs @@ -1,6 +1,6 @@ use crate::config::EnvConfig; -use crate::runtime_config::get_str_config; use crate::types::item_type_name; +use sentry_options::options; use chrono::{DateTime, NaiveDateTime, Utc}; use schemars::JsonSchema; use sentry_arroyo::counter; @@ -16,7 +16,7 @@ pub const INVALID_TIMESTAMP_FUTURE_INTERVAL_SECONDS: i64 = 7 * 24 * 60 * 60; /// timestamp dropping is enabled. pub const INVALID_TIMESTAMP_PAST_INTERVAL_SECONDS: i64 = 30 * 24 * 60 * 60; -/// Runtime config key. When set to `"1"`, the eap-items consumer skips messages +/// sentry-options key. When `true`, the eap-items consumer skips messages /// whose event `timestamp` is more than one week in the future or more than /// thirty days in the past (see `out_of_valid_interval_secs`). pub const DROP_INVALID_TIMESTAMPS_KEY: &str = "eap_items_drop_invalid_timestamps"; @@ -34,10 +34,10 @@ pub fn out_of_valid_interval_secs(ts: DateTime, now: DateTime) -> bool } pub fn get_drop_invalid_timestamps_enabled() -> bool { - get_str_config(DROP_INVALID_TIMESTAMPS_KEY) + options("snuba") .ok() - .flatten() - .map(|s| s == "1") + .and_then(|o| o.get(DROP_INVALID_TIMESTAMPS_KEY).ok()) + .and_then(|v| v.as_bool()) .unwrap_or(false) } diff --git a/rust_snuba/src/strategies/healthcheck.rs b/rust_snuba/src/strategies/healthcheck.rs index 9245db18a4c..d7fdb9e9d8e 100644 --- a/rust_snuba/src/strategies/healthcheck.rs +++ b/rust_snuba/src/strategies/healthcheck.rs @@ -7,7 +7,7 @@ use sentry_arroyo::processing::strategies::{ }; use sentry_arroyo::types::Message; -use crate::runtime_config::get_str_config; +use sentry_options::options; const TOUCH_INTERVAL: Duration = Duration::from_secs(1); @@ -57,11 +57,11 @@ where fn poll(&mut self) -> Result, StrategyError> { let poll_result = self.next_step.poll(); - if get_str_config("experimental_healthcheck") + if options("snuba") .ok() - .flatten() - .unwrap_or("0".to_string()) - == "1" + .and_then(|o| o.get("experimental_healthcheck").ok()) + .and_then(|v| v.as_bool()) + .unwrap_or(false) { // If we are receiving a commit request, it means we are making progress and this can be considered a healthy state if let Ok(Some(_commit_request)) = poll_result.as_ref() { @@ -97,16 +97,24 @@ where #[cfg(test)] mod tests { use super::HealthCheck; - use crate::runtime_config::patch_str_config_for_test; use sentry_arroyo::processing::strategies::{ CommitRequest, ProcessingStrategy, StrategyError, SubmitError, }; use sentry_arroyo::types::Message; + use sentry_options::init_with_schemas; + use sentry_options::testing::override_options; + use serde_json::json; use std::collections::HashMap; use std::fs; use std::path::Path; + use std::sync::Once; use std::time::Duration; + static INIT: Once = Once::new(); + fn init_config() { + INIT.call_once(|| init_with_schemas(&[("snuba", crate::SNUBA_SCHEMA)]).unwrap()); + } + // Mock strategy that can be configured to return commit requests struct MockStrategy { return_commit_request: bool, @@ -148,7 +156,9 @@ mod tests { #[test] fn test_file_created_when_making_progress() { // Setup - patch_str_config_for_test("experimental_healthcheck", Some("1")); + init_config(); + let _guard = + override_options(&[("snuba", "experimental_healthcheck", json!(true))]).unwrap(); let file_path = format!("/tmp/healthcheck_test_{}", uuid::Uuid::new_v4()); // Create a mock strategy that returns a commit request @@ -167,7 +177,9 @@ mod tests { #[test] fn test_not_making_progress() { // Setup - patch_str_config_for_test("experimental_healthcheck", Some("1")); + init_config(); + let _guard = + override_options(&[("snuba", "experimental_healthcheck", json!(true))]).unwrap(); let file_path = format!("/tmp/healthcheck_test_{}", uuid::Uuid::new_v4()); // Create a mock strategy that doesn't return a commit request diff --git a/sentry-options/schemas/snuba/schema.json b/sentry-options/schemas/snuba/schema.json index 2ea86bd21c4..190c6251d58 100644 --- a/sentry-options/schemas/snuba/schema.json +++ b/sentry-options/schemas/snuba/schema.json @@ -26,6 +26,321 @@ "type": "integer", "default": 120, "description": "BLQ hysteresis in seconds. Once routing stale, keep routing messages at least (stale_threshold - static_friction) old. Set to 0 to disable friction." + }, + "enable_any_attribute_filter": { + "type": "boolean", + "default": true, + "description": "Enable translating any_attribute_filter trace-item filters into ClickHouse predicates. When false, an any_attribute_filter is treated as always-true (no filtering applied)." + }, + "aggregation_deprecation_enabled": { + "type": "boolean", + "default": true, + "description": "Enable the deprecation check that rejects deprecated aggregation expressions in EAP time-series requests. When false, the check is skipped." + }, + "enable_trace_pagination": { + "type": "boolean", + "default": true, + "description": "Enable pagination (page tokens / limit) in the EndpointGetTrace RPC. When false, the endpoint returns the full trace without pagination." + }, + "use.low.cardinality.processor": { + "type": "boolean", + "default": true, + "description": "Enable the low-cardinality query processor that hints ClickHouse to treat certain columns as low cardinality. When false, the processor is a no-op." + }, + "cross_item_queries_no_sample_outer": { + "type": "boolean", + "default": true, + "description": "For cross-item EAP queries with trace filters, skip sampling on the outer query (the inner trace-id query still samples). When false, the outer query samples at the routing tier." + }, + "default_tier": { + "type": "integer", + "default": 1, + "description": "Default storage routing tier for EAP queries when no other tier is selected. Maps to a Tier enum value (e.g. 1, 8, 64, 512)." + }, + "export_trace_items_default_page_size": { + "type": "integer", + "default": 10000, + "description": "Default page size used by the ExportTraceItems RPC when the request does not specify one." + }, + "use_sampling_factor_timestamp_seconds": { + "type": "integer", + "default": 1744131600, + "description": "Unix timestamp (seconds) cutoff controlling when the sampling-factor codepath applies for EAP queries." + }, + "EndpointGetTrace.apply_final_rollout_percentage": { + "type": "number", + "default": 0.0, + "description": "Rollout percentage (0.0-1.0) for applying the FINAL keyword in EndpointGetTrace queries." + }, + "rpc_logging_sample_rate": { + "type": "number", + "default": 0.0, + "description": "Sample rate (0.0-1.0) for logging RPC requests. 0 disables RPC request logging." + }, + "rpc_logging_flush_logs": { + "type": "number", + "default": 0.0, + "description": "When greater than 0, flush buffered RPC logs. 0 disables flushing." + }, + "ExecutionStage.max_query_size_bytes": { + "type": "integer", + "default": 131072, + "description": "Maximum allowed serialized ClickHouse query size in bytes before the execution stage rejects it." + }, + "ExecutionStage.disable_max_query_size_check_for_clusters": { + "type": "string", + "default": "", + "description": "Comma-separated list of ClickHouse cluster names for which the max-query-size check is disabled. Empty means the check applies to all clusters." + }, + "eap_items_drop_invalid_timestamps": { + "type": "boolean", + "default": false, + "description": "When true, the eap-items consumer skips messages whose event timestamp is more than one week in the future or more than thirty days in the past." + }, + "experimental_healthcheck": { + "type": "boolean", + "default": false, + "description": "When true, the consumer healthcheck strategy treats commit requests (progress) as healthy and touches the healthcheck file accordingly." + }, + "enable_cache_partitioning": { + "type": "boolean", + "default": true, + "description": "When true, query results use the per-storage cache partition; when false they fall back to the default cache partition." + }, + "randomize_query_id": { + "type": "boolean", + "default": false, + "description": "When true, assign a random ClickHouse query_id per execution instead of the deterministic content-based id." + }, + "retry_duplicate_query_id": { + "type": "boolean", + "default": false, + "description": "When true, retry a query that ClickHouse rejected because another query with the same id is already running." + }, + "enable_bypass_cache_referrers": { + "type": "boolean", + "default": false, + "description": "When true, referrers in settings.BYPASS_CACHE_REFERRERS skip the read-through query cache." + }, + "debug_buffer_size_bytes": { + "type": "integer", + "default": 0, + "description": "Size in bytes of the prefix of an INSERT stream kept in memory so a failing row can be attached to the Sentry error. 0 disables the debug buffer." + }, + "project_quota_time_percentage": { + "type": "number", + "default": 1.0, + "description": "Fraction of the counter window a project may spend on replacements before it is reported as exceeding the time limit." + }, + "counter_window_size_minutes": { + "type": "integer", + "default": 10, + "description": "Size in minutes of the rolling window the replacement bucket timer uses to track per-project processing time." + }, + "allows_skipping_single_project_replacements": { + "type": "boolean", + "default": false, + "description": "When true, a single project that exceeds the replacement time limit can be skipped (otherwise only multi-project groups are skipped)." + }, + "use_sentry_metrics": { + "type": "boolean", + "default": false, + "description": "When true, the dual-write metrics backend also emits to the Sentry metrics backend (sampled by settings.DDM_METRICS_SAMPLE_RATE)." + }, + "ondemand_profiler_hostnames": { + "type": "string", + "default": "", + "description": "Comma-separated list of hostnames for which the on-demand profiler should capture a profile." + }, + "throw_on_uniq_select_and_having": { + "type": "boolean", + "default": false, + "description": "When true, raise MismatchedAggregationException if a uniq aggregation appears in HAVING but not SELECT instead of only logging it." + }, + "function-validator.enabled": { + "type": "boolean", + "default": false, + "description": "When true, invalid function names raise InvalidFunctionCall; otherwise only an invalid_funcs metric is emitted." + }, + "mandatory_condition_enforce": { + "type": "boolean", + "default": false, + "description": "When true, queries missing mandatory condition columns raise an assertion; otherwise the omission is only logged." + }, + "eap.reject_string_timestamp_filters": { + "type": "boolean", + "default": true, + "description": "When true, reject EAP time-series filters comparing sentry.timestamp to a TYPE_STRING value." + }, + "trace_ids_cross_item_query_limit": { + "type": "integer", + "default": 50000000, + "description": "Maximum number of trace ids fetched by the cross-item-query trace-id lookup when no explicit limit is supplied." + }, + "storage_routing.enable_get_cluster_loadinfo": { + "type": "boolean", + "default": false, + "description": "When true, the storage routing strategy fetches ClickHouse cluster load info to inform routing decisions." + }, + "max_spans_per_transaction": { + "type": "integer", + "default": 2000, + "description": "Maximum number of spans processed per transaction by the transactions consumer." + }, + "admin.querylog_threads": { + "type": "integer", + "default": 4, + "description": "max_threads ClickHouse setting used by the admin querylog query; clamped to a hard maximum of 4." + }, + "enable_eap_readonly_table": { + "type": "boolean", + "default": false, + "description": "When true, non-consistent EAP-items queries are routed to the read-only table replica." + }, + "enable_events_readonly_table": { + "type": "boolean", + "default": false, + "description": "When true, non-consistent errors/events queries are routed to the read-only table replica." + }, + "use_cross_item_path_for_single_item_queries": { + "type": "boolean", + "default": false, + "description": "When true, GetTraces uses the cross-item query path for single-item queries as well as cross-item queries." + }, + "executor_queue_size_factor": { + "type": "integer", + "default": 10, + "description": "Multiplier applied to max_concurrent_queries to size the subscription executor's pending-future queue before backpressure." + }, + "snuba_api_cogs_probability": { + "type": "number", + "default": 0.0, + "description": "Sampling probability [0,1] for recording per-query COGS (cost-of-goods) accounting from the querylog." + }, + "cache_expiry_sec": { + "type": "integer", + "default": 1, + "description": "TTL in seconds applied to query-result cache entries written to Redis." + }, + "read_through_cache.short_circuit": { + "type": "boolean", + "default": false, + "description": "When true, bypass the read-through query cache entirely and call the underlying function directly (escape hatch for Redis issues)." + }, + "lw_deletions_offpeak_enabled": { + "type": "boolean", + "default": false, + "description": "When true, lightweight deletes are only processed during the configured off-peak window." + }, + "lw_deletions_offpeak_start": { + "type": "integer", + "default": 0, + "description": "UTC hour (0-24) at which the lightweight-deletes off-peak window starts." + }, + "lw_deletions_offpeak_end": { + "type": "integer", + "default": 24, + "description": "UTC hour (0-24) at which the lightweight-deletes off-peak window ends." + }, + "org_ids_delete_allowlist": { + "type": "string", + "default": "", + "description": "Comma-separated org ids allowed to issue EAP-items lightweight deletes; empty means all orgs are allowed." + }, + "max_parts_mutating_for_delete": { + "type": "integer", + "default": 20, + "description": "Maximum number of parts that may be mutating before a lightweight delete is deferred." + }, + "permit_delete_by_attribute": { + "type": "boolean", + "default": false, + "description": "When true, delete-by-attribute requests are permitted; otherwise they are ignored." + }, + "MAX_ONGOING_MUTATIONS_FOR_DELETE": { + "type": "integer", + "default": 5, + "description": "Maximum number of ongoing mutations allowed before a delete is rejected." + }, + "storage_deletes_enabled": { + "type": "boolean", + "default": true, + "description": "Master switch for storage delete (DELETE FROM) support; when false all delete requests are rejected." + }, + "enforce_max_rows_to_delete": { + "type": "boolean", + "default": true, + "description": "When true, reject deletes whose estimated row count exceeds the storage's max_rows_to_delete." + }, + "ignore_clickhouse_settings_override": { + "type": "string", + "default": "", + "description": "Comma/space-delimited ClickHouse setting names to strip from a query's settings override; empty keeps all." + }, + "enable_long_term_retention_downsampling": { + "type": "boolean", + "default": false, + "description": "When true, EAP queries older than 31 days (for non-full-retention item types) are routed to the most-downsampled tier." + }, + "storage_routing_config_override": { + "type": "string", + "default": "{}", + "description": "JSON object mapping organization_id to a per-org StorageRoutingConfig override." + }, + "default_storage_routing_config": { + "type": "string", + "default": "{}", + "description": "JSON-encoded default StorageRoutingConfig used when no per-org override applies." + }, + "subscription_primary_task_builder": { + "type": "string", + "default": "jittered", + "description": "TaskBuilderMode for the subscription scheduler: one of immediate, jittered, transition_jitter, transition_immediate." + }, + "consistent_override": { + "type": "string", + "default": "", + "description": "Semicolon-delimited referrer=probability pairs; for a matching referrer, consistency is dropped with the given probability. Empty means no override." + }, + "skip_final_subscriptions_projects": { + "type": "string", + "default": "[]", + "description": "Bracketed comma-separated list of project ids for which subscription queries skip the FINAL keyword." + }, + "post_replacement_consistency_projects_denylist": { + "type": "string", + "default": "[]", + "description": "Bracketed comma-separated list of project ids that are forced to FINAL after a replacement." + }, + "max_group_ids_exclude": { + "type": "integer", + "default": 256, + "description": "Maximum number of group ids excluded from a query before it falls back to FINAL instead of an exclusion set." + }, + "skip_seen_offsets": { + "type": "boolean", + "default": false, + "description": "When true, the replacer skips replacement messages whose offset has already been seen." + }, + "consumer_groups_to_reset_offset_check": { + "type": "string", + "default": "[]", + "description": "Bracketed comma-separated list of consumer groups whose replacer offset-seen check should be reset." + }, + "write_node_replacements_global": { + "type": "number", + "default": 1.0, + "description": "Probability [0,1] that a replacement is written to every storage node rather than only the distributed table." + }, + "replacements_bypass_projects": { + "type": "string", + "default": "[]", + "description": "JSON array of project ids for which error replacements are skipped." + }, + "generic_metrics_use_case_killswitch": { + "type": "string", + "default": "", + "description": "Substring-matched list of generic-metrics use case ids to drop in the consumer; a message is skipped when its use_case_id appears in this string." } } } diff --git a/snuba/admin/clickhouse/querylog.py b/snuba/admin/clickhouse/querylog.py index f78c5e9d6a7..fa6300c1802 100644 --- a/snuba/admin/clickhouse/querylog.py +++ b/snuba/admin/clickhouse/querylog.py @@ -1,4 +1,3 @@ -from snuba import state from snuba.admin.audit_log.query import audit_log from snuba.admin.clickhouse.common import ( get_ro_query_node_connection, @@ -9,14 +8,11 @@ from snuba.datasets.schemas.tables import TableSchema from snuba.datasets.storages.factory import get_storage from snuba.datasets.storages.storage_key import StorageKey +from snuba.state.sentry_options import get_int_option _MAX_CH_THREADS = 4 -class BadThreadsValue(Exception): - pass - - @audit_log def run_querylog_query(query: str, user: str) -> ClickhouseResult: """ @@ -39,17 +35,8 @@ def describe_querylog_schema() -> ClickhouseResult: def _get_clickhouse_threads() -> int: - config_threads = state.get_config("admin.querylog_threads", _MAX_CH_THREADS) - try: - return min( - int(config_threads) if config_threads is not None else _MAX_CH_THREADS, - _MAX_CH_THREADS, - ) - except ValueError: - # in case the config is set incorrectly - raise BadThreadsValue( - f"{config_threads} is not a valid configuration option for Clickhouse `max_threads`" - ) + config_threads = get_int_option("admin.querylog_threads", _MAX_CH_THREADS) + return min(config_threads, _MAX_CH_THREADS) def __run_querylog_query(query: str) -> ClickhouseResult: diff --git a/snuba/clickhouse/http.py b/snuba/clickhouse/http.py index 2b32dd5dd0c..fc5318af1fd 100644 --- a/snuba/clickhouse/http.py +++ b/snuba/clickhouse/http.py @@ -28,6 +28,7 @@ from snuba.clickhouse.errors import ClickhouseWriterError from snuba.clickhouse.formatter.expression import ClickhouseExpressionFormatter from snuba.clickhouse.query import Expression +from snuba.state.sentry_options import get_int_option from snuba.utils.codecs import Encoder from snuba.utils.iterators import chunked from snuba.utils.metrics import MetricsBackend @@ -315,11 +316,7 @@ def __init__( self.__statement = statement self.__buffer_size = buffer_size self.__chunk_size = chunk_size - self.__debug_buffer_size_bytes = state.get_config("debug_buffer_size_bytes", None) - assert ( - isinstance(self.__debug_buffer_size_bytes, int) - or self.__debug_buffer_size_bytes is None - ) + self.__debug_buffer_size_bytes = get_int_option("debug_buffer_size_bytes", 0) def __repr__(self) -> str: return f"<{type(self).__name__}: {self.__statement.get_qualified_table()} on {self.__pool.host}:{self.__pool.port}>" diff --git a/snuba/datasets/entities/storage_selectors/eap_items.py b/snuba/datasets/entities/storage_selectors/eap_items.py index cc80c5cf7f1..58526632760 100644 --- a/snuba/datasets/entities/storage_selectors/eap_items.py +++ b/snuba/datasets/entities/storage_selectors/eap_items.py @@ -1,6 +1,5 @@ from typing import Sequence -from snuba import state from snuba.datasets.entities.storage_selectors import QueryStorageSelector from snuba.datasets.storage import EntityStorageConnection from snuba.datasets.storages.factory import get_storage @@ -8,6 +7,7 @@ from snuba.downsampled_storage_tiers import Tier from snuba.query.logical import Query from snuba.query.query_settings import HTTPQuerySettings, QuerySettings +from snuba.state.sentry_options import get_bool_option class EAPItemsStorageSelector(QueryStorageSelector): @@ -22,7 +22,7 @@ def select_storage( tier = query_settings.get_sampling_tier() use_readonly_storage = ( - state.get_config("enable_eap_readonly_table", False) + get_bool_option("enable_eap_readonly_table", False) and not query_settings.get_consistent() ) diff --git a/snuba/datasets/entities/storage_selectors/errors.py b/snuba/datasets/entities/storage_selectors/errors.py index d5ec9df084b..525c455beb0 100644 --- a/snuba/datasets/entities/storage_selectors/errors.py +++ b/snuba/datasets/entities/storage_selectors/errors.py @@ -1,6 +1,5 @@ from typing import Sequence -from snuba import state from snuba.datasets.entities.storage_selectors import QueryStorageSelector from snuba.datasets.storage import ( EntityStorageConnection, @@ -10,6 +9,7 @@ ) from snuba.query.logical import Query from snuba.query.query_settings import QuerySettings +from snuba.state.sentry_options import get_bool_option class ErrorsQueryStorageSelector(QueryStorageSelector): @@ -21,7 +21,7 @@ def select_storage( storage_connections: Sequence[EntityStorageConnection], ) -> EntityStorageConnection: use_readonly_storage = ( - state.get_config("enable_events_readonly_table", False) + get_bool_option("enable_events_readonly_table", False) and not query_settings.get_consistent() ) diff --git a/snuba/datasets/processors/transactions_processor.py b/snuba/datasets/processors/transactions_processor.py index 859818d0b16..f2479e656fd 100644 --- a/snuba/datasets/processors/transactions_processor.py +++ b/snuba/datasets/processors/transactions_processor.py @@ -29,7 +29,7 @@ _ensure_valid_ip, _unicodify, ) -from snuba.state import get_config +from snuba.state.sentry_options import get_int_option from snuba.utils.metrics.wrapper import MetricsWrapper logger = logging.getLogger(__name__) @@ -344,12 +344,7 @@ def _process_spans( data = event_dict["data"] trace_context = data["contexts"]["trace"] - try: - max_spans_per_transaction = get_config("max_spans_per_transaction", 2000) - assert isinstance(max_spans_per_transaction, (int, float)) - except Exception: - metrics.increment("bad_config.max_spans_per_transaction") - max_spans_per_transaction = 2000 + max_spans_per_transaction = get_int_option("max_spans_per_transaction", 2000) num_processed = 0 processed_spans = [] diff --git a/snuba/environment.py b/snuba/environment.py index b8311e27540..8f10626cd3a 100644 --- a/snuba/environment.py +++ b/snuba/environment.py @@ -125,6 +125,10 @@ def setup_sentry() -> None: }, ) + from snuba.state.sentry_options import init_options + + init_options() + from snuba.utils.profiler import run_ondemand_profiler if settings.SENTRY_DSN is not None: diff --git a/snuba/lw_deletions/off_peak.py b/snuba/lw_deletions/off_peak.py index 68631c1df5b..d8a933304db 100644 --- a/snuba/lw_deletions/off_peak.py +++ b/snuba/lw_deletions/off_peak.py @@ -7,7 +7,7 @@ from arroyo.processing.strategies.abstract import MessageRejected from arroyo.types import Message -from snuba.state import get_int_config +from snuba.state.sentry_options import get_bool_option, get_int_option from snuba.utils.metrics import MetricsBackend _CACHE_TTL_SECONDS = 60 @@ -49,14 +49,14 @@ def _is_off_peak(self) -> bool: if self.__cached_result is not None and (now - self.__cached_at) < _CACHE_TTL_SECONDS: return self.__cached_result - enabled = get_int_config("lw_deletions_offpeak_enabled", default=0) + enabled = get_bool_option("lw_deletions_offpeak_enabled", False) if not enabled: self.__cached_result = True self.__cached_at = now return True - start = get_int_config("lw_deletions_offpeak_start", default=0) or 0 - end = get_int_config("lw_deletions_offpeak_end", default=24) or 24 + start = get_int_option("lw_deletions_offpeak_start", 0) + end = get_int_option("lw_deletions_offpeak_end", 24) or 24 current_hour = datetime.now(timezone.utc).hour if start == end: diff --git a/snuba/lw_deletions/strategy.py b/snuba/lw_deletions/strategy.py index 6676c5a8f62..03056a91915 100644 --- a/snuba/lw_deletions/strategy.py +++ b/snuba/lw_deletions/strategy.py @@ -2,7 +2,6 @@ import json import logging import time -import typing from datetime import datetime, timedelta from typing import List, Mapping, Optional, Sequence, TypeVar @@ -35,7 +34,8 @@ from snuba.query.expressions import Expression, FunctionCall from snuba.query.query_settings import HTTPQuerySettings from snuba.redis import RedisClientKey, get_redis_client -from snuba.state import get_int_config, get_str_config +from snuba.state import get_int_config +from snuba.state.sentry_options import get_int_option, get_str_option from snuba.utils.metrics import MetricsBackend from snuba.web import QueryException from snuba.web.bulk_delete_query import construct_or_conditions, construct_query @@ -85,7 +85,7 @@ def _filter_allowed_conditions( if self.__storage.get_storage_key() != StorageKey.EAP_ITEMS: return conditions - str_config = get_str_config("org_ids_delete_allowlist", "") + str_config = get_str_option("org_ids_delete_allowlist", "") if not str_config: return conditions # allowlist not set → allow all @@ -317,12 +317,8 @@ def _check_ongoing_mutations(self, skip_throttle: bool = False) -> None: start = time.time() parts_mutating = _num_parts_currently_mutating(self.__storage.get_cluster()) self.__last_ongoing_mutations_check = time.time() - max_parts_mutating = typing.cast( - int, - get_int_config( - "max_parts_mutating_for_delete", - default=settings.MAX_PARTS_MUTATING_FOR_DELETE, - ), + max_parts_mutating = get_int_option( + "max_parts_mutating_for_delete", settings.MAX_PARTS_MUTATING_FOR_DELETE ) self.__metrics.timing("ongoing_mutations_query_ms", (time.time() - start) * 1000) if parts_mutating > max_parts_mutating: diff --git a/snuba/pipeline/stages/query_execution.py b/snuba/pipeline/stages/query_execution.py index b7b200984ea..c4c73da193e 100644 --- a/snuba/pipeline/stages/query_execution.py +++ b/snuba/pipeline/stages/query_execution.py @@ -9,7 +9,7 @@ import sentry_sdk -from snuba import environment, state +from snuba import environment from snuba import settings as snuba_settings from snuba.attribution.attribution_info import AttributionInfo from snuba.clickhouse.formatter.query import format_query @@ -30,6 +30,7 @@ ) from snuba.reader import Reader from snuba.settings import MAX_QUERY_SIZE_BYTES +from snuba.state.sentry_options import get_int_option, get_str_option from snuba.utils.metrics.gauge import Gauge from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.wrapper import MetricsWrapper @@ -163,17 +164,12 @@ def _run_and_apply_column_names( def _max_query_size_bytes() -> int: - return ( - state.get_int_config(MAX_QUERY_SIZE_BYTES_CONFIG, MAX_QUERY_SIZE_BYTES) - or MAX_QUERY_SIZE_BYTES - ) + return get_int_option(MAX_QUERY_SIZE_BYTES_CONFIG, MAX_QUERY_SIZE_BYTES) or MAX_QUERY_SIZE_BYTES def _disable_max_query_size_check_for_clusters() -> set[str]: return set( - (state.get_str_config(DISABLE_MAX_QUERY_SIZE_CHECK_FOR_CLUSTERS_CONFIG, "") or "").split( - "," - ) + (get_str_option(DISABLE_MAX_QUERY_SIZE_CHECK_FOR_CLUSTERS_CONFIG, "") or "").split(",") ) diff --git a/snuba/query/processors/logical/low_cardinality_processor.py b/snuba/query/processors/logical/low_cardinality_processor.py index 07aa77b5e48..ee8f797bfec 100644 --- a/snuba/query/processors/logical/low_cardinality_processor.py +++ b/snuba/query/processors/logical/low_cardinality_processor.py @@ -1,6 +1,5 @@ from dataclasses import replace -from snuba import state from snuba.query.expressions import ( Column, Expression, @@ -11,6 +10,7 @@ from snuba.query.logical import Query from snuba.query.processors.logical import LogicalQueryProcessor from snuba.query.query_settings import QuerySettings +from snuba.state.sentry_options import get_bool_option class LowCardinalityProcessor(LogicalQueryProcessor): @@ -66,7 +66,7 @@ def transform_expressions(exp: Expression) -> Expression: ) return exp - if state.get_int_config("use.low.cardinality.processor", 1) == 0: + if not get_bool_option("use.low.cardinality.processor", True): return query.transform_expressions(transform_expressions) diff --git a/snuba/query/processors/physical/clickhouse_settings_override.py b/snuba/query/processors/physical/clickhouse_settings_override.py index 90d15854043..694cfcbcb2a 100644 --- a/snuba/query/processors/physical/clickhouse_settings_override.py +++ b/snuba/query/processors/physical/clickhouse_settings_override.py @@ -3,7 +3,7 @@ from snuba.clickhouse.query import Query from snuba.query.processors.physical import ClickhouseQueryProcessor from snuba.query.query_settings import QuerySettings -from snuba.state import get_str_config +from snuba.state.sentry_options import get_str_option class ClickhouseSettingsOverride(ClickhouseQueryProcessor): @@ -39,7 +39,7 @@ def process_query(self, query: Query, query_settings: QuerySettings) -> None: new_settings.update(self.__settings) new_settings.update(query_settings.get_clickhouse_settings()) - ignored_settings = get_str_config("ignore_clickhouse_settings_override") + ignored_settings = get_str_option("ignore_clickhouse_settings_override", "") if ignored_settings: new_settings = { setting: value diff --git a/snuba/query/processors/physical/conditions_enforcer.py b/snuba/query/processors/physical/conditions_enforcer.py index 4b1cb9fa406..cb5faf1a234 100644 --- a/snuba/query/processors/physical/conditions_enforcer.py +++ b/snuba/query/processors/physical/conditions_enforcer.py @@ -6,7 +6,7 @@ from snuba.query.processors.condition_checkers import ConditionChecker from snuba.query.processors.physical import ClickhouseQueryProcessor from snuba.query.query_settings import QuerySettings -from snuba.state import get_config +from snuba.state.sentry_options import get_bool_option logger = logging.getLogger(__name__) @@ -48,7 +48,7 @@ def inspect_expression(condition: Expression) -> None: inspect_expression(prewhere) missing_ids = {checker.get_id() for checker in missing_checkers} - if get_config("mandatory_condition_enforce", 0): + if get_bool_option("mandatory_condition_enforce", False): assert not missing_checkers, ( f"Missing mandatory columns in query. Missing {missing_ids}" ) diff --git a/snuba/query/processors/physical/replaced_groups.py b/snuba/query/processors/physical/replaced_groups.py index 4c49b1ac0d9..a6a26be880f 100644 --- a/snuba/query/processors/physical/replaced_groups.py +++ b/snuba/query/processors/physical/replaced_groups.py @@ -14,7 +14,7 @@ from snuba.query.query_settings import QuerySettings, SubscriptionQuerySettings from snuba.replacers.projects_query_flags import ProjectsQueryFlags from snuba.replacers.replacer_processor import ReplacerState -from snuba.state import get_config +from snuba.state.sentry_options import get_int_option, get_str_option from snuba.utils.metrics.wrapper import MetricsWrapper metrics = MetricsWrapper(environment.metrics, "processors.replaced_groups") @@ -51,8 +51,8 @@ def process_query(self, query: Query, query_settings: QuerySettings) -> None: self._set_query_final(query, False) return - for no_final_subscriptions_project in ( - get_config("skip_final_subscriptions_projects") or "[]" + for no_final_subscriptions_project in get_str_option( + "skip_final_subscriptions_projects", "[]" )[1:-1].split(","): if ( no_final_subscriptions_project @@ -63,8 +63,8 @@ def process_query(self, query: Query, query_settings: QuerySettings) -> None: self._set_query_final(query, False) return - for denied_project_id_string in ( - get_config("post_replacement_consistency_projects_denylist") or "[]" + for denied_project_id_string in get_str_option( + "post_replacement_consistency_projects_denylist", "[]" )[1:-1].split(","): if denied_project_id_string and int(denied_project_id_string) in project_ids: metrics.increment(name=CONSISTENCY_DENYLIST_METRIC) @@ -96,11 +96,9 @@ def process_query(self, query: Query, query_settings: QuerySettings) -> None: elif flags.group_ids_to_exclude: # If the number of groups to exclude exceeds our limit, the query # should just use final instead of the exclusion set. - max_group_ids_exclude = get_config( - "max_group_ids_exclude", - settings.REPLACER_MAX_GROUP_IDS_TO_EXCLUDE, + max_group_ids_exclude = get_int_option( + "max_group_ids_exclude", settings.REPLACER_MAX_GROUP_IDS_TO_EXCLUDE ) - assert isinstance(max_group_ids_exclude, int) groups_to_exclude = self._groups_to_exclude(query, flags.group_ids_to_exclude) if ( len(flags.group_ids_to_exclude) > 2 * max_group_ids_exclude diff --git a/snuba/query/processors/physical/uniq_in_select_and_having.py b/snuba/query/processors/physical/uniq_in_select_and_having.py index 8f98e16b499..8a685456d4a 100644 --- a/snuba/query/processors/physical/uniq_in_select_and_having.py +++ b/snuba/query/processors/physical/uniq_in_select_and_having.py @@ -16,7 +16,7 @@ from snuba.query.matchers import Param, String from snuba.query.processors.physical import ClickhouseQueryProcessor from snuba.query.query_settings import QuerySettings -from snuba.state import get_config +from snuba.state.sentry_options import get_bool_option class MismatchedAggregationException(InvalidQueryException): @@ -53,7 +53,7 @@ def process_query(self, query: Query, query_settings: QuerySettings) -> None: for col in selected_columns: col.expression.accept(matcher) if not all(matcher.found_expressions): - should_throw = get_config("throw_on_uniq_select_and_having", False) + should_throw = get_bool_option("throw_on_uniq_select_and_having", False) error = MismatchedAggregationException( "Aggregation is in HAVING clause but not SELECT", query=str(query) ) diff --git a/snuba/query/validation/functions.py b/snuba/query/validation/functions.py index 93c366a9931..22939d64dfe 100644 --- a/snuba/query/validation/functions.py +++ b/snuba/query/validation/functions.py @@ -1,10 +1,11 @@ from typing import Sequence -from snuba import environment, state +from snuba import environment from snuba.query.data_source import DataSource from snuba.query.expressions import Expression from snuba.query.functions import is_valid_global_function from snuba.query.validation import FunctionCallValidator, InvalidFunctionCall +from snuba.state.sentry_options import get_bool_option from snuba.utils.metrics.wrapper import MetricsWrapper metrics = MetricsWrapper(environment.metrics, "validation.functions") @@ -22,7 +23,7 @@ def validate( if is_valid_global_function(func_name): return - if state.get_config("function-validator.enabled", False): + if get_bool_option("function-validator.enabled", False): raise InvalidFunctionCall(f"Invalid function name: {func_name}") else: metrics.increment("invalid_funcs", tags={"func_name": func_name}) diff --git a/snuba/querylog/__init__.py b/snuba/querylog/__init__.py index fcef4cccf7a..ef5032df52c 100644 --- a/snuba/querylog/__init__.py +++ b/snuba/querylog/__init__.py @@ -15,6 +15,7 @@ from snuba.query.exceptions import QueryPlanException from snuba.querylog.query_metadata import QueryStatus, SnubaQueryMetadata, Status from snuba.request import Request +from snuba.state.sentry_options import get_float_option from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.wrapper import MetricsWrapper from snuba.web import QueryException, QueryResult @@ -138,7 +139,7 @@ def _record_cogs( cluster_name = query_metadata.query_list[0].stats.get("cluster_name", "") if cluster_name.startswith("snuba-events-analytics-platform"): - if random() < (state.get_config("snuba_api_cogs_probability") or 0): + if random() < get_float_option("snuba_api_cogs_probability", 0.0): record_cogs( resource_id="eap_clickhouse", app_feature=_get_eap_app_feature(request), @@ -173,7 +174,7 @@ def _record_cogs( .replace("_0", "") ) - if random() < (state.get_config("snuba_api_cogs_probability") or 0): + if random() < get_float_option("snuba_api_cogs_probability", 0.0): record_cogs( resource_id=f"{cluster_name}", app_feature=app_feature, diff --git a/snuba/replacer.py b/snuba/replacer.py index 6cf9e7f547b..bed627ef643 100644 --- a/snuba/replacer.py +++ b/snuba/replacer.py @@ -48,7 +48,7 @@ ReplacementMessage, ReplacementMessageMetadata, ) -from snuba.state import get_int_config, get_str_config +from snuba.state.sentry_options import get_bool_option, get_str_option from snuba.utils.bucket_timer import Counter from snuba.utils.metrics import MetricsBackend from snuba.utils.rate_limiter import RateLimiter @@ -422,7 +422,7 @@ def process_message( "offset": metadata.offset, }, ) - if get_int_config("skip_seen_offsets"): + if get_bool_option("skip_seen_offsets", False): return None seq_message = json.loads(message.payload.value) [version, action_type, data] = seq_message @@ -530,7 +530,7 @@ def _reset_offset_check(self, key: str) -> None: temporarily, then cleared once relevant consumers restart. """ # expected format is "[consumer_group1,consumer_group2,..]" - consumer_groups = (get_str_config(RESET_CHECK_CONFIG) or "[]")[1:-1].split(",") + consumer_groups = get_str_option(RESET_CHECK_CONFIG, "[]")[1:-1].split(",") if self.__consumer_group in consumer_groups: self.__last_offset_processed_per_partition[key] = -1 redis_client.delete(key) diff --git a/snuba/replacers/errors_replacer.py b/snuba/replacers/errors_replacer.py index 011601b4ea5..1924bac70e5 100644 --- a/snuba/replacers/errors_replacer.py +++ b/snuba/replacers/errors_replacer.py @@ -55,7 +55,7 @@ ReplacerProcessor, ReplacerState, ) -from snuba.state import get_config, get_float_config +from snuba.state.sentry_options import get_float_option, get_str_option from snuba.utils.metrics.wrapper import MetricsWrapper """ @@ -109,8 +109,7 @@ def get_project_id(self) -> int: raise NotImplementedError() def should_write_every_node(self) -> bool: - write_node_replacement_setting = get_float_config("write_node_replacements_global", 1.0) - assert isinstance(write_node_replacement_setting, float) + write_node_replacement_setting = get_float_option("write_node_replacements_global", 1.0) if random.random() < write_node_replacement_setting: return True return False @@ -192,7 +191,7 @@ def process_message( raise InvalidMessageType("Invalid message type: {}".format(type_)) if processed is not None: - manual_bypass_projects = get_config("replacements_bypass_projects", "[]") + manual_bypass_projects = get_str_option("replacements_bypass_projects", "[]") auto_bypass_projects = list( get_config_auto_replacements_bypass_projects(datetime.now()).keys() ) diff --git a/snuba/replacers/projects_query_flags.py b/snuba/replacers/projects_query_flags.py index b1e59f99341..99699aa7ad5 100644 --- a/snuba/replacers/projects_query_flags.py +++ b/snuba/replacers/projects_query_flags.py @@ -13,7 +13,7 @@ from snuba.processor import ReplacementType from snuba.redis import RedisClientKey, get_redis_client from snuba.replacers.replacer_processor import ReplacerState -from snuba.state import get_config +from snuba.state.sentry_options import get_int_option redis_client = get_redis_client(RedisClientKey.REPLACEMENTS_STORE) @@ -82,11 +82,9 @@ def set_project_exclude_groups( # the redis key size limit is defined as 2 times the clickhouse query size # limit. there is an explicit check in the query processor for the same # limit - max_group_ids_exclude = get_config( - "max_group_ids_exclude", - settings.REPLACER_MAX_GROUP_IDS_TO_EXCLUDE, + max_group_ids_exclude = get_int_option( + "max_group_ids_exclude", settings.REPLACER_MAX_GROUP_IDS_TO_EXCLUDE ) - assert isinstance(max_group_ids_exclude, int) group_id_data: MutableMapping[str | bytes, bytes | float | int | str] = {} for group_id in group_ids: diff --git a/snuba/request/validation.py b/snuba/request/validation.py index 43ea9643f16..3379391ad57 100644 --- a/snuba/request/validation.py +++ b/snuba/request/validation.py @@ -30,6 +30,7 @@ from snuba.request import Request from snuba.request.exceptions import InvalidJsonRequestException from snuba.request.schema import RequestParts, RequestSchema +from snuba.state.sentry_options import get_str_option from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.wrapper import MetricsWrapper @@ -71,8 +72,8 @@ def parse_mql_query( def _consistent_override(original_setting: bool, referrer: str) -> bool: - consistent_config = state.get_config("consistent_override", None) - if isinstance(consistent_config, str): + consistent_config = get_str_option("consistent_override", "") + if consistent_config: referrers_override = consistent_config.split(";") for config in referrers_override: referrer_config, percentage = config.split("=") diff --git a/snuba/state/cache/redis/backend.py b/snuba/state/cache/redis/backend.py index 375f1ab18cb..eeb70166efc 100644 --- a/snuba/state/cache/redis/backend.py +++ b/snuba/state/cache/redis/backend.py @@ -8,8 +8,8 @@ from snuba import environment, settings from snuba.redis import RedisClientType -from snuba.state import get_config from snuba.state.cache.abstract import Cache, TValue +from snuba.state.sentry_options import get_bool_option, get_int_option from snuba.utils.codecs import ExceptionAwareCodec from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.wrapper import MetricsWrapper @@ -76,7 +76,7 @@ def set(self, key: str, value: TValue) -> None: self.__client.set( self.__build_key(key), self.__codec.encode(value), - ex=get_config("cache_expiry_sec", 1), + ex=get_int_option("cache_expiry_sec", 1), ) def __get_value_with_simple_readthrough( @@ -113,7 +113,7 @@ def __get_value_with_simple_readthrough( self.__client.set( result_key, self.__codec.encode(value), - ex=get_config("cache_expiry_sec", 1), + ex=get_int_option("cache_expiry_sec", 1), ) except Exception as e: @@ -140,7 +140,7 @@ def get_readthrough( ) -> TValue: # in case something is wrong with redis, we want to be able to # disable the read_through_cache but still serve traffic. - if get_config("read_through_cache.short_circuit", 0): + if get_bool_option("read_through_cache.short_circuit", False): return function() try: diff --git a/snuba/state/sentry_options.py b/snuba/state/sentry_options.py new file mode 100644 index 00000000000..8e9e35494b6 --- /dev/null +++ b/snuba/state/sentry_options.py @@ -0,0 +1,127 @@ +"""Thin wrapper around the ``sentry_options`` client for Snuba. + +This is the Python counterpart to the Rust consumers' use of the +``sentry-options`` crate (see ``rust_snuba/src/strategies/blq_router.rs``). +Both read the same ``snuba`` namespace, whose schema lives in +``sentry-options/schemas/snuba/schema.json`` and whose values are managed in +sentry-options-automator and delivered as volume-mounted JSON. + +Unlike runtime config (``snuba.state.get_config`` and friends, backed by +Redis), sentry-options values are read-only from Snuba's perspective: they are +edited centrally and synced into the process, with no in-Snuba write path. +""" + +from __future__ import annotations + +import logging + +import sentry_options +from sentry_options import OptionValue + +logger = logging.getLogger(__name__) + +# Namespace Snuba registers with sentry-options. Must match the directory name +# under ``sentry-options/schemas/`` and the namespace the Rust consumers use. +SNUBA_OPTIONS_NAMESPACE = "snuba" + +_initialized = False + + +def init_options() -> None: + """Initialize the sentry-options client once per process. + + Schemas and values are discovered via the ``sentry_options`` fallback chain + (the ``SENTRY_OPTIONS_DIR`` env var, then ``/etc/sentry-options``, then + ``./sentry-options``). Safe to call repeatedly; only the first successful + call does any work. + + Failures are logged but never raised: a missing or misconfigured options + mount must not take down a service at startup. When initialization fails, + :func:`get_option` falls back to the default passed by each call site, so + behavior matches the pre-sentry-options world. + """ + global _initialized + if _initialized: + return + try: + sentry_options.init() + _initialized = True + except Exception: + logger.warning("Failed to initialize sentry-options", exc_info=True) + + +def get_option(key: str, default: OptionValue) -> OptionValue: + """Read ``key`` from the Snuba sentry-options namespace. + + Returns the configured value, or the schema default when no value is set. + If sentry-options is unavailable for any reason — not initialized, unknown + option, or any other client error — ``default`` is returned, so call sites + behave exactly as they did before the option existed. + """ + try: + return sentry_options.options(SNUBA_OPTIONS_NAMESPACE).get(key) + except sentry_options.OptionsError: + # Expected fallbacks: the client never initialized + # (NotInitializedError), the option/namespace is unknown, or the + # schema is invalid. These all subclass OptionsError; return the + # call-site default silently so behavior matches the pre-option world. + return default + except Exception: + # The client should only ever raise OptionsError, but a hot query path + # must never crash on a config read: honor the "any reason" contract + # above and log the unexpected error so it is still noticed. + logger.warning( + "Unexpected error reading sentry-option %r; using default", key, exc_info=True + ) + return default + + +def get_bool_option(key: str, default: bool) -> bool: + """Read ``key`` as a bool. Replaces ``state.get_int_config`` used as a flag. + + The schema type for these keys is ``boolean``, so ``get`` returns a real + ``bool``; the int/str coercion below only guards against a misconfigured + value and otherwise falls back to ``default``. + """ + value = get_option(key, default) + if isinstance(value, bool): + return value + if isinstance(value, (int, float)): + return bool(value) + if isinstance(value, str): + return value.strip().lower() in ("1", "true", "yes", "on") + return default + + +def get_int_option(key: str, default: int) -> int: + """Read ``key`` as an int. Counterpart to ``state.get_int_config``.""" + value = get_option(key, default) + if isinstance(value, bool): + return int(value) + if isinstance(value, (int, float, str)): + try: + return int(value) + except (TypeError, ValueError): + return default + return default + + +def get_float_option(key: str, default: float) -> float: + """Read ``key`` as a float. Counterpart to ``state.get_float_config``.""" + value = get_option(key, default) + if isinstance(value, bool): + return float(value) + if isinstance(value, (int, float, str)): + try: + return float(value) + except (TypeError, ValueError): + return default + return default + + +def get_str_option(key: str, default: str) -> str: + """Read ``key`` as a str. Counterpart to ``state.get_str_config``.""" + value = get_option(key, default) + if isinstance(value, str): + return value + return default diff --git a/snuba/subscriptions/executor_consumer.py b/snuba/subscriptions/executor_consumer.py index 18c67de7ff4..8049f4e96fb 100644 --- a/snuba/subscriptions/executor_consumer.py +++ b/snuba/subscriptions/executor_consumer.py @@ -21,7 +21,6 @@ from arroyo.processing.strategies.produce import Produce from arroyo.types import Commit -from snuba import state from snuba.clickhouse.errors import ClickhouseError from snuba.consumers.utils import get_partition_count from snuba.datasets.dataset import Dataset @@ -30,6 +29,7 @@ from snuba.datasets.factory import get_dataset from snuba.datasets.table_storage import KafkaTopicSpec from snuba.reader import Result +from snuba.state.sentry_options import get_int_option from snuba.subscriptions.codecs import ( SubscriptionScheduledTaskEncoder, SubscriptionTaskResultEncoder, @@ -324,8 +324,7 @@ def submit(self, message: Message[KafkaPayload]) -> None: # If there are max_concurrent_queries + 10 pending futures in the queue, # we will start raising MessageRejected to slow down the consumer as # it means our executor cannot keep up - queue_size_factor = state.get_config("executor_queue_size_factor", 10) - assert queue_size_factor is not None, "Invalid executor_queue_size_factor config" + queue_size_factor = get_int_option("executor_queue_size_factor", 10) max_queue_size = self.__max_concurrent_queries * queue_size_factor # Tell the consumer to pause until we have removed some futures from diff --git a/snuba/subscriptions/scheduler.py b/snuba/subscriptions/scheduler.py index 767f80837c6..3b1317363f8 100644 --- a/snuba/subscriptions/scheduler.py +++ b/snuba/subscriptions/scheduler.py @@ -13,13 +13,14 @@ Tuple, ) -from snuba import settings, state +from snuba import settings from snuba.datasets.entities.entity_key import EntityKey from snuba.datasets.entities.factory import get_entity from snuba.datasets.slicing import ( map_logical_partition_to_slice, map_org_id_to_logical_partition, ) +from snuba.state.sentry_options import get_str_option from snuba.subscriptions.data import ( PartitionId, ScheduledSubscriptionTask, @@ -192,7 +193,7 @@ def get_start_mode(self, transition_mode: TaskBuilderMode) -> TaskBuilderMode: def get_current_mode(self, subscription: Subscription, timestamp: int) -> TaskBuilderMode: general_mode = TaskBuilderMode( - state.get_config("subscription_primary_task_builder", TaskBuilderMode.JITTERED) + get_str_option("subscription_primary_task_builder", TaskBuilderMode.JITTERED.value) ) if general_mode == TaskBuilderMode.IMMEDIATE or general_mode == TaskBuilderMode.JITTERED: @@ -337,7 +338,7 @@ def __reset_builder(self) -> None: This function is called for every tick. """ general_mode = TaskBuilderMode( - state.get_config("subscription_primary_task_builder", TaskBuilderMode.JITTERED) + get_str_option("subscription_primary_task_builder", TaskBuilderMode.JITTERED.value) ) if general_mode == TaskBuilderMode.JITTERED: self.__builder: TaskBuilder = self.__jittered_builder diff --git a/snuba/utils/bucket_timer.py b/snuba/utils/bucket_timer.py index 7bd1f05fd63..726d00b16c2 100644 --- a/snuba/utils/bucket_timer.py +++ b/snuba/utils/bucket_timer.py @@ -1,12 +1,11 @@ from __future__ import annotations -import typing from collections import defaultdict from datetime import datetime, timedelta from typing import List, MutableMapping -from snuba import environment, state -from snuba.state import get_int_config +from snuba import environment +from snuba.state.sentry_options import get_bool_option, get_float_option, get_int_option from snuba.utils.metrics.wrapper import MetricsWrapper metrics = MetricsWrapper(environment.metrics, "bucket_timer") @@ -37,11 +36,8 @@ def __init__(self, consumer_group: str) -> None: self.consumer_group: str = consumer_group self.buckets: Buckets = {} - percentage = state.get_config("project_quota_time_percentage", 1.0) - assert isinstance(percentage, float) - counter_window_size_minutes = typing.cast( - int, get_int_config(key="counter_window_size_minutes", default=10) - ) + percentage = get_float_option("project_quota_time_percentage", 1.0) + counter_window_size_minutes = get_int_option("counter_window_size_minutes", 10) self.counter_window_size = timedelta(minutes=counter_window_size_minutes) self.limit = self.counter_window_size * percentage @@ -92,7 +88,7 @@ def get_projects_exceeding_limit(self) -> List[int]: for project_id, total_processing_time in project_groups.items(): if total_processing_time > self.limit and ( len(project_groups) > 1 - or get_int_config("allows_skipping_single_project_replacements", default=0) + or get_bool_option("allows_skipping_single_project_replacements", False) ): projects_exceeding_time_limit.append(project_id) diff --git a/snuba/utils/metrics/backends/dualwrite.py b/snuba/utils/metrics/backends/dualwrite.py index a7f1cc1ba00..91584b077a4 100644 --- a/snuba/utils/metrics/backends/dualwrite.py +++ b/snuba/utils/metrics/backends/dualwrite.py @@ -23,9 +23,9 @@ def __init__( self.sentry = sentry def _use_sentry(self) -> bool: - from snuba import state + from snuba.state.sentry_options import get_bool_option - if str(state.get_config("use_sentry_metrics", "0")) == "1": + if get_bool_option("use_sentry_metrics", False): return bool(random.random() < settings.DDM_METRICS_SAMPLE_RATE) return False diff --git a/snuba/utils/profiler.py b/snuba/utils/profiler.py index fece6ac753c..3d0512d647d 100644 --- a/snuba/utils/profiler.py +++ b/snuba/utils/profiler.py @@ -7,7 +7,7 @@ import sentry_sdk from sentry_sdk.tracing import NoOpSpan, Transaction -from snuba.state import get_config +from snuba.state.sentry_options import get_str_option logger = logging.getLogger(__name__) @@ -22,8 +22,7 @@ def _profiler_main() -> None: own_hostname = socket.gethostname() while True: - queried_hostnames = get_config("ondemand_profiler_hostnames") or "" - queried_hostnames = queried_hostnames.split(",") + queried_hostnames = get_str_option("ondemand_profiler_hostnames", "").split(",") if own_hostname in queried_hostnames and current_transaction is None: # Log an error to Sentry on purpose, if the pod slows down it diff --git a/snuba/web/bulk_delete_query.py b/snuba/web/bulk_delete_query.py index 336bd65ccf1..8a0efd982d7 100644 --- a/snuba/web/bulk_delete_query.py +++ b/snuba/web/bulk_delete_query.py @@ -25,7 +25,8 @@ from snuba.query.exceptions import InvalidQueryException, NoRowsToDeleteException from snuba.query.expressions import Expression from snuba.reader import Result -from snuba.state import get_int_config, get_str_config +from snuba.state import get_str_config +from snuba.state.sentry_options import get_bool_option from snuba.utils.metrics.util import with_span from snuba.utils.metrics.wrapper import MetricsWrapper from snuba.utils.schemas import ColumnValidator, InvalidColumnType @@ -228,7 +229,7 @@ def delete_from_storage( if attribute_conditions: _validate_attribute_conditions(attribute_conditions, delete_settings) - if not get_int_config("permit_delete_by_attribute", default=0): + if not get_bool_option("permit_delete_by_attribute", False): metrics.increment("delete_query.delete_ignored") return {} diff --git a/snuba/web/db_query.py b/snuba/web/db_query.py index ff8344330a6..a365ebd6709 100644 --- a/snuba/web/db_query.py +++ b/snuba/web/db_query.py @@ -58,6 +58,7 @@ ) from snuba.state.quota import ResourceQuota from snuba.state.rate_limit import RateLimitExceeded +from snuba.state.sentry_options import get_bool_option from snuba.util import force_bytes from snuba.utils.codecs import ExceptionAwareCodec from snuba.utils.metrics.timer import Timer @@ -201,7 +202,7 @@ def get_query_cache_key(formatted_query: FormattedQuery) -> str: def _get_cache_partition(reader: Reader) -> Cache[Result]: - enable_cache_partitioning = state.get_config("enable_cache_partitioning", 1) + enable_cache_partitioning = get_bool_option("enable_cache_partitioning", True) if not enable_cache_partitioning: return cache_partitions[DEFAULT_CACHE_PARTITION_ID] @@ -235,7 +236,7 @@ def execute_query_with_query_id( robust: bool, referrer: str, ) -> Result: - if state.get_config("randomize_query_id", False): + if get_bool_option("randomize_query_id", False): query_id = uuid.uuid4().hex else: query_id = get_query_cache_key(formatted_query) @@ -254,7 +255,7 @@ def execute_query_with_query_id( referrer, ) except ClickhouseError as e: - if e.code != ErrorCodes.QUERY_WITH_SAME_ID_IS_ALREADY_RUNNING or not state.get_config( + if e.code != ErrorCodes.QUERY_WITH_SAME_ID_IS_ALREADY_RUNNING or not get_bool_option( "retry_duplicate_query_id", False ): raise @@ -291,8 +292,8 @@ def execute_query_with_readthrough_caching( query_id: str, referrer: str, ) -> Result: - if referrer in settings.BYPASS_CACHE_REFERRERS and state.get_config( - "enable_bypass_cache_referrers" + if referrer in settings.BYPASS_CACHE_REFERRERS and get_bool_option( + "enable_bypass_cache_referrers", False ): query_id = f"randomized-{uuid.uuid4().hex}" clickhouse_query_settings["query_id"] = query_id diff --git a/snuba/web/delete_query.py b/snuba/web/delete_query.py index 5fad9069525..f2c0b077c05 100644 --- a/snuba/web/delete_query.py +++ b/snuba/web/delete_query.py @@ -36,7 +36,7 @@ from snuba.query.expressions import Expression, FunctionCall from snuba.query.query_settings import HTTPQuerySettings from snuba.reader import Result -from snuba.state import get_config, get_int_config +from snuba.state.sentry_options import get_bool_option, get_int_option from snuba.utils.metrics.util import with_span from snuba.utils.schemas import ColumnValidator, InvalidColumnType from snuba.web import QueryException, QueryExtraData, QueryResult @@ -97,9 +97,8 @@ def delete_from_storage( # fail if too many mutations ongoing ongoing_mutations = _num_ongoing_mutations(storage.get_cluster(), delete_settings.tables) - max_ongoing_mutations = get_int_config( - "MAX_ONGOING_MUTATIONS_FOR_DELETE", - default=settings.MAX_ONGOING_MUTATIONS_FOR_DELETE, + max_ongoing_mutations = get_int_option( + "MAX_ONGOING_MUTATIONS_FOR_DELETE", settings.MAX_ONGOING_MUTATIONS_FOR_DELETE ) assert max_ongoing_mutations if ongoing_mutations > max_ongoing_mutations: @@ -224,7 +223,7 @@ def _num_parts_currently_mutating(cluster: ClickhouseCluster) -> int: def deletes_are_enabled() -> bool: - return bool(get_config("storage_deletes_enabled", 1)) + return get_bool_option("storage_deletes_enabled", True) def _get_rows_to_delete(storage_key: StorageKey, select_query_to_count_rows: Query) -> int: @@ -291,10 +290,7 @@ def get_new_from_clause() -> Table: if rows_to_delete == 0: raise NoRowsToDeleteException max_rows_allowed = get_storage(storage_key).get_deletion_settings().max_rows_to_delete - if ( - get_int_config("enforce_max_rows_to_delete", default=1) - and rows_to_delete > max_rows_allowed - ): + if get_bool_option("enforce_max_rows_to_delete", True) and rows_to_delete > max_rows_allowed: raise TooManyDeleteRowsException( f"Too many rows to delete ({rows_to_delete}), maximum allowed is {max_rows_allowed}" ) diff --git a/snuba/web/rpc/__init__.py b/snuba/web/rpc/__init__.py index 8846525a960..8fb3c06bb13 100644 --- a/snuba/web/rpc/__init__.py +++ b/snuba/web/rpc/__init__.py @@ -13,8 +13,9 @@ from sentry_protos.snuba.v1.error_pb2 import Error as ErrorProto from sentry_protos.snuba.v1.request_common_pb2 import RequestMeta, TraceItemType -from snuba import environment, state +from snuba import environment from snuba.query.allocation_policies import AllocationPolicyViolations +from snuba.state.sentry_options import get_float_option from snuba.utils.metrics.backends.abstract import MetricsBackend from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.wrapper import MetricsWrapper @@ -55,9 +56,7 @@ def _should_log_rpc_request() -> bool: """ Determine if this RPC request should be logged based on runtime configuration. """ - sample_rate = state.get_float_config("rpc_logging_sample_rate", 0) - if sample_rate is None: - sample_rate = 0 + sample_rate = get_float_option("rpc_logging_sample_rate", 0.0) # If sample rate is 0, never log if sample_rate <= 0.0: @@ -331,7 +330,7 @@ def _before_execute(self, in_msg: Tin) -> None: f"RPC request started - endpoint: {self.__class__.__name__}, request_id: {request_id}" ) - flush_logs = state.get_float_config("rpc_logging_flush_logs", 0) + flush_logs = get_float_option("rpc_logging_flush_logs", 0.0) if flush_logs and flush_logs > 0: _flush_logs() @@ -389,7 +388,7 @@ def _after_execute(self, in_msg: Tin, out_msg: Tout, error: Exception | None) -> logging.info( f"RPC request finished - endpoint: {self.__class__.__name__}, request_id: {request_id}, status: {status}" ) - flush_logs = state.get_float_config("rpc_logging_flush_logs", 0) + flush_logs = get_float_option("rpc_logging_flush_logs", 0.0) if flush_logs and flush_logs > 0: _flush_logs() diff --git a/snuba/web/rpc/common/common.py b/snuba/web/rpc/common/common.py index d196a66e802..2dd66c97e7e 100644 --- a/snuba/web/rpc/common/common.py +++ b/snuba/web/rpc/common/common.py @@ -1,7 +1,7 @@ import json import math from datetime import datetime, timedelta, timezone -from typing import Any, Callable, TypeVar, cast +from typing import Any, Callable, TypeVar from google.protobuf.message import Message as ProtobufMessage from sentry_protos.snuba.v1.request_common_pb2 import RequestMeta @@ -12,7 +12,7 @@ TraceItemFilter, ) -from snuba import settings, state +from snuba import settings from snuba.clickhouse import DATETIME_FORMAT from snuba.protos.common import ( ATTRIBUTES_TO_COALESCE, @@ -47,6 +47,7 @@ Lambda, SubscriptableReference, ) +from snuba.state.sentry_options import get_bool_option, get_int_option from snuba.web.rpc.common.exceptions import BadSnubaRPCRequestException @@ -235,12 +236,9 @@ def use_sampling_factor(meta: RequestMeta) -> bool: """ Since we started writing the sampling factor on a specific date, we should only use it on queries that start after that date. """ - use_sampling_factor_timestamp_seconds = cast( - int, - state.get_int_config( - "use_sampling_factor_timestamp_seconds", - settings.USE_SAMPLING_FACTOR_TIMESTAMP_SECONDS, - ), + use_sampling_factor_timestamp_seconds = get_int_option( + "use_sampling_factor_timestamp_seconds", + settings.USE_SAMPLING_FACTOR_TIMESTAMP_SECONDS, ) if use_sampling_factor_timestamp_seconds == 0: return False @@ -1078,7 +1076,7 @@ def trace_item_filters_to_expression( ) if item_filter.HasField("any_attribute_filter"): - if not state.get_int_config("enable_any_attribute_filter", 1): + if not get_bool_option("enable_any_attribute_filter", True): return literal(True) return _any_attribute_filter_to_expression( item_filter.any_attribute_filter, membership_as_has=membership_as_has diff --git a/snuba/web/rpc/storage_routing/routing_strategies/outcomes_based.py b/snuba/web/rpc/storage_routing/routing_strategies/outcomes_based.py index fe22fd1e095..bc3320d764c 100644 --- a/snuba/web/rpc/storage_routing/routing_strategies/outcomes_based.py +++ b/snuba/web/rpc/storage_routing/routing_strategies/outcomes_based.py @@ -25,6 +25,7 @@ from snuba.query.logical import Query from snuba.query.query_settings import OutcomesQuerySettings from snuba.request import Request as SnubaRequest +from snuba.state.sentry_options import get_bool_option from snuba.web.query import run_query from snuba.web.rpc.common.common import ( timestamp_in_range_condition, @@ -245,7 +246,7 @@ def _update_routing_decision( older_than_thirty_days = thirty_one_days_ago_ts > in_msg_meta.start_timestamp.seconds if ( - state.get_int_config("enable_long_term_retention_downsampling", 0) + get_bool_option("enable_long_term_retention_downsampling", False) and older_than_thirty_days and in_msg_meta.trace_item_type not in ITEM_TYPE_FULL_RETENTION ): diff --git a/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py b/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py index 648a8ab981c..635ba02db79 100644 --- a/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py +++ b/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py @@ -52,6 +52,7 @@ from snuba.query.allocation_policies.utils import get_max_bytes_to_read from snuba.query.query_settings import HTTPQuerySettings from snuba.state import record_query +from snuba.state.sentry_options import get_bool_option, get_int_option from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.wrapper import MetricsWrapper from snuba.utils.registered_class import import_submodules_in_directory @@ -315,7 +316,7 @@ def _get_default_config_definitions(self) -> list[Configuration]: return cast(list[Configuration], self._default_config_definitions) def _get_default_routing_decision_tier(self) -> Tier: - tier_int = state.get_int_config("default_tier", 1) + tier_int = get_int_option("default_tier", 1) if tier_int == 512: return Tier.TIER_512 @@ -509,7 +510,7 @@ def get_routing_decision(self, routing_context: RoutingContext) -> RoutingDecisi routing_context.cluster_load_info = ( get_cluster_loadinfo() - if state.get_config("storage_routing.enable_get_cluster_loadinfo", False) + if get_bool_option("storage_routing.enable_get_cluster_loadinfo", False) else None ) diff --git a/snuba/web/rpc/storage_routing/routing_strategy_selector.py b/snuba/web/rpc/storage_routing/routing_strategy_selector.py index d3d20c75035..bff849c7a19 100644 --- a/snuba/web/rpc/storage_routing/routing_strategy_selector.py +++ b/snuba/web/rpc/storage_routing/routing_strategy_selector.py @@ -8,7 +8,7 @@ from sentry_protos.snuba.v1.downsampled_storage_pb2 import DownsampledStorageConfig from snuba import settings -from snuba.state import get_config +from snuba.state.sentry_options import get_str_option from snuba.web.rpc.storage_routing.common import extract_message_meta from snuba.web.rpc.storage_routing.routing_strategies.outcomes_based import ( OutcomesBasedRoutingStrategy, @@ -89,11 +89,11 @@ def get_storage_routing_config(self, in_msg: ProtobufMessage) -> StorageRoutingC in_msg_meta = extract_message_meta(in_msg) organization_id = str(in_msg_meta.organization_id) try: - overrides = json.loads(str(get_config(_STORAGE_ROUTING_CONFIG_OVERRIDE_KEY, "{}"))) + overrides = json.loads(get_str_option(_STORAGE_ROUTING_CONFIG_OVERRIDE_KEY, "{}")) if organization_id in overrides.keys(): return StorageRoutingConfig.from_json(overrides[organization_id]) - config = str(get_config(_DEFAULT_STORAGE_ROUTING_CONFIG_KEY, "{}")) + config = get_str_option(_DEFAULT_STORAGE_ROUTING_CONFIG_KEY, "{}") return StorageRoutingConfig.from_json(json.loads(config)) except Exception as e: sentry_sdk.capture_message(f"Error getting storage routing config: {e}") diff --git a/snuba/web/rpc/v1/endpoint_export_trace_items.py b/snuba/web/rpc/v1/endpoint_export_trace_items.py index 471047922e0..bd093200c68 100644 --- a/snuba/web/rpc/v1/endpoint_export_trace_items.py +++ b/snuba/web/rpc/v1/endpoint_export_trace_items.py @@ -19,7 +19,6 @@ ) from sentry_protos.snuba.v1.trace_item_pb2 import AnyValue, ArrayValue, TraceItem -from snuba import state from snuba.attribution.appid import AppID from snuba.attribution.attribution_info import AttributionInfo from snuba.datasets.entities.entity_key import EntityKey @@ -32,6 +31,7 @@ from snuba.query.logical import Query from snuba.query.query_settings import HTTPQuerySettings from snuba.request import Request as SnubaRequest +from snuba.state.sentry_options import get_int_option from snuba.web.query import run_query from snuba.web.rpc import RPCEndpoint from snuba.web.rpc.common.common import ( @@ -502,7 +502,7 @@ def response_class(cls) -> Type[ExportTraceItemsResponse]: def _execute(self, in_msg: ExportTraceItemsRequest) -> ExportTraceItemsResponse: default_page_size = ( - state.get_int_config("export_trace_items_default_page_size", _DEFAULT_PAGE_SIZE) + get_int_option("export_trace_items_default_page_size", _DEFAULT_PAGE_SIZE) or _DEFAULT_PAGE_SIZE ) if in_msg.limit > 0: diff --git a/snuba/web/rpc/v1/endpoint_get_trace.py b/snuba/web/rpc/v1/endpoint_get_trace.py index 411883bdca2..aa8cb1d1813 100644 --- a/snuba/web/rpc/v1/endpoint_get_trace.py +++ b/snuba/web/rpc/v1/endpoint_get_trace.py @@ -23,7 +23,6 @@ TraceItemFilter, ) -from snuba import state from snuba.attribution.appid import AppID from snuba.attribution.attribution_info import AttributionInfo from snuba.datasets.entities.entity_key import EntityKey @@ -41,6 +40,7 @@ ENABLE_TRACE_PAGINATION_DEFAULT, ENDPOINT_GET_TRACE_PAGINATION_MAX_ITEMS, ) +from snuba.state.sentry_options import get_bool_option, get_float_option from snuba.utils.metrics.util import with_span from snuba.web.query import run_query from snuba.web.rpc import RPCEndpoint @@ -297,7 +297,7 @@ def _build_query( expression=column("item_id"), ), ] - if state.get_int_config("enable_trace_pagination", ENABLE_TRACE_PAGINATION_DEFAULT): + if get_bool_option("enable_trace_pagination", bool(ENABLE_TRACE_PAGINATION_DEFAULT)): order_by = new_order_by else: order_by = old_order_by @@ -337,7 +337,7 @@ def _build_query( def _get_apply_final_rollout_percentage() -> float: return ( - state.get_float_config( + get_float_option( APPLY_FINAL_ROLLOUT_PERCENTAGE_CONFIG_KEY, 0.0, ) @@ -592,8 +592,8 @@ def _execute(self, in_msg: GetTraceRequest) -> GetTraceResponse: "eap_trace_request_without_limit", 1, tags={"referrer": in_msg.meta.referrer} ) - enable_pagination = state.get_int_config( - "enable_trace_pagination", ENABLE_TRACE_PAGINATION_DEFAULT + enable_pagination = get_bool_option( + "enable_trace_pagination", bool(ENABLE_TRACE_PAGINATION_DEFAULT) ) if enable_pagination: limit = _get_pagination_limit(in_msg.limit) diff --git a/snuba/web/rpc/v1/endpoint_get_traces.py b/snuba/web/rpc/v1/endpoint_get_traces.py index 7096690d012..43cbbebaa1b 100644 --- a/snuba/web/rpc/v1/endpoint_get_traces.py +++ b/snuba/web/rpc/v1/endpoint_get_traces.py @@ -17,7 +17,6 @@ from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey, AttributeValue from sentry_protos.snuba.v1.trace_item_filter_pb2 import AndFilter, TraceItemFilter -from snuba import state from snuba.attribution.appid import AppID from snuba.attribution.attribution_info import AttributionInfo from snuba.datasets.entities.entity_key import EntityKey @@ -39,6 +38,7 @@ from snuba.query.logical import Query from snuba.query.query_settings import HTTPQuerySettings, QuerySettings from snuba.request import Request as SnubaRequest +from snuba.state.sentry_options import get_bool_option from snuba.web.query import run_query from snuba.web.rpc import RPCEndpoint from snuba.web.rpc.common.common import ( @@ -505,7 +505,7 @@ def _execute(self, in_msg: GetTracesRequest) -> GetTracesResponse: _validate_order_by(in_msg) # Feature flag: Use cross-item query path for all queries (single-item and cross-item) - use_cross_item_path = self._is_cross_event_query(in_msg.filters) or state.get_config( + use_cross_item_path = self._is_cross_event_query(in_msg.filters) or get_bool_option( "use_cross_item_path_for_single_item_queries", False ) diff --git a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py index 5e03379def2..63facb5ef75 100644 --- a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py +++ b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py @@ -22,7 +22,6 @@ ExtrapolationMode, ) -from snuba import state from snuba.attribution.appid import AppID from snuba.attribution.attribution_info import AttributionInfo from snuba.datasets.entities.entity_key import EntityKey @@ -37,6 +36,7 @@ from snuba.query.logical import Query from snuba.query.query_settings import HTTPQuerySettings from snuba.request import Request as SnubaRequest +from snuba.state.sentry_options import get_bool_option from snuba.utils.metrics.timer import Timer from snuba.web.query import run_query from snuba.web.rpc.common.common import ( @@ -488,7 +488,7 @@ def resolve( assert len(in_msg.aggregations) == 0 # aggregation is deprecated, it gets converted to conditional_aggregation - if state.get_int_config("aggregation_deprecation_enabled", 1): + if get_bool_option("aggregation_deprecation_enabled", True): for expr in in_msg.expressions: if expr.WhichOneof("expression") == "aggregation": raise RuntimeError( @@ -500,8 +500,8 @@ def resolve( routing_decision.strategy.merge_clickhouse_settings(routing_decision, query_settings) # When trace_filters are present and the feature is enabled, don't use sampling on the outer query # The inner query (getting trace IDs) will use sampling - cross_item_queries_no_sample_outer = state.get_int_config( - "cross_item_queries_no_sample_outer", 1 + cross_item_queries_no_sample_outer = get_bool_option( + "cross_item_queries_no_sample_outer", True ) if not (in_msg.trace_filters and cross_item_queries_no_sample_outer): query_settings.set_sampling_tier(routing_decision.tier) diff --git a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py index caa9cd2da9f..2d80a0735ae 100644 --- a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py +++ b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py @@ -24,7 +24,6 @@ VirtualColumnContext, ) -from snuba import state from snuba.attribution.appid import AppID from snuba.attribution.attribution_info import AttributionInfo from snuba.datasets.entities.entity_key import EntityKey @@ -52,6 +51,7 @@ from snuba.query.logical import Query from snuba.query.query_settings import HTTPQuerySettings from snuba.request import Request as SnubaRequest +from snuba.state.sentry_options import get_bool_option from snuba.utils.metrics.timer import Timer from snuba.web.query import run_query from snuba.web.rpc.common.common import ( @@ -751,8 +751,8 @@ def resolve( routing_decision.strategy.merge_clickhouse_settings(routing_decision, query_settings) # When trace_filters are present and the feature is enabled, don't use sampling on the outer query # The inner query (getting trace IDs) will use sampling - cross_item_queries_no_sample_outer = state.get_int_config( - "cross_item_queries_no_sample_outer", 1 + cross_item_queries_no_sample_outer = get_bool_option( + "cross_item_queries_no_sample_outer", True ) if not (in_msg.trace_filters and cross_item_queries_no_sample_outer): query_settings.set_sampling_tier(routing_decision.tier) diff --git a/snuba/web/rpc/v1/resolvers/common/cross_item_queries.py b/snuba/web/rpc/v1/resolvers/common/cross_item_queries.py index b51ddf6adef..223badbb333 100644 --- a/snuba/web/rpc/v1/resolvers/common/cross_item_queries.py +++ b/snuba/web/rpc/v1/resolvers/common/cross_item_queries.py @@ -8,7 +8,6 @@ TraceItemFilterWithType, ) -from snuba import state from snuba.attribution.appid import AppID from snuba.attribution.attribution_info import AttributionInfo from snuba.datasets.entities.entity_key import EntityKey @@ -22,6 +21,7 @@ from snuba.query.logical import Query from snuba.query.query_settings import HTTPQuerySettings from snuba.request import Request as SnubaRequest +from snuba.state.sentry_options import get_int_option from snuba.utils.metrics.timer import Timer from snuba.web import QueryResult from snuba.web.query import run_query @@ -153,7 +153,7 @@ def get_trace_ids_sql_for_cross_item_query( expression=f.max(column("timestamp")), ), ], - limit=limit or state.get_config("trace_ids_cross_item_query_limit", _TRACE_LIMIT), + limit=limit or get_int_option("trace_ids_cross_item_query_limit", _TRACE_LIMIT), ) treeify_or_and_conditions(query) diff --git a/snuba/web/rpc/v1/visitors/time_series_request_visitor.py b/snuba/web/rpc/v1/visitors/time_series_request_visitor.py index b46318c58cc..0368c8ccf44 100644 --- a/snuba/web/rpc/v1/visitors/time_series_request_visitor.py +++ b/snuba/web/rpc/v1/visitors/time_series_request_visitor.py @@ -15,7 +15,7 @@ ) from sentry_protos.snuba.v1.trace_item_filter_pb2 import TraceItemFilter -from snuba.state import get_config +from snuba.state.sentry_options import get_bool_option from snuba.web.rpc.common.exceptions import BadSnubaRPCRequestException from snuba.web.rpc.v1.visitors.trace_item_table_request_visitor import ( NormalizeFormulaLabelsVisitor, @@ -127,7 +127,7 @@ def visit_TraceItemFilter(self, node: TraceItemFilter) -> None: elif node.HasField("comparison_filter"): k = node.comparison_filter.key if k.name == "sentry.timestamp" and k.type == AttributeKey.TYPE_STRING: - if get_config("eap.reject_string_timestamp_filters", 1): + if get_bool_option("eap.reject_string_timestamp_filters", True): raise BadSnubaRPCRequestException( "sentry.timestamp can only be compared to TYPE_INT or TYPE_DOUBLE, got TYPE_STRING" ) diff --git a/tests/admin/clickhouse/test_querylog.py b/tests/admin/clickhouse/test_querylog.py index b5d998ccdfb..ab71e84e0b3 100644 --- a/tests/admin/clickhouse/test_querylog.py +++ b/tests/admin/clickhouse/test_querylog.py @@ -1,15 +1,9 @@ from __future__ import annotations -from typing import Type - import pytest +from sentry_options.testing import override_options -from snuba import state -from snuba.admin.clickhouse.querylog import ( - _MAX_CH_THREADS, - BadThreadsValue, - _get_clickhouse_threads, -) +from snuba.admin.clickhouse.querylog import _MAX_CH_THREADS, _get_clickhouse_threads @pytest.mark.parametrize( @@ -20,19 +14,6 @@ ], ) @pytest.mark.redis_db -def test_get_clickhouse_threads(config_val: str | int, expected_threads: int) -> None: - state.set_config("admin.querylog_threads", str(config_val)) - assert _get_clickhouse_threads() == expected_threads - - -@pytest.mark.parametrize( - "config_val, error", - [ - pytest.param("invalid_value", BadThreadsValue, id="invalid_value"), - ], -) -@pytest.mark.redis_db -def test_get_clickhouse_threads_error(config_val: str | int, error: Type[Exception]) -> None: - state.set_config("admin.querylog_threads", str(config_val)) - with pytest.raises(error): - _get_clickhouse_threads() +def test_get_clickhouse_threads(config_val: int, expected_threads: int) -> None: + with override_options("snuba", {"admin.querylog_threads": config_val}): + assert _get_clickhouse_threads() == expected_threads diff --git a/tests/conftest.py b/tests/conftest.py index 48dfe33c5c9..2e36359c521 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -47,6 +47,14 @@ def pytest_configure() -> None: """ assert settings.TESTING, "settings.TESTING is False, try `SNUBA_SETTINGS=test` or `make test`" + # Point sentry-options at the in-repo schemas so init() succeeds regardless + # of the working directory tests are launched from. (Without this it relies + # on the ./sentry-options relative fallback.) + os.environ.setdefault( + "SENTRY_OPTIONS_DIR", + os.path.join(os.path.dirname(__file__), os.pardir, "sentry-options"), + ) + initialize_snuba() setup_sentry() initialize_snuba() diff --git a/tests/datasets/entities/storage_selectors/test_eap_items.py b/tests/datasets/entities/storage_selectors/test_eap_items.py index 7034226823c..5f8e9c597a7 100644 --- a/tests/datasets/entities/storage_selectors/test_eap_items.py +++ b/tests/datasets/entities/storage_selectors/test_eap_items.py @@ -1,6 +1,6 @@ import pytest +from sentry_options.testing import override_options -from snuba import state from snuba.datasets.entities.entity_key import EntityKey from snuba.datasets.entities.factory import get_entity from snuba.datasets.entities.storage_selectors.eap_items import EAPItemsStorageSelector @@ -43,48 +43,39 @@ def test_selects_correct_eap_items_tier() -> None: @pytest.mark.redis_db +@override_options("snuba", {"enable_eap_readonly_table": True}) def test_selects_eap_items_ro_when_enabled() -> None: unimportant_query = Query(from_clause=EAP_ITEMS_ENTITY) query_settings = HTTPQuerySettings() query_settings.set_sampling_tier(Tier.TIER_1) - state.set_config("enable_eap_readonly_table", 1) - try: - selected_storage = EAPItemsStorageSelector().select_storage( - unimportant_query, query_settings, EAP_ITEMS_STORAGE_CONNECTIONS - ) - assert selected_storage.storage == get_storage(StorageKey.EAP_ITEMS_RO) - finally: - state.delete_config("enable_eap_readonly_table") + selected_storage = EAPItemsStorageSelector().select_storage( + unimportant_query, query_settings, EAP_ITEMS_STORAGE_CONNECTIONS + ) + assert selected_storage.storage == get_storage(StorageKey.EAP_ITEMS_RO) @pytest.mark.redis_db +@override_options("snuba", {"enable_eap_readonly_table": True}) def test_selects_writable_when_consistent() -> None: unimportant_query = Query(from_clause=EAP_ITEMS_ENTITY) query_settings = HTTPQuerySettings(consistent=True) query_settings.set_sampling_tier(Tier.TIER_1) - state.set_config("enable_eap_readonly_table", 1) - try: - selected_storage = EAPItemsStorageSelector().select_storage( - unimportant_query, query_settings, EAP_ITEMS_STORAGE_CONNECTIONS - ) - assert selected_storage.storage == get_storage(StorageKey.EAP_ITEMS) - finally: - state.delete_config("enable_eap_readonly_table") + selected_storage = EAPItemsStorageSelector().select_storage( + unimportant_query, query_settings, EAP_ITEMS_STORAGE_CONNECTIONS + ) + assert selected_storage.storage == get_storage(StorageKey.EAP_ITEMS) @pytest.mark.redis_db +@override_options("snuba", {"enable_eap_readonly_table": True}) def test_selects_downsample_ro_when_enabled() -> None: unimportant_query = Query(from_clause=EAP_ITEMS_ENTITY) query_settings = HTTPQuerySettings() query_settings.set_sampling_tier(Tier.TIER_512) - state.set_config("enable_eap_readonly_table", 1) - try: - selected_storage = EAPItemsStorageSelector().select_storage( - unimportant_query, query_settings, EAP_ITEMS_STORAGE_CONNECTIONS - ) - assert selected_storage.storage == get_storage(StorageKey.EAP_ITEMS_DOWNSAMPLE_512_RO) - finally: - state.delete_config("enable_eap_readonly_table") + selected_storage = EAPItemsStorageSelector().select_storage( + unimportant_query, query_settings, EAP_ITEMS_STORAGE_CONNECTIONS + ) + assert selected_storage.storage == get_storage(StorageKey.EAP_ITEMS_DOWNSAMPLE_512_RO) diff --git a/tests/datasets/entities/storage_selectors/test_errors.py b/tests/datasets/entities/storage_selectors/test_errors.py index 570e133b463..285d4c7d864 100644 --- a/tests/datasets/entities/storage_selectors/test_errors.py +++ b/tests/datasets/entities/storage_selectors/test_errors.py @@ -1,8 +1,8 @@ from typing import List import pytest +from sentry_options.testing import override_options -from snuba import state from snuba.clickhouse.translators.snuba.mappers import ( ColumnToColumn, ColumnToIPAddress, @@ -109,15 +109,14 @@ def test_query_storage_selector( use_readable: bool, expected_storage: Storage, ) -> None: - state.set_config("enable_events_readonly_table", use_readable) - - query = parse_snql_query(str(snql_query), dataset) - assert isinstance(query, Query) + with override_options("snuba", {"enable_events_readonly_table": use_readable}): + query = parse_snql_query(str(snql_query), dataset) + assert isinstance(query, Query) - selected_storage = selector.select_storage( - query, HTTPQuerySettings(referrer="r"), storage_connections - ) - assert selected_storage.storage == expected_storage + selected_storage = selector.select_storage( + query, HTTPQuerySettings(referrer="r"), storage_connections + ) + assert selected_storage.storage == expected_storage def test_assert_raises() -> None: diff --git a/tests/datasets/storages/processors/test_replaced_groups.py b/tests/datasets/storages/processors/test_replaced_groups.py index fda74dc59bd..d601b87544a 100644 --- a/tests/datasets/storages/processors/test_replaced_groups.py +++ b/tests/datasets/storages/processors/test_replaced_groups.py @@ -2,8 +2,8 @@ from typing import Sequence import pytest +from sentry_options.testing import override_options -from snuba import state from snuba.clickhouse.columns import ColumnSet from snuba.clickhouse.query import Query as ClickhouseQuery from snuba.datasets.storages.storage_key import StorageKey @@ -139,25 +139,20 @@ def test_without_turbo_with_projects_needing_final(query: ClickhouseQuery) -> No ) query_settings = HTTPQuerySettings() - PostReplacementConsistencyEnforcer( - "project_id", ReplacerState.ERRORS - ).process_query(query, query_settings) + PostReplacementConsistencyEnforcer("project_id", ReplacerState.ERRORS).process_query( + query, query_settings + ) assert query.get_condition() == build_in("project_id", [2]) assert query.get_from_clause().final assert ( - query_settings.get_clickhouse_settings()[ - "do_not_merge_across_partitions_select_final" - ] - == 1 + query_settings.get_clickhouse_settings()["do_not_merge_across_partitions_select_final"] == 1 ) @pytest.mark.redis_db def test_without_turbo_without_projects_needing_final(query: ClickhouseQuery) -> None: - PostReplacementConsistencyEnforcer("project_id", None).process_query( - query, HTTPQuerySettings() - ) + PostReplacementConsistencyEnforcer("project_id", None).process_query(query, HTTPQuerySettings()) assert query.get_condition() == build_in("project_id", [2]) assert not query.get_from_clause().final @@ -171,22 +166,22 @@ def test_remove_final_subscriptions(query: ClickhouseQuery) -> None: ReplacementType.EXCLUDE_GROUPS, # Arbitrary replacement type, no impact on tests ) - PostReplacementConsistencyEnforcer( - "project_id", ReplacerState.ERRORS - ).process_query(query, SubscriptionQuerySettings()) + PostReplacementConsistencyEnforcer("project_id", ReplacerState.ERRORS).process_query( + query, SubscriptionQuerySettings() + ) assert query.get_condition() == build_in("project_id", [2]) assert query.get_from_clause().final - state.set_config("skip_final_subscriptions_projects", "[2,3,4]") - PostReplacementConsistencyEnforcer( - "project_id", ReplacerState.ERRORS - ).process_query(query, SubscriptionQuerySettings()) - assert not query.get_from_clause().final + with override_options("snuba", {"skip_final_subscriptions_projects": "[2,3,4]"}): + PostReplacementConsistencyEnforcer("project_id", ReplacerState.ERRORS).process_query( + query, SubscriptionQuerySettings() + ) + assert not query.get_from_clause().final @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 5}) def test_not_many_groups_to_exclude(query: ClickhouseQuery) -> None: - state.set_config("max_group_ids_exclude", 5) ProjectsQueryFlags.set_project_exclude_groups( 2, [100, 101, 102], @@ -194,9 +189,9 @@ def test_not_many_groups_to_exclude(query: ClickhouseQuery) -> None: ReplacementType.EXCLUDE_GROUPS, # Arbitrary replacement type, no impact on tests ) - PostReplacementConsistencyEnforcer( - "project_id", ReplacerState.ERRORS - ).process_query(query, HTTPQuerySettings()) + PostReplacementConsistencyEnforcer("project_id", ReplacerState.ERRORS).process_query( + query, HTTPQuerySettings() + ) assert query.get_condition() == build_and( FunctionCall( @@ -221,8 +216,8 @@ def test_not_many_groups_to_exclude(query: ClickhouseQuery) -> None: @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 2}) def test_too_many_groups_to_exclude(query: ClickhouseQuery) -> None: - state.set_config("max_group_ids_exclude", 2) ProjectsQueryFlags.set_project_exclude_groups( 2, [100, 101, 102], @@ -230,15 +225,16 @@ def test_too_many_groups_to_exclude(query: ClickhouseQuery) -> None: ReplacementType.EXCLUDE_GROUPS, # Arbitrary replacement type, no impact on tests ) - PostReplacementConsistencyEnforcer( - "project_id", ReplacerState.ERRORS - ).process_query(query, HTTPQuerySettings()) + PostReplacementConsistencyEnforcer("project_id", ReplacerState.ERRORS).process_query( + query, HTTPQuerySettings() + ) assert query.get_condition() == build_in("project_id", [2]) assert query.get_from_clause().final @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 2}) def test_query_overlaps_replacements_processor( query: ClickhouseQuery, query_with_timestamp: ClickhouseQuery, @@ -252,7 +248,6 @@ def test_query_overlaps_replacements_processor( assert not query_with_timestamp.get_from_clause().final # overlaps replacement and should be final due to too many groups to exclude - state.set_config("max_group_ids_exclude", 2) ProjectsQueryFlags.set_project_exclude_groups( 2, [100, 101, 102], @@ -275,6 +270,7 @@ def test_query_overlaps_replacements_processor( @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 2}) def test_single_no_replacements(query_with_single_group_id: ClickhouseQuery) -> None: """ Query is looking for a group that has not been replaced, but the project itself @@ -290,7 +286,6 @@ def test_single_no_replacements(query_with_single_group_id: ClickhouseQuery) -> ) enforcer._set_query_final(query_with_single_group_id, True) - state.set_config("max_group_ids_exclude", 2) enforcer.process_query(query_with_single_group_id, HTTPQuerySettings()) assert query_with_single_group_id.get_condition() == build_and( @@ -300,6 +295,7 @@ def test_single_no_replacements(query_with_single_group_id: ClickhouseQuery) -> @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 2}) def test_single_too_many_exclude(query_with_single_group_id: ClickhouseQuery) -> None: """ Query is looking for a group that has been replaced, and there are too many @@ -315,7 +311,6 @@ def test_single_too_many_exclude(query_with_single_group_id: ClickhouseQuery) -> ) enforcer._set_query_final(query_with_single_group_id, True) - state.set_config("max_group_ids_exclude", 2) enforcer.process_query(query_with_single_group_id, HTTPQuerySettings()) assert query_with_single_group_id.get_condition() == build_and( @@ -326,6 +321,7 @@ def test_single_too_many_exclude(query_with_single_group_id: ClickhouseQuery) -> @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 5}) def test_single_not_too_many_exclude( query_with_single_group_id: ClickhouseQuery, ) -> None: @@ -343,7 +339,6 @@ def test_single_not_too_many_exclude( ) enforcer._set_query_final(query_with_single_group_id, True) - state.set_config("max_group_ids_exclude", 5) enforcer.process_query(query_with_single_group_id, HTTPQuerySettings()) assert query_with_single_group_id.get_condition() == build_and( @@ -354,6 +349,7 @@ def test_single_not_too_many_exclude( @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 5}) def test_multiple_disjoint_replaced( query_with_multiple_group_ids: ClickhouseQuery, ) -> None: @@ -371,7 +367,6 @@ def test_multiple_disjoint_replaced( ) enforcer._set_query_final(query_with_multiple_group_ids, True) - state.set_config("max_group_ids_exclude", 5) enforcer.process_query(query_with_multiple_group_ids, HTTPQuerySettings()) assert query_with_multiple_group_ids.get_condition() == build_and( @@ -381,6 +376,7 @@ def test_multiple_disjoint_replaced( @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 5}) def test_multiple_fewer_exclude_than_queried( query_with_multiple_group_ids: ClickhouseQuery, ) -> None: @@ -398,7 +394,6 @@ def test_multiple_fewer_exclude_than_queried( ) enforcer._set_query_final(query_with_multiple_group_ids, True) - state.set_config("max_group_ids_exclude", 5) enforcer.process_query(query_with_multiple_group_ids, HTTPQuerySettings()) assert query_with_multiple_group_ids.get_condition() == build_and( @@ -409,6 +404,7 @@ def test_multiple_fewer_exclude_than_queried( @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 2}) def test_multiple_too_many_excludes( query_with_multiple_group_ids: ClickhouseQuery, ) -> None: @@ -426,7 +422,6 @@ def test_multiple_too_many_excludes( ) enforcer._set_query_final(query_with_multiple_group_ids, True) - state.set_config("max_group_ids_exclude", 2) enforcer.process_query(query_with_multiple_group_ids, HTTPQuerySettings()) assert query_with_multiple_group_ids.get_condition() == build_and( @@ -438,6 +433,7 @@ def test_multiple_too_many_excludes( @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 5}) def test_multiple_not_too_many_excludes( query_with_multiple_group_ids: ClickhouseQuery, ) -> None: @@ -455,7 +451,6 @@ def test_multiple_not_too_many_excludes( ) enforcer._set_query_final(query_with_multiple_group_ids, True) - state.set_config("max_group_ids_exclude", 5) enforcer.process_query(query_with_multiple_group_ids, HTTPQuerySettings()) assert query_with_multiple_group_ids.get_condition() == build_and( @@ -466,6 +461,7 @@ def test_multiple_not_too_many_excludes( @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 3}) def test_no_groups_not_too_many_excludes(query: ClickhouseQuery) -> None: """ Query has no groups, and not too many to exclude. @@ -480,7 +476,6 @@ def test_no_groups_not_too_many_excludes(query: ClickhouseQuery) -> None: ) enforcer._set_query_final(query, True) - state.set_config("max_group_ids_exclude", 3) enforcer.process_query(query, HTTPQuerySettings()) assert query.get_condition() == build_and( @@ -491,6 +486,7 @@ def test_no_groups_not_too_many_excludes(query: ClickhouseQuery) -> None: @pytest.mark.redis_db +@override_options("snuba", {"max_group_ids_exclude": 1}) def test_no_groups_too_many_excludes(query: ClickhouseQuery) -> None: """ Query has no groups, and too many to exclude. @@ -505,7 +501,6 @@ def test_no_groups_too_many_excludes(query: ClickhouseQuery) -> None: ) enforcer._set_query_final(query, True) - state.set_config("max_group_ids_exclude", 1) enforcer.process_query(query, HTTPQuerySettings()) assert query.get_condition() == build_in("project_id", [2]) diff --git a/tests/datasets/test_errors_replacer.py b/tests/datasets/test_errors_replacer.py index b00c8dba62f..00db1030ed3 100644 --- a/tests/datasets/test_errors_replacer.py +++ b/tests/datasets/test_errors_replacer.py @@ -8,6 +8,7 @@ import simplejson as json from arroyo.backends.kafka import KafkaPayload from arroyo.types import BrokerValue, Message, Partition, Topic +from sentry_options.testing import override_options from snuba import replacer, settings from snuba.clickhouse import DATETIME_FORMAT @@ -19,7 +20,6 @@ from snuba.redis import RedisClientKey, get_redis_client from snuba.replacers import errors_replacer from snuba.settings import PAYLOAD_DATETIME_FORMAT -from snuba.state import delete_config, set_config from snuba.utils.metrics.backends.dummy import DummyMetricsBackend from tests.fixtures import get_raw_event from tests.helpers import write_unprocessed_events @@ -54,9 +54,9 @@ def setup_method(self) -> None: # Total query time range is 24h before to 24h after now to account # for local machine time zones - self.from_time = datetime.now().replace( - minute=0, second=0, microsecond=0 - ) - timedelta(days=1) + self.from_time = datetime.now().replace(minute=0, second=0, microsecond=0) - timedelta( + days=1 + ) self.to_time = self.from_time + timedelta(days=2) @@ -111,9 +111,7 @@ def _get_group_id(self, project_id: int, event_id: str) -> Optional[int]: args = { "project": [project_id], "selected_columns": ["group_id"], - "conditions": [ - ["event_id", "=", str(uuid.UUID(event_id)).replace("-", "")] - ], + "conditions": [["event_id", "=", str(uuid.UUID(event_id)).replace("-", "")]], "from_date": self.from_time.isoformat(), "to_date": self.to_time.isoformat(), "tenant_ids": {"referrer": "r", "organization_id": 1234}, @@ -329,8 +327,8 @@ def test_unmerge_insert(self) -> None: assert self._issue_count(self.project_id) == [{"count": 1, "group_id": 2}] + @override_options("snuba", {"skip_seen_offsets": True}) def test_process_offset_twice(self) -> None: - set_config("skip_seen_offsets", True) self.event["project_id"] = self.project_id self.event["group_id"] = 1 self.event["primary_hash"] = "a" * 32 @@ -349,9 +347,7 @@ def test_process_offset_twice(self) -> None: "previous_group_id": 1, "new_group_id": 2, "hashes": ["a" * 32], - "datetime": datetime.utcnow().strftime( - PAYLOAD_DATETIME_FORMAT - ), + "datetime": datetime.utcnow().strftime(PAYLOAD_DATETIME_FORMAT), }, ) ).encode("utf-8"), @@ -369,11 +365,11 @@ def test_process_offset_twice(self) -> None: # should be None since the offset should be in Redis, indicating it should be skipped assert self.replacer.process_message(message) is None + @override_options("snuba", {"skip_seen_offsets": True}) def test_multiple_partitions(self) -> None: """ Different partitions should have independent offset checks. """ - set_config("skip_seen_offsets", True) self.event["project_id"] = self.project_id self.event["group_id"] = 1 self.event["primary_hash"] = "a" * 32 @@ -421,8 +417,8 @@ def test_multiple_partitions(self) -> None: # different partition should be unaffected even if it's the same offset assert self.replacer.process_message(partition_two) is not None + @override_options("snuba", {"skip_seen_offsets": True}) def test_reset_consumer_group_offset_check(self) -> None: - set_config("skip_seen_offsets", True) self.event["project_id"] = self.project_id self.event["group_id"] = 1 self.event["primary_hash"] = "a" * 32 @@ -441,9 +437,7 @@ def test_reset_consumer_group_offset_check(self) -> None: "previous_group_id": 1, "new_group_id": 2, "hashes": ["a" * 32], - "datetime": datetime.utcnow().strftime( - PAYLOAD_DATETIME_FORMAT - ), + "datetime": datetime.utcnow().strftime(PAYLOAD_DATETIME_FORMAT), }, ) ).encode("utf-8"), @@ -457,16 +451,15 @@ def test_reset_consumer_group_offset_check(self) -> None: self.replacer.flush_batch([self.replacer.process_message(message)]) - set_config(replacer.RESET_CHECK_CONFIG, f"[{CONSUMER_GROUP}]") - - # Offset to check against should be reset so this message shouldn't be skipped - assert self.replacer.process_message(message) is not None + with override_options("snuba", {replacer.RESET_CHECK_CONFIG: f"[{CONSUMER_GROUP}]"}): + # Offset to check against should be reset so this message shouldn't be skipped + assert self.replacer.process_message(message) is not None + @override_options("snuba", {"skip_seen_offsets": True}) def test_offset_already_processed(self) -> None: """ Don't process an offset that already exists in Redis. """ - set_config("skip_seen_offsets", True) self.event["project_id"] = self.project_id self.event["group_id"] = 1 self.event["primary_hash"] = "a" * 32 @@ -596,9 +589,7 @@ def test_delete_unpromoted_tag_process(self) -> None: assert replacement.get_query_time_flags() == errors_replacer.NeedsFinal() - @pytest.mark.parametrize( - "old_primary_hash", ["e3d704f3542b44a621ebed70dc0efe13", False, None] - ) + @pytest.mark.parametrize("old_primary_hash", ["e3d704f3542b44a621ebed70dc0efe13", False, None]) def test_tombstone_events_process(self, old_primary_hash) -> None: timestamp = datetime.now() message_kwargs = { @@ -617,9 +608,7 @@ def test_tombstone_events_process(self, old_primary_hash) -> None: _, replacement = meta_and_replacement old_primary_condition = ( - " AND primary_hash = 'e3d704f3-542b-44a6-21eb-ed70dc0efe13'" - if old_primary_hash - else "" + " AND primary_hash = 'e3d704f3-542b-44a6-21eb-ed70dc0efe13'" if old_primary_hash else "" ) query_args = { @@ -759,9 +748,7 @@ def test_merge_process(self) -> None: % query_args ) - assert replacement.get_query_time_flags() == errors_replacer.ExcludeGroups( - [1, 2] - ) + assert replacement.get_query_time_flags() == errors_replacer.ExcludeGroups([1, 2]) def test_unmerge_process(self) -> None: timestamp = datetime.now() @@ -898,15 +885,17 @@ def test_project_bypass(self) -> None: _, replacement = meta_and_replacement assert replacement is not None - set_config("replacements_bypass_projects", f"[{self.project_id + 1}]") - meta_and_replacement = self.replacer.process_message(self._wrap(message)) - assert meta_and_replacement is not None - _, replacement = meta_and_replacement - assert replacement is not None - - set_config( - "replacements_bypass_projects", f"[{self.project_id + 1},{self.project_id}]" - ) - meta_and_replacement = self.replacer.process_message(self._wrap(message)) - assert meta_and_replacement is None - delete_config("replacements_bypass_projects") + with override_options( + "snuba", {"replacements_bypass_projects": f"[{self.project_id + 1}]"} + ): + meta_and_replacement = self.replacer.process_message(self._wrap(message)) + assert meta_and_replacement is not None + _, replacement = meta_and_replacement + assert replacement is not None + + with override_options( + "snuba", + {"replacements_bypass_projects": f"[{self.project_id + 1},{self.project_id}]"}, + ): + meta_and_replacement = self.replacer.process_message(self._wrap(message)) + assert meta_and_replacement is None diff --git a/tests/datasets/test_events.py b/tests/datasets/test_events.py index b3e5637edfc..23acd0819cd 100644 --- a/tests/datasets/test_events.py +++ b/tests/datasets/test_events.py @@ -1,6 +1,6 @@ import pytest +from sentry_options.testing import override_options -from snuba import state from snuba.clickhouse.columns import ColumnSet from snuba.clickhouse.translators.snuba.mapping import TranslationMappers from snuba.clusters.cluster import ClickhouseClientSettings @@ -52,9 +52,8 @@ def test_tags_hash_map(self) -> None: @pytest.mark.redis_db +@override_options("snuba", {"enable_events_readonly_table": True}) def test_storage_selector() -> None: - state.set_config("enable_events_readonly_table", True) - storage = get_storage(StorageKey.ERRORS) storage_ro = get_storage(StorageKey.ERRORS_RO) storage_connections = [ diff --git a/tests/datasets/test_transaction_processor.py b/tests/datasets/test_transaction_processor.py index c5bd2695cc3..c016ae05261 100644 --- a/tests/datasets/test_transaction_processor.py +++ b/tests/datasets/test_transaction_processor.py @@ -6,6 +6,7 @@ from unittest.mock import ANY import pytest +from sentry_options.testing import override_options from snuba import settings from snuba.consumers.types import KafkaMessageMetadata @@ -13,7 +14,6 @@ TransactionsMessageProcessor, ) from snuba.processor import InsertBatch -from snuba.state import set_config @dataclass @@ -221,9 +221,7 @@ def build_result(self, meta: KafkaMessageMetadata) -> Mapping[str, Any]: start_timestamp = datetime.utcfromtimestamp(self.start_timestamp) finish_timestamp = datetime.utcfromtimestamp(self.timestamp) - spans = sorted( - [(self.op, int("a" * 16, 16), 1.2345), ("http", int("b" * 16, 16), 0.1234)] - ) + spans = sorted([(self.op, int("a" * 16, 16), 1.2345), ("http", int("b" * 16, 16), 0.1234)]) ret = { "deleted": 0, @@ -240,9 +238,7 @@ def build_result(self, meta: KafkaMessageMetadata) -> Mapping[str, Any]: "start_ms": int(start_timestamp.microsecond / 1000), "finish_ts": finish_timestamp, "finish_ms": int(finish_timestamp.microsecond / 1000), - "duration": int( - (finish_timestamp - start_timestamp).total_seconds() * 1000 - ), + "duration": int((finish_timestamp - start_timestamp).total_seconds() * 1000), "platform": self.platform, "environment": self.environment, "release": self.release, @@ -334,9 +330,7 @@ def __get_transaction_event(self) -> TransactionEvent: op="navigation", timestamp=finish, start_timestamp=start, - received=( - datetime.now(tz=timezone.utc) - timedelta(seconds=15) - ).timestamp(), + received=(datetime.now(tz=timezone.utc) - timedelta(seconds=15)).timestamp(), platform="python", dist="", user_name="me", @@ -369,9 +363,7 @@ def test_skip_non_transactions(self) -> None: # Force an invalid event payload[2]["data"]["type"] = "error" - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) processor = TransactionsMessageProcessor() assert processor.process_message(payload, meta) is None @@ -381,9 +373,7 @@ def test_missing_trace_context(self) -> None: # Force an invalid event del payload[2]["data"]["contexts"] - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) processor = TransactionsMessageProcessor() assert processor.process_message(payload, meta) is None @@ -393,23 +383,19 @@ def test_base_process(self) -> None: message = self.__get_transaction_event() - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) assert TransactionsMessageProcessor().process_message( message.serialize(), meta ) == InsertBatch([message.build_result(meta)], ANY) settings.TRANSACT_SKIP_CONTEXT_STORE = old_skip_context + @override_options("snuba", {"max_spans_per_transaction": 1}) def test_too_many_spans(self) -> None: old_skip_context = settings.TRANSACT_SKIP_CONTEXT_STORE settings.TRANSACT_SKIP_CONTEXT_STORE = {1: {"experiments"}} - set_config("max_spans_per_transaction", 1) message = self.__get_transaction_event() - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) payload = message.serialize() @@ -421,9 +407,9 @@ def test_too_many_spans(self) -> None: result["spans.exclusive_time"] = [0] result["spans.exclusive_time_32"] = [1.2345] - assert TransactionsMessageProcessor().process_message( - payload, meta - ) == InsertBatch([result], ANY) + assert TransactionsMessageProcessor().process_message(payload, meta) == InsertBatch( + [result], ANY + ) settings.TRANSACT_SKIP_CONTEXT_STORE = old_skip_context def test_missing_transaction_source(self) -> None: @@ -436,9 +422,7 @@ def test_missing_transaction_source(self) -> None: # Remove transaction_info del payload_wo_transaction_info[2]["data"]["transaction_info"] - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) actual_message = TransactionsMessageProcessor().process_message( payload_wo_transaction_info, meta ) @@ -447,12 +431,8 @@ def test_missing_transaction_source(self) -> None: # Remove transaction_info.source del payload_wo_source[2]["data"]["transaction_info"]["source"] - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) - actual_message = TransactionsMessageProcessor().process_message( - payload_wo_source, meta - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) + actual_message = TransactionsMessageProcessor().process_message(payload_wo_source, meta) assert actual_message.rows[0]["transaction_source"] == "" def test_app_ctx_none(self) -> None: @@ -462,9 +442,7 @@ def test_app_ctx_none(self) -> None: message = self.__get_transaction_event() message.has_app_ctx = False - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) assert TransactionsMessageProcessor().process_message( message.serialize(), meta ) == InsertBatch([message.build_result(meta)], ANY) @@ -478,14 +456,10 @@ def test_replay_id_as_tag(self) -> None: message = self.__get_transaction_event() payload = message.serialize() - payload[2]["data"]["tags"].append( - ["replayId", "d2731f8ed8934c6fa5253e450915aa12"] - ) + payload[2]["data"]["tags"].append(["replayId", "d2731f8ed8934c6fa5253e450915aa12"]) del payload[2]["data"]["contexts"]["replay"] - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) result = message.build_result(meta) # when the replay_id is sent as a tag instead of a context, @@ -495,9 +469,9 @@ def test_replay_id_as_tag(self) -> None: result["tags.key"].insert(1, "replayId") result["tags.value"].insert(1, "d2731f8ed8934c6fa5253e450915aa12") - assert TransactionsMessageProcessor().process_message( - payload, meta - ) == InsertBatch([result], ANY) + assert TransactionsMessageProcessor().process_message(payload, meta) == InsertBatch( + [result], ANY + ) def test_replay_id_as_tag_and_context(self) -> None: """ @@ -509,13 +483,9 @@ def test_replay_id_as_tag_and_context(self) -> None: message = self.__get_transaction_event() payload = message.serialize() - payload[2]["data"]["tags"].append( - ["replayId", "d2731f8ed8934c6fa5253e450915aa12"] - ) + payload[2]["data"]["tags"].append(["replayId", "d2731f8ed8934c6fa5253e450915aa12"]) - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) result = message.build_result(meta) # when the replay_id is sent as a tag instead of a context, @@ -525,9 +495,9 @@ def test_replay_id_as_tag_and_context(self) -> None: result["tags.key"].insert(1, "replayId") result["tags.value"].insert(1, "d2731f8ed8934c6fa5253e450915aa12") - assert TransactionsMessageProcessor().process_message( - payload, meta - ) == InsertBatch([result], ANY) + assert TransactionsMessageProcessor().process_message(payload, meta) == InsertBatch( + [result], ANY + ) def test_replay_id_as_invalid_tag(self) -> None: """ @@ -541,9 +511,7 @@ def test_replay_id_as_invalid_tag(self) -> None: del payload[2]["data"]["contexts"]["replay"] payload[2]["data"]["tags"].append(["replayId", "I_AM_NOT_A_UUID"]) - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) result = message.build_result(meta) del result["replay_id"] @@ -552,9 +520,9 @@ def test_replay_id_as_invalid_tag(self) -> None: result["tags.key"].insert(1, "replayId") result["tags.value"].insert(1, "I_AM_NOT_A_UUID") - assert TransactionsMessageProcessor().process_message( - payload, meta - ) == InsertBatch([result], ANY) + assert TransactionsMessageProcessor().process_message(payload, meta) == InsertBatch( + [result], ANY + ) def test_trace_data_is_none(self) -> None: """ @@ -566,9 +534,7 @@ def test_trace_data_is_none(self) -> None: # Force an invalid event payload[2]["data"]["contexts"]["trace"]["data"] = None - meta = KafkaMessageMetadata( - offset=1, partition=2, timestamp=datetime(1970, 1, 1) - ) + meta = KafkaMessageMetadata(offset=1, partition=2, timestamp=datetime(1970, 1, 1)) result = message.build_result(meta) diff --git a/tests/lw_deletions/test_lw_deletions.py b/tests/lw_deletions/test_lw_deletions.py index 8bb2ff33fd3..670b166af32 100644 --- a/tests/lw_deletions/test_lw_deletions.py +++ b/tests/lw_deletions/test_lw_deletions.py @@ -8,6 +8,7 @@ import rapidjson from arroyo.backends.kafka import KafkaPayload from arroyo.types import BrokerValue, Message, Partition, Topic +from sentry_options.testing import override_options from snuba import state from snuba.clusters.cluster import ClickhouseNode @@ -518,6 +519,7 @@ def _make_eap_message( @patch("snuba.lw_deletions.strategy._num_parts_currently_mutating", return_value=1) @patch("snuba.lw_deletions.strategy._execute_query") @pytest.mark.redis_db +@override_options("snuba", {"org_ids_delete_allowlist": "1"}) def test_allowlist_partial_batch(mock_execute: Mock, mock_num_mutations: Mock) -> None: """ Batch with 2 conditions (org 1 and org 2), allowlist = "1". @@ -528,8 +530,6 @@ def test_allowlist_partial_batch(mock_execute: Mock, mock_num_mutations: Mock) - metrics = Mock() storage = get_writable_storage(StorageKey("eap_items")) - state.set_config("org_ids_delete_allowlist", "1") - format_query = FormatQuery(commit_step, storage, EAPItemsFormatter(), metrics) # Build a batch with two messages: org 1 (allowed) and org 2 (not allowed) @@ -558,6 +558,7 @@ def test_allowlist_partial_batch(mock_execute: Mock, mock_num_mutations: Mock) - @patch("snuba.lw_deletions.strategy._num_parts_currently_mutating", return_value=1) @patch("snuba.lw_deletions.strategy._execute_query") @pytest.mark.redis_db +@override_options("snuba", {"org_ids_delete_allowlist": "999"}) def test_allowlist_all_blocked(mock_execute: Mock, mock_num_mutations: Mock) -> None: """ All conditions have unallowed org IDs. _execute_query should not be called, @@ -567,8 +568,6 @@ def test_allowlist_all_blocked(mock_execute: Mock, mock_num_mutations: Mock) -> metrics = Mock() storage = get_writable_storage(StorageKey("eap_items")) - state.set_config("org_ids_delete_allowlist", "999") - format_query = FormatQuery(commit_step, storage, EAPItemsFormatter(), metrics) msg1 = _make_eap_message(5, {"organization_id": [1], "project_id": [1]}) @@ -597,6 +596,7 @@ def test_allowlist_all_blocked(mock_execute: Mock, mock_num_mutations: Mock) -> @patch("snuba.lw_deletions.strategy._num_parts_currently_mutating", return_value=1) @patch("snuba.lw_deletions.strategy._execute_query") @pytest.mark.redis_db +@override_options("snuba", {"org_ids_delete_allowlist": "1,2"}) def test_allowlist_all_allowed(mock_execute: Mock, mock_num_mutations: Mock) -> None: """ All conditions have allowed org IDs. Normal execution, no delete_skipped. @@ -605,8 +605,6 @@ def test_allowlist_all_allowed(mock_execute: Mock, mock_num_mutations: Mock) -> metrics = Mock() storage = get_writable_storage(StorageKey("eap_items")) - state.set_config("org_ids_delete_allowlist", "1,2") - format_query = FormatQuery(commit_step, storage, EAPItemsFormatter(), metrics) msg1 = _make_eap_message(5, {"organization_id": [1], "project_id": [1]}) diff --git a/tests/lw_deletions/test_off_peak.py b/tests/lw_deletions/test_off_peak.py index 6377fa533ca..8c2a23e2eb0 100644 --- a/tests/lw_deletions/test_off_peak.py +++ b/tests/lw_deletions/test_off_peak.py @@ -1,6 +1,8 @@ from __future__ import annotations +from contextlib import contextmanager from datetime import datetime, timedelta, timezone +from typing import Generator from unittest.mock import MagicMock import pytest @@ -8,8 +10,8 @@ from arroyo.backends.kafka import KafkaPayload from arroyo.processing.strategies.abstract import MessageRejected from arroyo.types import BrokerValue, Message, Partition, Topic +from sentry_options.testing import override_options -from snuba import state from snuba.lw_deletions.off_peak import OffPeakProcessingStrategy from snuba.state import get_raw_configs @@ -54,10 +56,17 @@ def _make_strategy( return strategy, next_step, metrics -def _set_offpeak_config(start: int, end: int) -> None: - state.set_config("lw_deletions_offpeak_enabled", 1) - state.set_config("lw_deletions_offpeak_start", start) - state.set_config("lw_deletions_offpeak_end", end) +@contextmanager +def _offpeak_config(start: int, end: int) -> Generator[None, None, None]: + with override_options( + "snuba", + { + "lw_deletions_offpeak_enabled": True, + "lw_deletions_offpeak_start": start, + "lw_deletions_offpeak_end": end, + }, + ): + yield @pytest.mark.redis_db @@ -70,8 +79,8 @@ def test_messages_pass_through(self) -> None: strategy.submit(msg) next_step.submit.assert_called_once_with(msg) + @override_options("snuba", {"lw_deletions_offpeak_enabled": False}) def test_messages_pass_through_when_explicitly_disabled(self) -> None: - state.set_config("lw_deletions_offpeak_enabled", 0) strategy, next_step, _ = _make_strategy() msg = _make_message() strategy.submit(msg) @@ -83,15 +92,13 @@ class TestOffPeakSameDayWindow: """Window like 2-8 (2am to 8am UTC).""" def test_within_window_passes(self) -> None: - with time_machine.travel(_tomorrow_at(5), tick=False): - _set_offpeak_config(start=2, end=8) + with time_machine.travel(_tomorrow_at(5), tick=False), _offpeak_config(start=2, end=8): strategy, next_step, _ = _make_strategy() strategy.submit(_make_message()) next_step.submit.assert_called_once() def test_outside_window_rejects(self) -> None: - with time_machine.travel(_tomorrow_at(12), tick=False): - _set_offpeak_config(start=2, end=8) + with time_machine.travel(_tomorrow_at(12), tick=False), _offpeak_config(start=2, end=8): strategy, next_step, metrics = _make_strategy() with pytest.raises(MessageRejected): strategy.submit(_make_message()) @@ -99,15 +106,13 @@ def test_outside_window_rejects(self) -> None: metrics.increment.assert_called_with("off_peak_rejected") def test_at_start_boundary_passes(self) -> None: - with time_machine.travel(_tomorrow_at(2), tick=False): - _set_offpeak_config(start=2, end=8) + with time_machine.travel(_tomorrow_at(2), tick=False), _offpeak_config(start=2, end=8): strategy, next_step, _ = _make_strategy() strategy.submit(_make_message()) next_step.submit.assert_called_once() def test_at_end_boundary_rejects(self) -> None: - with time_machine.travel(_tomorrow_at(8), tick=False): - _set_offpeak_config(start=2, end=8) + with time_machine.travel(_tomorrow_at(8), tick=False), _offpeak_config(start=2, end=8): strategy, next_step, _ = _make_strategy() with pytest.raises(MessageRejected): strategy.submit(_make_message()) @@ -118,22 +123,19 @@ class TestOffPeakMidnightSpanningWindow: """Window like 22-6 (10pm to 6am UTC, spanning midnight).""" def test_before_midnight_passes(self) -> None: - with time_machine.travel(_tomorrow_at(23), tick=False): - _set_offpeak_config(start=22, end=6) + with time_machine.travel(_tomorrow_at(23), tick=False), _offpeak_config(start=22, end=6): strategy, next_step, _ = _make_strategy() strategy.submit(_make_message()) next_step.submit.assert_called_once() def test_after_midnight_passes(self) -> None: - with time_machine.travel(_tomorrow_at(3), tick=False): - _set_offpeak_config(start=22, end=6) + with time_machine.travel(_tomorrow_at(3), tick=False), _offpeak_config(start=22, end=6): strategy, next_step, _ = _make_strategy() strategy.submit(_make_message()) next_step.submit.assert_called_once() def test_during_day_rejects(self) -> None: - with time_machine.travel(_tomorrow_at(14), tick=False): - _set_offpeak_config(start=22, end=6) + with time_machine.travel(_tomorrow_at(14), tick=False), _offpeak_config(start=22, end=6): strategy, next_step, _ = _make_strategy() with pytest.raises(MessageRejected): strategy.submit(_make_message()) @@ -144,8 +146,7 @@ class TestOffPeakSameStartEnd: """When start == end, never off-peak (disables processing).""" def test_always_rejects(self) -> None: - with time_machine.travel(_tomorrow_at(5), tick=False): - _set_offpeak_config(start=5, end=5) + with time_machine.travel(_tomorrow_at(5), tick=False), _offpeak_config(start=5, end=5): strategy, next_step, _ = _make_strategy() with pytest.raises(MessageRejected): strategy.submit(_make_message()) diff --git a/tests/pipeline/test_execution_stage.py b/tests/pipeline/test_execution_stage.py index 0fec3519970..e6c1275c1d2 100644 --- a/tests/pipeline/test_execution_stage.py +++ b/tests/pipeline/test_execution_stage.py @@ -1,9 +1,9 @@ import uuid import pytest +from sentry_options.testing import override_options from snuba import settings as snubasettings -from snuba import state from snuba.attribution import get_app_id from snuba.attribution.attribution_info import AttributionInfo from snuba.clickhouse.columns import ColumnSet @@ -236,16 +236,15 @@ def test_max_query_size_bytes(ch_query: Query) -> None: timer = Timer("test") metadata = get_fake_metadata() - state.set_config(MAX_QUERY_SIZE_BYTES_CONFIG, 1) - - res = ExecutionStage(attinfo, query_metadata=metadata).execute( - QueryPipelineResult( - data=ch_query, - query_settings=settings, - timer=timer, - error=None, + with override_options("snuba", {MAX_QUERY_SIZE_BYTES_CONFIG: 1}): + res = ExecutionStage(attinfo, query_metadata=metadata).execute( + QueryPipelineResult( + data=ch_query, + query_settings=settings, + timer=timer, + error=None, + ) ) - ) assert res.data is None assert isinstance(res.error, QueryException) @@ -267,18 +266,22 @@ def test_disable_max_query_size_check(ch_query: Query) -> None: else "test_cluster" ) - # Lowering this should make the query too big... - state.set_config(MAX_QUERY_SIZE_BYTES_CONFIG, 1) - # Unless we disable the check for this cluster. - state.set_config(DISABLE_MAX_QUERY_SIZE_CHECK_FOR_CLUSTERS_CONFIG, cluster_name) - - res = ExecutionStage(attinfo, query_metadata=metadata).execute( - QueryPipelineResult( - data=ch_query, - query_settings=settings, - timer=timer, - error=None, + # Lowering this should make the query too big, unless we disable the check + # for this cluster. + with override_options( + "snuba", + { + MAX_QUERY_SIZE_BYTES_CONFIG: 1, + DISABLE_MAX_QUERY_SIZE_CHECK_FOR_CLUSTERS_CONFIG: cluster_name, + }, + ): + res = ExecutionStage(attinfo, query_metadata=metadata).execute( + QueryPipelineResult( + data=ch_query, + query_settings=settings, + timer=timer, + error=None, + ) ) - ) assert res.data diff --git a/tests/query/parser/validation/test_functions.py b/tests/query/parser/validation/test_functions.py index 50d4a053169..6991eabf2c2 100644 --- a/tests/query/parser/validation/test_functions.py +++ b/tests/query/parser/validation/test_functions.py @@ -4,9 +4,9 @@ from unittest.mock import MagicMock import pytest +from sentry_options.testing import override_options import snuba.query.parser.validation.functions as functions -from snuba import state from snuba.clickhouse.columns import ColumnSet from snuba.datasets.entities.entity_key import EntityKey from snuba.datasets.entities.factory import get_entity @@ -108,9 +108,9 @@ def test_functions( @pytest.mark.parametrize("expression, should_raise", test_expressions[:1]) @pytest.mark.redis_db +@override_options("snuba", {"function-validator.enabled": True}) def test_invalid_function_name(expression: FunctionCall, should_raise: bool) -> None: data_source = QueryEntity(EntityKey.EVENTS, ColumnSet([])) - state.set_config("function-validator.enabled", True) with pytest.raises(InvalidExpressionException): FunctionCallsValidator().validate(expression, data_source) @@ -118,9 +118,9 @@ def test_invalid_function_name(expression: FunctionCall, should_raise: bool) -> @pytest.mark.parametrize("expression, should_raise", test_expressions) @pytest.mark.redis_db +@override_options("snuba", {"function-validator.enabled": True}) def test_allowed_functions_validator(expression: FunctionCall, should_raise: bool) -> None: data_source = QueryEntity(EntityKey.EVENTS, ColumnSet([])) - state.set_config("function-validator.enabled", True) if should_raise: with pytest.raises(InvalidFunctionCall): diff --git a/tests/query/processors/test_clickhouse_settings_override.py b/tests/query/processors/test_clickhouse_settings_override.py index c5b20d9681c..2e56bf577b1 100644 --- a/tests/query/processors/test_clickhouse_settings_override.py +++ b/tests/query/processors/test_clickhouse_settings_override.py @@ -1,8 +1,8 @@ from typing import Any, MutableMapping import pytest +from sentry_options.testing import override_options -from snuba import state from snuba.clickhouse.columns import ColumnSet, DateTime, String from snuba.clickhouse.columns import SchemaModifiers as Modifiers from snuba.clickhouse.query import Query @@ -96,11 +96,11 @@ def test_per_query_settings() -> None: @pytest.mark.redis_db +@override_options( + "snuba", + {"ignore_clickhouse_settings_override": "max_execution_time,timeout_overflow_mode"}, +) def test_ignore_clickhouse_settings_overrides() -> None: - state.set_config( - "ignore_clickhouse_settings_override", - "max_execution_time,timeout_overflow_mode", - ) query = Query( Table( "discover", diff --git a/tests/query/processors/test_mandatory_condition_enforcer.py b/tests/query/processors/test_mandatory_condition_enforcer.py index 43ef45c64ea..1a0b663e3a0 100644 --- a/tests/query/processors/test_mandatory_condition_enforcer.py +++ b/tests/query/processors/test_mandatory_condition_enforcer.py @@ -1,6 +1,7 @@ from datetime import datetime import pytest +from sentry_options.testing import override_options from snuba.clickhouse.columns import ColumnSet from snuba.clickhouse.query import Query @@ -22,7 +23,6 @@ MandatoryConditionEnforcer, ) from snuba.query.query_settings import HTTPQuerySettings -from snuba.state import set_config TABLE = Table("errors", ColumnSet([]), storage_key=StorageKey("errors")) @@ -142,8 +142,8 @@ @pytest.mark.parametrize("query, valid, org_id_enforcer", test_data) @pytest.mark.redis_db +@override_options("snuba", {"mandatory_condition_enforce": True}) def test_condition_enforcer(query: Query, valid: bool, org_id_enforcer: OrgIdEnforcer) -> None: - set_config("mandatory_condition_enforce", 1) query_settings = HTTPQuerySettings(consistent=True) processor = MandatoryConditionEnforcer([org_id_enforcer, ProjectIdEnforcer()]) if valid: diff --git a/tests/query/processors/test_uniq_in_select_and_having.py b/tests/query/processors/test_uniq_in_select_and_having.py index 6a9a46a3c5e..ac70e8839f4 100644 --- a/tests/query/processors/test_uniq_in_select_and_having.py +++ b/tests/query/processors/test_uniq_in_select_and_having.py @@ -1,6 +1,7 @@ from copy import deepcopy import pytest +from sentry_options.testing import override_options from snuba.clickhouse.query import Query as ClickhouseQuery from snuba.query.expressions import Column, FunctionCall, Literal @@ -9,7 +10,6 @@ UniqInSelectAndHavingProcessor, ) from snuba.query.query_settings import HTTPQuerySettings -from snuba.state import set_config from tests.query.processors.query_builders import build_query @@ -81,16 +81,16 @@ def uniq_expression(alias: str = None, column_name: str = "user") -> FunctionCal @pytest.mark.parametrize("input_query", deepcopy(INVALID_QUERY_CASES)) @pytest.mark.redis_db +@override_options("snuba", {"throw_on_uniq_select_and_having": True}) def test_invalid_uniq_queries(input_query: ClickhouseQuery) -> None: - set_config("throw_on_uniq_select_and_having", True) with pytest.raises(MismatchedAggregationException): UniqInSelectAndHavingProcessor().process_query(input_query, HTTPQuerySettings()) @pytest.mark.parametrize("input_query", deepcopy(VALID_QUERY_CASES)) @pytest.mark.redis_db +@override_options("snuba", {"throw_on_uniq_select_and_having": True}) def test_valid_uniq_queries(input_query: ClickhouseQuery) -> None: - set_config("throw_on_uniq_select_and_having", True) og_query = deepcopy(input_query) UniqInSelectAndHavingProcessor().process_query(input_query, HTTPQuerySettings()) # query should not change diff --git a/tests/replacer/test_cluster_replacements.py b/tests/replacer/test_cluster_replacements.py index 1eff00086ff..dd8eda0de44 100644 --- a/tests/replacer/test_cluster_replacements.py +++ b/tests/replacer/test_cluster_replacements.py @@ -15,6 +15,7 @@ ) import pytest +from sentry_options.testing import override_options from snuba.clickhouse.native import ClickhousePool from snuba.clusters import cluster @@ -189,20 +190,22 @@ def test_write_each_node( Test the execution of replacement queries on both storage nodes and query nodes. """ - set_config("write_node_replacements_global", write_node_replacements_global) - override_func = request.getfixturevalue(override_fixture) - test_cluster = override_func(True) - - replacer = ReplacerWorker( - get_writable_storage(StorageKey.ERRORS), - "consumer_group", - DummyMetricsBackend(), - ) + with override_options( + "snuba", {"write_node_replacements_global": write_node_replacements_global} + ): + override_func = request.getfixturevalue(override_fixture) + test_cluster = override_func(True) + + replacer = ReplacerWorker( + get_writable_storage(StorageKey.ERRORS), + "consumer_group", + DummyMetricsBackend(), + ) - replacer.flush_batch([(ReplacementMessageMetadata(0, 0, ""), DummyReplacement())]) + replacer.flush_batch([(ReplacementMessageMetadata(0, 0, ""), DummyReplacement())]) - queries = test_cluster.get_queries() - assert queries == expected_queries + queries = test_cluster.get_queries() + assert queries == expected_queries @pytest.mark.redis_db diff --git a/tests/state/test_cache.py b/tests/state/test_cache.py index 8d8460c4631..20c544d65cd 100644 --- a/tests/state/test_cache.py +++ b/tests/state/test_cache.py @@ -13,10 +13,10 @@ from redis import RedisError, ResponseError from redis.exceptions import ReadOnlyError from redis.exceptions import TimeoutError as RedisTimeoutError +from sentry_options.testing import override_options from sentry_redis_tools.failover_redis import FailoverRedis from snuba.redis import RedisClientKey, get_redis_client -from snuba.state import set_config from snuba.state.cache.abstract import Cache from snuba.state.cache.redis.backend import RedisCache from snuba.utils.codecs import ExceptionAwareCodec @@ -109,8 +109,8 @@ def noop(value: int) -> None: @pytest.mark.redis_db +@override_options("snuba", {"read_through_cache.short_circuit": True}) def test_short_circuit(backend: Cache[bytes]) -> None: - set_config("read_through_cache.short_circuit", 1) key = "key" value = b"value" function = mock.MagicMock(return_value=value) @@ -208,7 +208,9 @@ def worker() -> bytes: with pytest.raises(ReadThroughCustomException) as excinfo: waiter.result() - assert excinfo.value.message == "error" + # mypy infers excinfo.value as the ExceptionInfo TypeVar default rather than + # the locally-defined exception class, so .message is not visible to it. + assert excinfo.value.message == "error" # type: ignore[attr-defined] @pytest.mark.parametrize( diff --git a/tests/state/test_sentry_options.py b/tests/state/test_sentry_options.py new file mode 100644 index 00000000000..e7d4ffeeb77 --- /dev/null +++ b/tests/state/test_sentry_options.py @@ -0,0 +1,78 @@ +from unittest import mock + +from sentry_options.testing import override_options + +from snuba.state.sentry_options import ( + SNUBA_OPTIONS_NAMESPACE, + get_bool_option, + get_float_option, + get_int_option, + get_option, + get_str_option, +) + + +def test_get_option_returns_schema_default() -> None: + # `enable_any_attribute_filter` has a schema default of `true`. With + # sentry-options initialized (see tests/conftest.py) we read that schema + # default rather than the fallback passed here. + assert get_option("enable_any_attribute_filter", False) is True + + +def test_override_options_changes_value() -> None: + with override_options(SNUBA_OPTIONS_NAMESPACE, {"enable_any_attribute_filter": False}): + assert get_option("enable_any_attribute_filter", True) is False + # the override is scoped to the context manager; the default is restored. + assert get_option("enable_any_attribute_filter", False) is True + + +def test_unknown_option_falls_back_to_default() -> None: + # Keys absent from the schema raise UnknownOptionError internally, which + # get_option swallows in favor of the caller-supplied default. + assert get_option("option_that_does_not_exist", "fallback") == "fallback" + + +def test_typed_accessors_return_schema_defaults() -> None: + # Each typed accessor returns the schema default with the right Python type. + assert get_bool_option("aggregation_deprecation_enabled", False) is True + assert get_int_option("default_tier", 0) == 1 + assert get_int_option("export_trace_items_default_page_size", 0) == 10000 + assert get_float_option("rpc_logging_sample_rate", 1.0) == 0.0 + assert get_str_option("ExecutionStage.disable_max_query_size_check_for_clusters", "x") == "" + + +def test_typed_accessors_honor_overrides() -> None: + with override_options( + SNUBA_OPTIONS_NAMESPACE, + { + "aggregation_deprecation_enabled": False, + "default_tier": 8, + "rpc_logging_sample_rate": 0.25, + }, + ): + assert get_bool_option("aggregation_deprecation_enabled", True) is False + assert get_int_option("default_tier", 1) == 8 + assert get_float_option("rpc_logging_sample_rate", 0.0) == 0.25 + + +def test_typed_accessors_fall_back_on_unknown_option() -> None: + # Unknown keys fall back to the caller-supplied default at the right type. + assert get_bool_option("missing_bool", True) is True + assert get_int_option("missing_int", 7) == 7 + assert get_float_option("missing_float", 1.5) == 1.5 + assert get_str_option("missing_str", "fallback") == "fallback" + + +def test_unexpected_error_falls_back_to_default() -> None: + # The client should only ever raise OptionsError, but a non-OptionsError + # escaping from the client must not crash hot query paths: get_option (and + # the typed accessors built on it) honor the "any reason" fallback contract. + with mock.patch( + "snuba.state.sentry_options.sentry_options.options", + side_effect=RuntimeError("boom"), + ): + assert get_option("enable_any_attribute_filter", "fallback") == "fallback" + assert get_bool_option("enable_any_attribute_filter", True) is True + assert get_int_option("default_tier", 7) == 7 + assert get_float_option("rpc_logging_sample_rate", 1.5) == 1.5 + assert get_str_option("some_str", "fallback") == "fallback" diff --git a/tests/subscriptions/test_builder_mode_state.py b/tests/subscriptions/test_builder_mode_state.py index 2837ad36ce5..d10144eac3c 100644 --- a/tests/subscriptions/test_builder_mode_state.py +++ b/tests/subscriptions/test_builder_mode_state.py @@ -2,8 +2,9 @@ from typing import Sequence, Tuple import pytest +from sentry_options.testing import override_options -from snuba import settings, state +from snuba import settings from snuba.subscriptions.data import Subscription from snuba.subscriptions.scheduler import TaskBuilderMode, TaskBuilderModeState from tests.subscriptions.subscriptions_utils import build_subscription @@ -99,11 +100,11 @@ def test_state_changes( ) -> None: prev_threshold = settings.MAX_RESOLUTION_FOR_JITTER settings.MAX_RESOLUTION_FOR_JITTER = 300 - state.set_config("subscription_primary_task_builder", general_mode) - mode_state = TaskBuilderModeState() - modes = [ - mode_state.get_current_mode(subscription, timestamp) - for subscription, timestamp in subscriptions - ] - assert modes == expected_modes + with override_options("snuba", {"subscription_primary_task_builder": general_mode}): + mode_state = TaskBuilderModeState() + modes = [ + mode_state.get_current_mode(subscription, timestamp) + for subscription, timestamp in subscriptions + ] + assert modes == expected_modes settings.MAX_RESOLUTION_FOR_JITTER = prev_threshold diff --git a/tests/subscriptions/test_executor_consumer.py b/tests/subscriptions/test_executor_consumer.py index 9f1b90a5f78..dc571f5f618 100644 --- a/tests/subscriptions/test_executor_consumer.py +++ b/tests/subscriptions/test_executor_consumer.py @@ -16,8 +16,8 @@ from arroyo.types import BrokerValue, Message, Partition, Topic from arroyo.utils.clock import MockedClock from confluent_kafka.admin import AdminClient +from sentry_options.testing import override_options -from snuba import state from snuba.datasets.entities.entity_key import EntityKey from snuba.datasets.entities.factory import get_entity from snuba.datasets.factory import get_dataset @@ -341,9 +341,8 @@ def test_poll_skips_non_retryable_query_exception() -> None: @pytest.mark.redis_db @pytest.mark.clickhouse_db +@override_options("snuba", {"executor_queue_size_factor": 1}) def test_too_many_concurrent_queries() -> None: - state.set_config("executor_queue_size_factor", 1) - strategy = ExecuteQuery( dataset=get_dataset("events"), entity_names=["events"], diff --git a/tests/subscriptions/test_scheduler.py b/tests/subscriptions/test_scheduler.py index 9bbba7198c1..35b8203d59e 100644 --- a/tests/subscriptions/test_scheduler.py +++ b/tests/subscriptions/test_scheduler.py @@ -3,8 +3,8 @@ from typing import Callable, Collection, Optional, Tuple import pytest +from sentry_options.testing import override_options -from snuba import state from snuba.datasets.entities.entity_key import EntityKey from snuba.datasets.entities.factory import get_entity from snuba.redis import RedisClientKey, get_redis_client @@ -93,8 +93,8 @@ def run_test( assert result == expected @pytest.mark.redis_db + @override_options("snuba", {"subscription_primary_task_builder": "immediate"}) def test_simple(self) -> None: - state.set_config("subscription_primary_task_builder", "immediate") subscription = self.build_subscription(timedelta(minutes=1)) start = timedelta(minutes=-10) end = timedelta(minutes=0) @@ -169,8 +169,8 @@ def test_subscription_resolution_larger_than_interval(self) -> None: ) @pytest.mark.redis_db + @override_options("snuba", {"subscription_primary_task_builder": "immediate"}) def test_subscription_resolution_larger_than_tiny_interval(self) -> None: - state.set_config("subscription_primary_task_builder", "immediate") subscription = self.build_subscription(timedelta(minutes=1)) start = timedelta(seconds=-1) end = timedelta(seconds=1) @@ -228,8 +228,8 @@ def test_multiple_subscriptions(self) -> None: ) @pytest.mark.redis_db + @override_options("snuba", {"subscription_primary_task_builder": "immediate"}) def test_generic_metrics_gauges_does_not_error(self) -> None: - state.set_config("subscription_primary_task_builder", "immediate") subscription = Subscription( SubscriptionIdentifier(self.partition_id, uuid.uuid4()), SnQLSubscriptionData( diff --git a/tests/subscriptions/test_task_builder.py b/tests/subscriptions/test_task_builder.py index 3bf9ae6f21e..fe88a6c2532 100644 --- a/tests/subscriptions/test_task_builder.py +++ b/tests/subscriptions/test_task_builder.py @@ -2,8 +2,8 @@ from typing import Sequence, Tuple import pytest +from sentry_options.testing import override_options -from snuba import state from snuba.datasets.entities.entity_key import EntityKey from snuba.subscriptions.data import ( ScheduledSubscriptionTask, @@ -228,14 +228,14 @@ def test_sequences( subscriptions and validate the proper jitter is applied. state. """ - state.set_config("subscription_primary_task_builder", primary_builder_config) - output = [] - for timestamp, subscription in sequence_in: - ret = builder.get_task( - SubscriptionWithMetadata(EntityKey.EVENTS, subscription, 1), timestamp - ) - if ret: - output.append((timestamp, ret)) + with override_options("snuba", {"subscription_primary_task_builder": primary_builder_config}): + output = [] + for timestamp, subscription in sequence_in: + ret = builder.get_task( + SubscriptionWithMetadata(EntityKey.EVENTS, subscription, 1), timestamp + ) + if ret: + output.append((timestamp, ret)) - assert output == task_sequence - assert builder.reset_metrics() == metrics + assert output == task_sequence + assert builder.reset_metrics() == metrics diff --git a/tests/test_api.py b/tests/test_api.py index f0a589360a4..f8240ca07d2 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -11,6 +11,7 @@ import simplejson as json from confluent_kafka.admin import AdminClient from dateutil.parser import parse as parse_datetime +from sentry_options.testing import override_options from sentry_sdk import Client, Hub from snuba import settings, state @@ -28,7 +29,6 @@ from snuba.utils.streams.configuration_builder import get_default_kafka_configuration from snuba.utils.streams.topics import Topic as SnubaTopic from tests.base import BaseApiTest -from tests.conftest import SnubaSetConfig from tests.helpers import write_processed_messages @@ -1470,9 +1470,14 @@ def test_exception_captured_by_sentry(self) -> None: assert len(events) == 1 assert events[0]["exception"]["values"][0]["type"] == "ZeroDivisionError" + @override_options( + "snuba", + { + "read_through_cache.short_circuit": True, + "consistent_override": "test_override=0;another=0.5", + }, + ) def test_consistent(self) -> None: - state.set_config("consistent_override", "test_override=0;another=0.5") - state.set_config("read_through_cache.short_circuit", 1) query_data = { "project": 2, "tenant_ids": {"referrer": "test_query", "organization_id": 1234}, @@ -1490,7 +1495,7 @@ def test_consistent(self) -> None: query_data["tenant_ids"]["referrer"] = "test_override" # type: ignore query = json.dumps(query_data) response = json.loads(self.post(query, referrer="test_override").data) - assert response["stats"]["consistent"] == False + assert not response["stats"]["consistent"] def test_gracefully_handle_multiple_conditions_on_same_column(self) -> None: response = self.post( @@ -1961,5 +1966,6 @@ class TestAPIErrorsRO(TestApi): """ @pytest.fixture(autouse=True) - def use_readonly_table(self, snuba_set_config: SnubaSetConfig) -> None: - snuba_set_config("enable_events_readonly_table", 1) + def use_readonly_table(self) -> Generator[None, None, None]: + with override_options("snuba", {"enable_events_readonly_table": True}): + yield diff --git a/tests/test_discover_api.py b/tests/test_discover_api.py index 5c31dea7190..c6616159a61 100644 --- a/tests/test_discover_api.py +++ b/tests/test_discover_api.py @@ -1,15 +1,15 @@ from datetime import datetime, timedelta, timezone -from typing import Any, Callable, Tuple, Union +from typing import Any, Callable, Generator, Tuple, Union import pytest import simplejson as json +from sentry_options.testing import override_options from snuba.datasets.entities.entity_key import EntityKey from snuba.datasets.entities.factory import get_entity from snuba.datasets.storages.factory import get_writable_storage from snuba.datasets.storages.storage_key import StorageKey from tests.base import BaseApiTest -from tests.conftest import SnubaSetConfig from tests.fixtures import get_raw_event, get_raw_transaction from tests.helpers import write_unprocessed_events @@ -1771,7 +1771,7 @@ def test_symbolicated_in_app(self) -> None: data = json.loads(response.data) assert response.status_code == 200 assert len(data["data"]) == 1 - assert data["data"][0]["symbolicated_in_app"] == True + assert data["data"][0]["symbolicated_in_app"] def test_timestamp_ms_query(self) -> None: response = self.post( @@ -1849,5 +1849,6 @@ class TestDiscoverAPIErrorsRO(TestDiscoverApi): """ @pytest.fixture(autouse=True) - def use_readonly_table(self, snuba_set_config: SnubaSetConfig) -> None: - snuba_set_config("enable_events_readonly_table", 1) + def use_readonly_table(self) -> Generator[None, None, None]: + with override_options("snuba", {"enable_events_readonly_table": True}): + yield diff --git a/tests/test_search_issues_api.py b/tests/test_search_issues_api.py index 804afa5de24..02ea63fb835 100644 --- a/tests/test_search_issues_api.py +++ b/tests/test_search_issues_api.py @@ -5,11 +5,11 @@ import pytest import simplejson as json +from sentry_options.testing import override_options from snuba.core.initialize import initialize_snuba from snuba.datasets.entities.entity_key import EntityKey from snuba.datasets.entities.factory import get_entity -from snuba.state import set_config from tests.base import BaseApiTest from tests.datasets.configuration.utils import ConfigurationTest from tests.helpers import write_unprocessed_events @@ -105,8 +105,8 @@ def delete_query( ) @patch("snuba.web.bulk_delete_query.produce_delete_query") + @override_options("snuba", {"read_through_cache.short_circuit": True}) 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) occurrence_id = str(uuid.uuid4()) group_id = 4 @@ -135,25 +135,25 @@ def test_simple_delete(self, mock_produce_delete: Mock) -> None: write_unprocessed_events(self.events_storage, [evt]) # delete fails when feature flag is off - set_config("storage_deletes_enabled", 0) - response = self.delete_query(group_id) - assert int(int(response.status_code) / 100) != 2 + with override_options("snuba", {"storage_deletes_enabled": False}): + response = self.delete_query(group_id) + assert int(int(response.status_code) / 100) != 2 # delete succeeds when feature flag is on - set_config("storage_deletes_enabled", 1) - response = self.delete_query(group_id) - data = json.loads(response.data) - assert response.status_code == 200, data + with override_options("snuba", {"storage_deletes_enabled": True}): + response = self.delete_query(group_id) + data = json.loads(response.data) + assert response.status_code == 200, data - # check we produce the delete query message - assert mock_produce_delete.call_count == 1 + # check we produce the delete query message + assert mock_produce_delete.call_count == 1 - # check args for delete query message - called_args = mock_produce_delete.call_args[0][0] - assert called_args["storage_name"] == "search_issues" - assert called_args["conditions"]["project_id"] == [3] - assert called_args["conditions"]["group_id"] == [4] - assert called_args["rows_to_delete"] == 1 + # check args for delete query message + called_args = mock_produce_delete.call_args[0][0] + assert called_args["storage_name"] == "search_issues" + assert called_args["conditions"]["project_id"] == [3] + assert called_args["conditions"]["group_id"] == [4] + assert called_args["rows_to_delete"] == 1 def test_bad_delete(self) -> None: res = self.app.delete( diff --git a/tests/test_snql_api.py b/tests/test_snql_api.py index 32dcb4e170e..dc1bbfa0733 100644 --- a/tests/test_snql_api.py +++ b/tests/test_snql_api.py @@ -3,13 +3,13 @@ import uuid from datetime import datetime, timedelta from hashlib import md5 -from typing import Any +from typing import Any, Generator from unittest.mock import MagicMock, patch import pytest import simplejson as json +from sentry_options.testing import override_options -from snuba import state from snuba.configs.configuration import Configuration, ResourceIdentifier from snuba.datasets.entities.entity_key import EntityKey from snuba.datasets.entities.factory import get_entity @@ -27,7 +27,6 @@ from snuba.querylog.query_metadata import QueryStatus from snuba.utils.metrics.backends.testing import get_recorded_metric_calls from tests.base import BaseApiTest -from tests.conftest import SnubaSetConfig from tests.fixtures import get_raw_event, get_raw_transaction from tests.helpers import override_entity_column_validator, write_unprocessed_events @@ -389,8 +388,8 @@ def test_record_queries_on_error( metadata = record_query_mock.call_args[0][0] assert metadata["query_list"][0]["stats"]["error_code"] == 1123 + @override_options("snuba", {"snuba_api_cogs_probability": 1.0}) def test_record_queries_cogs(self) -> None: - state.set_config("snuba_api_cogs_probability", 1.0) with patch("snuba.querylog._record_cogs") as record_cogs_mock: result = json.loads( self.post( @@ -1579,5 +1578,6 @@ class TestSnQLApiErrorsRO(TestSnQLApi): """ @pytest.fixture(autouse=True) - def use_readonly_table(self, snuba_set_config: SnubaSetConfig) -> None: - snuba_set_config("enable_events_readonly_table", 1) + def use_readonly_table(self) -> Generator[None, None, None]: + with override_options("snuba", {"enable_events_readonly_table": True}): + yield diff --git a/tests/web/rpc/test_common.py b/tests/web/rpc/test_common.py index d6ce06c540b..1399d43007b 100644 --- a/tests/web/rpc/test_common.py +++ b/tests/web/rpc/test_common.py @@ -4,6 +4,7 @@ import pytest from google.protobuf import json_format, struct_pb2 from google.protobuf.timestamp_pb2 import Timestamp +from sentry_options.testing import override_options from sentry_protos.snuba.v1.endpoint_trace_item_table_pb2 import ( Column, TraceItemTableRequest, @@ -88,9 +89,9 @@ def test_use_sampling_factor(self, snuba_set_config: SnubaSetConfig) -> None: ) ) ) - snuba_set_config("use_sampling_factor_timestamp_seconds", 10) - assert use_sampling_factor(RequestMeta(start_timestamp=Timestamp(seconds=10))) - assert not use_sampling_factor(RequestMeta(start_timestamp=Timestamp(seconds=9))) + with override_options("snuba", {"use_sampling_factor_timestamp_seconds": 10}): + assert use_sampling_factor(RequestMeta(start_timestamp=Timestamp(seconds=10))) + assert not use_sampling_factor(RequestMeta(start_timestamp=Timestamp(seconds=9))) class TestTraceItemFiltersArrayLike: @@ -1082,3 +1083,31 @@ def test_like_wildcard_matches_present_not_absent(self) -> None: def test_not_like_wildcard_matches_only_absent(self) -> None: # Present rows all `like '%'`, so only the absent key survives NOT LIKE. assert self._execute(ComparisonFilter.OP_NOT_LIKE, value="%") == ["green"] + + +class TestAnyAttributeFilterOption: + """The `enable_any_attribute_filter` sentry-option gates whether + any_attribute_filter is translated into a predicate or treated as + always-true. It replaces the former `enable_any_attribute_filter` + runtime config.""" + + @staticmethod + def _filter() -> TraceItemFilter: + return TraceItemFilter( + any_attribute_filter=AnyAttributeFilter( + op=AnyAttributeFilter.OP_EQUALS, + value=AttributeValue(val_str="foo"), + ) + ) + + def test_enabled_by_default_translates_filter(self) -> None: + # Schema default is true: the filter is translated, not short-circuited. + result = trace_item_filters_to_expression(self._filter(), attribute_key_to_expression) + assert isinstance(result, FunctionCall) + assert result.function_name == "arrayExists" + + def test_disabled_returns_always_true(self) -> None: + with override_options("snuba", {"enable_any_attribute_filter": False}): + result = trace_item_filters_to_expression(self._filter(), attribute_key_to_expression) + assert isinstance(result, Literal) + assert result.value is True diff --git a/tests/web/rpc/v1/routing_strategies/test_outcomes_based.py b/tests/web/rpc/v1/routing_strategies/test_outcomes_based.py index 33a3357db02..d2b8be382e0 100644 --- a/tests/web/rpc/v1/routing_strategies/test_outcomes_based.py +++ b/tests/web/rpc/v1/routing_strategies/test_outcomes_based.py @@ -5,6 +5,7 @@ import pytest from google.protobuf.timestamp_pb2 import Timestamp +from sentry_options.testing import override_options from sentry_protos.snuba.v1.downsampled_storage_pb2 import DownsampledStorageConfig from sentry_protos.snuba.v1.endpoint_get_traces_pb2 import GetTracesRequest from sentry_protos.snuba.v1.endpoint_time_series_pb2 import TimeSeriesRequest @@ -95,15 +96,12 @@ def test_outcomes_based_routing_queries_daily_table() -> None: @pytest.mark.eap @pytest.mark.redis_db +@override_options("snuba", {"enable_long_term_retention_downsampling": True}) def test_item_type_full_retention() -> None: """ Certain item types will not use the long term retention downsampling, find them in ITEM_TYPE_FULL_RETENTION routing_strategies/common.py """ - state.set_config( - "enable_long_term_retention_downsampling", - 1, - ) strategy = OutcomesBasedRoutingStrategy() # request that queries last 50 days of data @@ -130,15 +128,12 @@ def test_item_type_full_retention() -> None: @pytest.mark.eap @pytest.mark.redis_db +@override_options("snuba", {"enable_long_term_retention_downsampling": True}) def test_item_type_full_retention_preprod() -> None: """ PREPROD item type should not use long term retention downsampling, it should always fetch tier1 for its 90 day retention period. """ - state.set_config( - "enable_long_term_retention_downsampling", - 1, - ) strategy = OutcomesBasedRoutingStrategy() # request that queries last 50 days of data @@ -165,11 +160,8 @@ def test_item_type_full_retention_preprod() -> None: @pytest.mark.eap @pytest.mark.redis_db +@override_options("snuba", {"enable_long_term_retention_downsampling": True}) def test_outcomes_based_routing_sampled_data_past_thirty_days() -> None: - state.set_config( - "enable_long_term_retention_downsampling", - 1, - ) strategy = OutcomesBasedRoutingStrategy() end_time = datetime.now(UTC).replace(hour=0, minute=0, second=0, microsecond=0) diff --git a/tests/web/rpc/v1/test_endpoint_get_trace.py b/tests/web/rpc/v1/test_endpoint_get_trace.py index fc625d7edf1..69e66e19947 100644 --- a/tests/web/rpc/v1/test_endpoint_get_trace.py +++ b/tests/web/rpc/v1/test_endpoint_get_trace.py @@ -7,6 +7,7 @@ import pytest from google.protobuf.json_format import MessageToDict from google.protobuf.timestamp_pb2 import Timestamp +from sentry_options.testing import override_options from sentry_protos.snuba.v1.endpoint_get_trace_pb2 import ( GetTraceRequest, GetTraceResponse, @@ -28,10 +29,10 @@ ) from sentry_protos.snuba.v1.trace_item_pb2 import AnyValue, TraceItem -from snuba import state from snuba.datasets.storages.factory import get_storage from snuba.datasets.storages.storage_key import StorageKey from snuba.settings import ENABLE_TRACE_PAGINATION_DEFAULT +from snuba.state.sentry_options import get_bool_option from snuba.web.rpc.common.common import ATTRIBUTES_ARRAY_ALLOWLIST from snuba.web.rpc.v1.endpoint_get_trace import ( APPLY_FINAL_ROLLOUT_PERCENTAGE_CONFIG_KEY, @@ -232,7 +233,7 @@ def test_with_data_all_attributes(self, setup_teardown: Any) -> None: ], page_token=( PageToken(end_pagination=True) - if state.get_int_config("enable_trace_pagination", ENABLE_TRACE_PAGINATION_DEFAULT) + if get_bool_option("enable_trace_pagination", bool(ENABLE_TRACE_PAGINATION_DEFAULT)) else None ), ) @@ -327,7 +328,7 @@ def test_with_specific_attributes(self, setup_teardown: Any) -> None: ], page_token=( PageToken(end_pagination=True) - if state.get_int_config("enable_trace_pagination", ENABLE_TRACE_PAGINATION_DEFAULT) + if get_bool_option("enable_trace_pagination", bool(ENABLE_TRACE_PAGINATION_DEFAULT)) else None ), ) @@ -364,23 +365,13 @@ def test_build_query_with_final(store_outcomes_data: Any) -> None: items=[item], ) - state.set_config( - APPLY_FINAL_ROLLOUT_PERCENTAGE_CONFIG_KEY, - 1.0, - ) - - query = _build_query(message, item) - - assert query.get_final() - - state.set_config( - APPLY_FINAL_ROLLOUT_PERCENTAGE_CONFIG_KEY, - 0.0, - ) - - query = _build_query(message, item) + with override_options("snuba", {APPLY_FINAL_ROLLOUT_PERCENTAGE_CONFIG_KEY: 1.0}): + query = _build_query(message, item) + assert query.get_final() - assert not query.get_final() + with override_options("snuba", {APPLY_FINAL_ROLLOUT_PERCENTAGE_CONFIG_KEY: 0.0}): + query = _build_query(message, item) + assert not query.get_final() def test_with_logs(self, setup_teardown: Any) -> None: ts = Timestamp(seconds=int(_BASE_TIME.timestamp())) @@ -449,7 +440,7 @@ def test_with_logs(self, setup_teardown: Any) -> None: ], page_token=( PageToken(end_pagination=True) - if state.get_int_config("enable_trace_pagination", ENABLE_TRACE_PAGINATION_DEFAULT) + if get_bool_option("enable_trace_pagination", bool(ENABLE_TRACE_PAGINATION_DEFAULT)) else None ), ) @@ -578,7 +569,6 @@ def test_process_results_keeps_empty_string_attribute() -> None: class TestGetTracePagination(BaseApiTest): def test_pagination_with_user_limit(self, setup_teardown: Any) -> None: """Test that pagination respects user-provided limit""" - state.set_config("enable_trace_pagination", 1) ts = Timestamp(seconds=int(_BASE_TIME.timestamp())) three_hours_later = int((_BASE_TIME + timedelta(hours=3)).timestamp()) mylimit = 10 @@ -640,7 +630,6 @@ def test_pagination_with_no_user_limit(self, setup_teardown: Any) -> None: with patch( "snuba.web.rpc.v1.endpoint_get_trace.ENDPOINT_GET_TRACE_PAGINATION_MAX_ITEMS", configmax ): - state.set_config("enable_trace_pagination", 1) """ import snuba.web.rpc.v1.endpoint_get_trace as endpoint_get_trace diff --git a/tests/web/rpc/v1/test_endpoint_get_traces.py b/tests/web/rpc/v1/test_endpoint_get_traces.py index 30b94387792..459e84d4062 100644 --- a/tests/web/rpc/v1/test_endpoint_get_traces.py +++ b/tests/web/rpc/v1/test_endpoint_get_traces.py @@ -1,11 +1,12 @@ import uuid from collections import defaultdict from datetime import datetime, timedelta, timezone -from typing import Any +from typing import Any, Generator import pytest from google.protobuf.json_format import MessageToDict from google.protobuf.timestamp_pb2 import Timestamp +from sentry_options.testing import override_options from sentry_protos.snuba.v1.endpoint_get_traces_pb2 import ( GetTracesRequest, GetTracesResponse, @@ -43,7 +44,6 @@ from snuba.web.rpc.common.exceptions import BadSnubaRPCRequestException from snuba.web.rpc.v1.endpoint_get_traces import EndpointGetTraces from tests.base import BaseApiTest -from tests.conftest import SnubaSetConfig from tests.helpers import write_raw_unprocessed_events from tests.web.rpc.v1.test_utils import ( comparison_filter, @@ -989,8 +989,7 @@ class TestEndpointGetTracesCrossItem(TestEndpointGetTraces): """Run all tests with use_cross_item_path_for_single_item_queries enabled.""" @pytest.fixture(autouse=True) - def use_cross_item_path( - self, clickhouse_db: Any, redis_db: Any, snuba_set_config: SnubaSetConfig - ) -> None: + def use_cross_item_path(self, clickhouse_db: Any, redis_db: Any) -> Generator[None, None, None]: """Enable the feature flag for cross-item path for all tests in this class.""" - snuba_set_config("use_cross_item_path_for_single_item_queries", 1) + with override_options("snuba", {"use_cross_item_path_for_single_item_queries": True}): + yield diff --git a/tests/web/rpc/v1/test_endpoint_time_series/test_endpoint_time_series_cross_item_sampling.py b/tests/web/rpc/v1/test_endpoint_time_series/test_endpoint_time_series_cross_item_sampling.py index 95ad4a199ba..cc75d33ef76 100644 --- a/tests/web/rpc/v1/test_endpoint_time_series/test_endpoint_time_series_cross_item_sampling.py +++ b/tests/web/rpc/v1/test_endpoint_time_series/test_endpoint_time_series_cross_item_sampling.py @@ -2,6 +2,7 @@ from unittest.mock import patch import pytest +from sentry_options.testing import override_options from sentry_protos.snuba.v1.endpoint_time_series_pb2 import ( Expression, TimeSeriesRequest, @@ -17,7 +18,6 @@ ) from sentry_protos.snuba.v1.trace_item_filter_pb2 import TraceItemFilter -from snuba import state from snuba.datasets.storages.storage_key import StorageKey from snuba.downsampled_storage_tiers import Tier from snuba.web.rpc import RPCEndpoint @@ -83,9 +83,6 @@ def test_cross_item_query_sampling_enabled(self) -> None: - The inner query uses downsampled storage (TIER_8) - The outer query uses full storage (EAP_ITEMS) """ - # Enable the feature flag - state.set_config("cross_item_queries_no_sample_outer", 1) - trace_ids, all_items, start_time, end_time = create_cross_item_test_data() write_cross_item_data_to_storage(all_items) @@ -102,7 +99,11 @@ def test_cross_item_query_sampling_enabled(self) -> None: storage_keys, storage_tracker = track_storage_selections() - with storage_tracker: + # Enable the feature flag for the duration of the query execution. + with ( + override_options("snuba", {"cross_item_queries_no_sample_outer": True}), + storage_tracker, + ): with patch.object(RPCEndpoint, "_RPCEndpoint__before_execute"): message = create_time_series_request( start_time=start_time, @@ -139,9 +140,6 @@ def test_cross_item_query_sampling_disabled(self) -> None: Test that when cross_item_queries_no_sample_outer is disabled (default): - Both queries use the same storage tier """ - # Explicitly disable the feature flag - state.set_config("cross_item_queries_no_sample_outer", 0) - trace_ids, all_items, start_time, end_time = create_cross_item_test_data() write_cross_item_data_to_storage(all_items) @@ -154,7 +152,11 @@ def test_cross_item_query_sampling_disabled(self) -> None: storage_keys, storage_tracker = track_storage_selections() - with storage_tracker: + # Explicitly disable the feature flag for the duration of the query execution. + with ( + override_options("snuba", {"cross_item_queries_no_sample_outer": False}), + storage_tracker, + ): with patch.object(RPCEndpoint, "_RPCEndpoint__before_execute"): message = create_time_series_request( start_time=start_time, diff --git a/tests/web/rpc/v1/test_endpoint_trace_item_table/test_endpoint_trace_item_table_cross_item_sampling.py b/tests/web/rpc/v1/test_endpoint_trace_item_table/test_endpoint_trace_item_table_cross_item_sampling.py index ab3b41ee650..9b70489a2c3 100644 --- a/tests/web/rpc/v1/test_endpoint_trace_item_table/test_endpoint_trace_item_table_cross_item_sampling.py +++ b/tests/web/rpc/v1/test_endpoint_trace_item_table/test_endpoint_trace_item_table_cross_item_sampling.py @@ -2,6 +2,7 @@ from unittest.mock import patch import pytest +from sentry_options.testing import override_options from sentry_protos.snuba.v1.endpoint_trace_item_table_pb2 import ( Column, TraceItemTableRequest, @@ -13,7 +14,6 @@ from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey from sentry_protos.snuba.v1.trace_item_filter_pb2 import TraceItemFilter -from snuba import state from snuba.datasets.storages.storage_key import StorageKey from snuba.downsampled_storage_tiers import Tier from snuba.web.rpc import RPCEndpoint @@ -64,9 +64,6 @@ def test_cross_item_query_sampling_enabled(self) -> None: - The inner query uses downsampled storage (TIER_8) - The outer query uses full storage (EAP_ITEMS) """ - # Enable the feature flag - state.set_config("cross_item_queries_no_sample_outer", 1) - trace_ids, all_items, start_time, end_time = create_cross_item_test_data() write_cross_item_data_to_storage(all_items) @@ -83,7 +80,11 @@ def test_cross_item_query_sampling_enabled(self) -> None: storage_keys, storage_tracker = track_storage_selections() - with storage_tracker: + # Enable the feature flag for the duration of the query execution. + with ( + override_options("snuba", {"cross_item_queries_no_sample_outer": True}), + storage_tracker, + ): with patch.object(RPCEndpoint, "_RPCEndpoint__before_execute"): message = create_trace_item_table_request( start_time=start_time, @@ -123,9 +124,6 @@ def test_cross_item_query_sampling_disabled(self) -> None: Test that when cross_item_queries_no_sample_outer is disabled (default): - Both queries use the same storage tier """ - # Explicitly disable the feature flag - state.set_config("cross_item_queries_no_sample_outer", 0) - trace_ids, all_items, start_time, end_time = create_cross_item_test_data() write_cross_item_data_to_storage(all_items) @@ -138,7 +136,11 @@ def test_cross_item_query_sampling_disabled(self) -> None: storage_keys, storage_tracker = track_storage_selections() - with storage_tracker: + # Explicitly disable the feature flag for the duration of the query execution. + with ( + override_options("snuba", {"cross_item_queries_no_sample_outer": False}), + storage_tracker, + ): with patch.object(RPCEndpoint, "_RPCEndpoint__before_execute"): message = create_trace_item_table_request( start_time=start_time, diff --git a/tests/web/rpc/v1/test_storage_routing.py b/tests/web/rpc/v1/test_storage_routing.py index 792bf70d7a4..7ed9bf52498 100644 --- a/tests/web/rpc/v1/test_storage_routing.py +++ b/tests/web/rpc/v1/test_storage_routing.py @@ -7,6 +7,7 @@ import pytest from google.protobuf.timestamp_pb2 import Timestamp from sentry_kafka_schemas import get_codec +from sentry_options.testing import override_options from sentry_protos.snuba.v1.downsampled_storage_pb2 import DownsampledStorageConfig from sentry_protos.snuba.v1.endpoint_time_series_pb2 import TimeSeriesRequest from sentry_protos.snuba.v1.request_common_pb2 import RequestMeta, TraceItemType @@ -240,32 +241,28 @@ def test_routing_strategy_selects_tier_1_if_highest_accuracy_mode() -> None: @pytest.mark.redis_db def test_routing_decision_forced_downsample_killswitch() -> None: - state.set_config("default_tier", 8) - - try: - ts = Timestamp() - ts.GetCurrentTime() - tstart = Timestamp(seconds=ts.seconds - 3600) - in_msg = TimeSeriesRequest( - meta=RequestMeta( - request_id=RANDOM_REQUEST_ID, - project_ids=[1, 2, 3], - organization_id=1, - cogs_category="something", - referrer="something", - start_timestamp=tstart, - end_timestamp=ts, - trace_item_type=TraceItemType.TRACE_ITEM_TYPE_SPAN, - ), - granularity_secs=60, - ) - routing_context = deepcopy(ROUTING_CONTEXT) - routing_context.in_msg = in_msg - in_msg.meta.downsampled_storage_config.mode = DownsampledStorageConfig.MODE_HIGHEST_ACCURACY + ts = Timestamp() + ts.GetCurrentTime() + tstart = Timestamp(seconds=ts.seconds - 3600) + in_msg = TimeSeriesRequest( + meta=RequestMeta( + request_id=RANDOM_REQUEST_ID, + project_ids=[1, 2, 3], + organization_id=1, + cogs_category="something", + referrer="something", + start_timestamp=tstart, + end_timestamp=ts, + trace_item_type=TraceItemType.TRACE_ITEM_TYPE_SPAN, + ), + granularity_secs=60, + ) + routing_context = deepcopy(ROUTING_CONTEXT) + routing_context.in_msg = in_msg + in_msg.meta.downsampled_storage_config.mode = DownsampledStorageConfig.MODE_HIGHEST_ACCURACY + with override_options("snuba", {"default_tier": 8}): routing_decision = AlwaysTier1RoutingStrategy().get_routing_decision(routing_context) - assert routing_decision.tier == Tier.TIER_8 - finally: - state.delete_config("default_tier") + assert routing_decision.tier == Tier.TIER_8 @pytest.mark.redis_db diff --git a/tests/web/test_bulk_delete_query.py b/tests/web/test_bulk_delete_query.py index 2fa54bc7de8..22930638cca 100644 --- a/tests/web/test_bulk_delete_query.py +++ b/tests/web/test_bulk_delete_query.py @@ -8,6 +8,7 @@ import rapidjson from confluent_kafka import Consumer from confluent_kafka.admin import AdminClient +from sentry_options.testing import override_options from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey from snuba import settings @@ -97,12 +98,12 @@ def test_deletes_not_enabled_on_storage() -> None: @pytest.mark.redis_db +@override_options("snuba", {"storage_deletes_enabled": False}) def test_deletes_not_enabled_runtime_config() -> None: storage = get_writable_storage(StorageKey("search_issues")) conditions = {"project_id": [1], "group_id": [1, 2, 3, 4]} attr_info = get_attribution_info() - set_config("storage_deletes_enabled", 0) with pytest.raises(DeletesNotEnabledError): delete_from_storage(storage, conditions, attr_info) @@ -264,10 +265,7 @@ def test_attribute_conditions_feature_flag_enabled() -> None: ) attr_info = get_attribution_info() - # Enable the feature flag - set_config("permit_delete_by_attribute", 1) - - try: + with override_options("snuba", {"permit_delete_by_attribute": True}): # 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: @@ -290,9 +288,6 @@ def test_attribute_conditions_feature_flag_enabled() -> None: } } assert call_args["attribute_conditions_item_type"] == TRACE_ITEM_TYPE_OCCURRENCE - finally: - # Clean up: disable the feature flag - set_config("permit_delete_by_attribute", 0) @pytest.mark.redis_db @@ -314,8 +309,7 @@ def test_eap_items_counts_each_table_against_its_readonly_replica() -> None: ) attr_info = get_attribution_info() - set_config("permit_delete_by_attribute", 1) - try: + with override_options("snuba", {"permit_delete_by_attribute": True}): with patch( "snuba.web.bulk_delete_query._enforce_max_rows", return_value=10 ) as mock_enforce: @@ -337,8 +331,6 @@ def test_eap_items_counts_each_table_against_its_readonly_replica() -> None: "eap_items_downsample_64_ro", "eap_items_downsample_512_ro", } - finally: - set_config("permit_delete_by_attribute", 0) def test_count_storage_key_mapping_without_readonly_storage_set() -> None: diff --git a/tests/web/test_db_query.py b/tests/web/test_db_query.py index 6b5290e2702..fa92576fc0a 100644 --- a/tests/web/test_db_query.py +++ b/tests/web/test_db_query.py @@ -4,6 +4,7 @@ from unittest import mock import pytest +from sentry_options.testing import override_options from snuba import state from snuba.attribution.appid import AppID @@ -397,8 +398,6 @@ def test_bypass_cache_referrer() -> None: query_metadata_list: list[ClickhouseQueryMetadata] = [] stats: dict[str, Any] = {"clickhouse_table": "errors_local"} - state.set_config("enable_bypass_cache_referrers", 1) - attribution_info = AttributionInfo( app_id=AppID(key="key"), tenant_ids={ @@ -413,7 +412,10 @@ def test_bypass_cache_referrer() -> None: # cache should not be used for "some_bypass_cache_referrer" so if the # bypass does not work, the test will try to use a bad cache - with mock.patch("snuba.settings.BYPASS_CACHE_REFERRERS", ["some_bypass_cache_referrer"]): + with ( + override_options("snuba", {"enable_bypass_cache_referrers": True}), + mock.patch("snuba.settings.BYPASS_CACHE_REFERRERS", ["some_bypass_cache_referrer"]), + ): with mock.patch("snuba.web.db_query._get_cache_partition"): result = db_query( clickhouse_query=query, diff --git a/tests/web/test_max_rows_enforcer.py b/tests/web/test_max_rows_enforcer.py index 729f8d19e99..8e1df3c283a 100644 --- a/tests/web/test_max_rows_enforcer.py +++ b/tests/web/test_max_rows_enforcer.py @@ -1,8 +1,9 @@ from datetime import datetime -from typing import Any, Callable +from typing import Any, Callable, Generator from unittest import mock import pytest +from sentry_options.testing import override_options from snuba.clickhouse.columns import ColumnSet from snuba.clickhouse.query import Query @@ -14,7 +15,6 @@ 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.web.rpc.v1.test_utils import write_eap_item @@ -43,8 +43,12 @@ def setup_method(self, test_method: Callable[..., Any]) -> None: is_delete=True, ) + @pytest.fixture(autouse=True) + def _short_circuit_cache(self) -> Generator[None, None, None]: + with override_options("snuba", {"read_through_cache.short_circuit": True}): + yield + def _insert_event(self) -> None: - set_config("read_through_cache.short_circuit", 1) now = datetime.now().replace(minute=0, second=0, microsecond=0) write_eap_item( @@ -86,7 +90,7 @@ def test_max_row_enforcer_rejects(self, mock: mock.MagicMock) -> None: allowed_columns=["project_id", "organization_id"], ), ) + @override_options("snuba", {"enforce_max_rows_to_delete": False}) def test_bypass_enforce_max_rows(self, mock: mock.MagicMock) -> None: - set_config("enforce_max_rows_to_delete", 0) self._insert_event() _enforce_max_rows(self.query) diff --git a/uv.lock b/uv.lock index 760487918eb..a37bf392f3c 100644 --- a/uv.lock +++ b/uv.lock @@ -933,6 +933,16 @@ wheels = [ { url = "https://pypi.devinfra.sentry.io/wheels/sentry_kafka_schemas-2.1.35-py2.py3-none-any.whl", hash = "sha256:067da6d401082cc5948005211179a58c5d5b9d559c6ce60b82eac47e02d31a5d" }, ] +[[package]] +name = "sentry-options" +version = "1.1.1" +source = { registry = "https://pypi.devinfra.sentry.io/simple" } +wheels = [ + { url = "https://pypi.devinfra.sentry.io/wheels/sentry_options-1.1.1-cp311-abi3-macosx_11_0_arm64.whl", hash = "sha256:a552680a15d348be7c4748a91931e6fb1d16bcc3945327761a46765ba272e0ab" }, + { url = "https://pypi.devinfra.sentry.io/wheels/sentry_options-1.1.1-cp311-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:c98569da06dc000bde0908e538015d359ac84fa718dc8e4763750b1092d092bf" }, + { url = "https://pypi.devinfra.sentry.io/wheels/sentry_options-1.1.1-cp311-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:0738a2c2ad68a4b56654d8a1d9304506fda372eb93c65f70074547605ac2d91f" }, +] + [[package]] name = "sentry-protos" version = "0.32.0" @@ -1072,6 +1082,7 @@ dependencies = [ { name = "sentry-arroyo", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, { name = "sentry-conventions", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, { name = "sentry-kafka-schemas", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, + { name = "sentry-options", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, { name = "sentry-protos", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, { name = "sentry-redis-tools", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, { name = "sentry-relay", marker = "sys_platform == 'darwin' or sys_platform == 'linux'" }, @@ -1141,6 +1152,7 @@ requires-dist = [ { name = "sentry-arroyo", specifier = ">=2.39.2" }, { name = "sentry-conventions", specifier = ">=0.12.0" }, { name = "sentry-kafka-schemas", specifier = ">=2.1.35" }, + { name = "sentry-options", specifier = ">=1.1.1" }, { name = "sentry-protos", specifier = ">=0.32.0" }, { name = "sentry-redis-tools", specifier = ">=0.5.1" }, { name = "sentry-relay", specifier = ">=0.9.25" },