Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
25 changes: 14 additions & 11 deletions rust_snuba/src/processors/generic_metrics.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -339,8 +339,8 @@ impl Parse for CountersRawRow {
}
}

fn should_use_killswitch(config: Result<Option<String>, Error>, use_case: &MessageUseCase) -> bool {
if let Some(killswitch) = config.ok().flatten() {
fn should_use_killswitch(config: Option<String>, use_case: &MessageUseCase) -> bool {
if let Some(killswitch) = config {
return killswitch.contains(use_case.use_case_id.as_str());
}

Expand All @@ -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));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics killswitch ignores runtime config

High Severity

The generic-metrics consumer reads generic_metrics_use_case_killswitch only from sentry-options, not from Snuba runtime config (Redis/admin). Overrides still managed through the admin UI or state.set_config no longer affect Rust processing, so an active killswitch can stop working after deploy until the same value exists in sentry-options-automator.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7659d7a. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional and consistent with the rest of this PR (and with the other Rust consumer killswitches, e.g. blq/healthcheck, which already read from sentry-options). This Rust consumer is the only reader of generic_metrics_use_case_killswitch, so there's no split-brain between Python and Rust — moving the single read site fully relocates the source of truth to sentry-options, where operators manage it via sentry-options-automator going forward.

The one real point here is rollout cutover, which applies to every key in this migration: a value currently set in Redis runtime config does not auto-copy into sentry-options. So as part of rolling this out, any presently-active overrides (this killswitch included) need to be mirrored into sentry-options-automator before/with the deploy so they carry over. That's a runbook/coordination step rather than a code change. Keeping the read on sentry-options.


Generated by Claude Code

let use_case: MessageUseCase = serde_json::from_slice(payload_bytes)?;

if should_use_killswitch(killswitch_config, &use_case) {
Expand Down Expand Up @@ -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(),
};
Expand All @@ -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));
}
Expand All @@ -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));
}
Expand All @@ -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));
}
Expand All @@ -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));
}
Expand All @@ -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<String> = None;

assert!(!should_use_killswitch(fake_config, &use_case));
}
Expand Down
10 changes: 5 additions & 5 deletions rust_snuba/src/processors/utils.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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";
Expand All @@ -34,10 +34,10 @@ pub fn out_of_valid_interval_secs(ts: DateTime<Utc>, now: DateTime<Utc>) -> 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)
}

Expand Down
28 changes: 20 additions & 8 deletions rust_snuba/src/strategies/healthcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -57,11 +57,11 @@ where
fn poll(&mut self) -> Result<Option<CommitRequest>, 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() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading