Skip to content

Commit 5b63073

Browse files
ref(eap-outcomes): use sentry-options to change which timestamp to use (#7838)
This is a duplicate of #7834, using sentry-options instead of the old config method. #7836 should be merged first to ensure all the scaffolding works in production. Because this is a duplicate, the logic, usage, and tests all mirror #7834. The only thing I've added is we use sentry-options instead. --------- Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
1 parent 0a2a602 commit 5b63073

File tree

8 files changed

+123
-28
lines changed

8 files changed

+123
-28
lines changed

Dockerfile

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ RUN set -ex; \
8888

8989
FROM build_rust_snuba_deps AS build_rust_snuba
9090
COPY ./rust_snuba/ ./rust_snuba/
91+
COPY ./sentry-options/schemas/ ./sentry-options/schemas/
9192
COPY --from=build_rust_snuba_deps /usr/src/snuba/rust_snuba/target/ ./rust_snuba/target/
9293
COPY --from=build_rust_snuba_deps /root/.cargo/ /root/.cargo/
9394
RUN set -ex; \
@@ -134,8 +135,7 @@ ENV LD_PRELOAD=/usr/src/snuba/libjemalloc.so.2 \
134135
PYTHONUNBUFFERED=1 \
135136
PYTHONDONTWRITEBYTECODE=1
136137

137-
# set up sentry options schemas and default path
138-
COPY sentry-options/schemas /etc/sentry-options/schemas
138+
# set default path for sentry options values
139139
ENV SENTRY_OPTIONS_DIR=/etc/sentry-options
140140

141141
USER snuba
@@ -167,7 +167,6 @@ FROM ghcr.io/getsentry/dhi/python:3.13-debian13 AS application-distroless
167167

168168
COPY --from=distroless_prep /.venv /.venv
169169
COPY --from=distroless_prep /usr/src/snuba /usr/src/snuba
170-
COPY --from=distroless_prep /etc/sentry-options /etc/sentry-options
171170
COPY --from=distroless_prep /usr/lib/*/libjemalloc.so.2 /usr/lib/libjemalloc.so.2
172171
COPY --from=distroless_prep /etc/passwd /etc/passwd
173172
COPY --from=distroless_prep /etc/group /etc/group
@@ -192,7 +191,6 @@ FROM ghcr.io/getsentry/dhi/python:3.13-debian13-dev AS application-distroless-de
192191

193192
COPY --from=distroless_prep /.venv /.venv
194193
COPY --from=distroless_prep /usr/src/snuba /usr/src/snuba
195-
COPY --from=distroless_prep /etc/sentry-options /etc/sentry-options
196194
COPY --from=distroless_prep /usr/lib/*/libjemalloc.so.2 /usr/lib/libjemalloc.so.2
197195
COPY --from=distroless_prep /etc/passwd /etc/passwd
198196
COPY --from=distroless_prep /etc/group /etc/group

rust_snuba/.cargo/config.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[env]
2+
SENTRY_OPTIONS_DIR = { value = "../sentry-options", relative = true }

rust_snuba/Cargo.lock

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust_snuba/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ sentry = { version = "0.41.0", default-features = false, features = [
5454
"logs"
5555
] }
5656
sentry-kafka-schemas = "2.1.24"
57-
sentry-options = "1.0.3"
57+
sentry-options = "1.0.5"
5858
sentry_protos = "0.7.0"
5959
sentry_arroyo = { version = "2.38.5", features = ["ssl"] }
6060
sentry_usage_accountant = { version = "0.1.2", features = ["kafka"] }

rust_snuba/src/accepted_outcomes_consumer.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ use sentry_arroyo::processing::strategies::{ProcessingStrategy, ProcessingStrate
1212
use sentry_arroyo::processing::StreamProcessor;
1313
use sentry_arroyo::types::{Partition, Topic};
1414

15+
use sentry_options::init_with_schemas;
16+
1517
use pyo3::prelude::*;
1618

1719
use crate::config;
@@ -33,7 +35,6 @@ pub struct AcceptedOutcomesStrategyFactory {
3335
producer: Arc<KafkaProducer>,
3436
concurrency: ConcurrencyConfig,
3537
skip_produce: bool,
36-
use_item_timestamp: bool,
3738
}
3839

3940
impl ProcessingStrategyFactory<KafkaPayload> for AcceptedOutcomesStrategyFactory {
@@ -55,7 +56,6 @@ impl ProcessingStrategyFactory<KafkaPayload> for AcceptedOutcomesStrategyFactory
5556
self.max_batch_size,
5657
self.max_batch_time_ms,
5758
self.bucket_interval,
58-
self.use_item_timestamp,
5959
))
6060
}
6161
}
@@ -120,6 +120,8 @@ pub fn accepted_outcomes_consumer_impl(
120120
commit_frequency_sec: u64,
121121
) -> usize {
122122
setup_logging();
123+
init_with_schemas(&[("snuba", crate::SNUBA_SCHEMA)])
124+
.expect("failed to initialize sentry-options");
123125

124126
let consumer_config = config::ConsumerConfig::load_from_str(consumer_config_raw).unwrap();
125127

@@ -204,11 +206,6 @@ pub fn accepted_outcomes_consumer_impl(
204206
.flatten()
205207
.unwrap_or("1".to_string())
206208
== "1";
207-
let use_item_timestamp = get_str_config("accepted_outcomes_use_item_timestamp")
208-
.ok()
209-
.flatten()
210-
.unwrap_or("0".to_string())
211-
== "1";
212209

213210
let factory = AcceptedOutcomesStrategyFactory {
214211
bucket_interval,
@@ -219,7 +216,6 @@ pub fn accepted_outcomes_consumer_impl(
219216
producer,
220217
concurrency: ConcurrencyConfig::new(concurrency),
221218
skip_produce,
222-
use_item_timestamp,
223219
};
224220
let processor = StreamProcessor::with_kafka(config, factory, topic, dlq_policy);
225221

rust_snuba/src/consumer.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ use sentry_arroyo::types::Topic;
1616
use pyo3::prelude::*;
1717
use pyo3::types::PyBytes;
1818

19+
use sentry_options::init_with_schemas;
20+
1921
use crate::config;
2022
use crate::factory_v2::ConsumerStrategyFactoryV2;
2123
use crate::logging::{setup_logging, setup_sentry};
@@ -94,6 +96,8 @@ pub fn consumer_impl(
9496
use_row_binary: bool,
9597
) -> usize {
9698
setup_logging();
99+
init_with_schemas(&[("snuba", crate::SNUBA_SCHEMA)])
100+
.expect("failed to initialize sentry-options");
97101

98102
let consumer_config = config::ConsumerConfig::load_from_str(consumer_config_raw).unwrap();
99103
let max_batch_size = consumer_config.max_batch_size;

rust_snuba/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
pub(crate) const SNUBA_SCHEMA: &str =
2+
include_str!("../../sentry-options/schemas/snuba/schema.json");
3+
14
mod accepted_outcomes_consumer;
25
mod config;
36
mod consumer;

rust_snuba/src/strategies/accepted_outcomes/aggregator.rs

Lines changed: 102 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ use sentry_arroyo::types::{InnerMessage, Message, Partition};
1212
use sentry_arroyo::utils::timing::Deadline;
1313
use sentry_protos::snuba::v1::TraceItem;
1414

15+
use sentry_options::options;
16+
1517
use crate::types::{AggregatedOutcomesBatch, BucketKey};
1618

1719
#[derive(Debug, Default)]
@@ -58,8 +60,7 @@ pub struct OutcomesAggregator<TNext> {
5860
message_carried_over: Option<Message<AggregatedOutcomesBatch>>,
5961
/// Commit request carried over from a poll where we had a message to retry.
6062
commit_request_carried_over: Option<CommitRequest>,
61-
/// Temporary option to change the timestamp source from
62-
/// `received` to `timestamp` on the item event.
63+
/// Cached value of the `consumer.use_item_timestamp` option, refreshed on each poll.
6364
use_item_timestamp: bool,
6465
}
6566

@@ -69,7 +70,6 @@ impl<TNext> OutcomesAggregator<TNext> {
6970
max_batch_size: usize,
7071
max_batch_time_ms: Duration,
7172
bucket_interval: u64,
72-
use_item_timestamp: bool,
7373
) -> Self {
7474
Self {
7575
next_step,
@@ -81,7 +81,7 @@ impl<TNext> OutcomesAggregator<TNext> {
8181
latest_offsets: HashMap::new(),
8282
message_carried_over: None,
8383
commit_request_carried_over: None,
84-
use_item_timestamp,
84+
use_item_timestamp: false,
8585
}
8686
}
8787

@@ -132,6 +132,12 @@ impl<TNext: ProcessingStrategy<AggregatedOutcomesBatch>> ProcessingStrategy<Kafk
132132
for OutcomesAggregator<TNext>
133133
{
134134
fn poll(&mut self) -> Result<Option<CommitRequest>, StrategyError> {
135+
self.use_item_timestamp = options("snuba")
136+
.ok()
137+
.and_then(|o| o.get("consumer.use_item_timestamp").ok())
138+
.and_then(|v| v.as_bool())
139+
.unwrap_or(false);
140+
135141
let commit_request = self.next_step.poll()?;
136142
self.commit_request_carried_over =
137143
merge_commit_request(self.commit_request_carried_over.take(), commit_request);
@@ -212,6 +218,7 @@ impl<TNext: ProcessingStrategy<AggregatedOutcomesBatch>> ProcessingStrategy<Kafk
212218
.map(|t| t.seconds as u64)
213219
.unwrap_or(0)
214220
};
221+
215222
let org_id = trace_item.organization_id;
216223
let project_id = trace_item.project_id;
217224

@@ -267,7 +274,10 @@ mod tests {
267274
use prost::Message as ProstMessage;
268275
use prost_types::Timestamp;
269276
use sentry_arroyo::types::{Partition, Topic};
277+
use sentry_options::init_with_schemas;
278+
use sentry_options::testing::override_options;
270279
use sentry_protos::snuba::v1::{CategoryCount, Outcomes};
280+
use serde_json::json;
271281

272282
struct Noop {
273283
last_message: Option<Message<AggregatedOutcomesBatch>>,
@@ -327,7 +337,6 @@ mod tests {
327337
500,
328338
Duration::from_millis(5_000),
329339
60,
330-
false,
331340
);
332341

333342
let topic = Topic::new("accepted-outcomes");
@@ -375,7 +384,6 @@ mod tests {
375384
500,
376385
Duration::from_millis(2_000),
377386
60,
378-
false,
379387
);
380388

381389
let topic = Topic::new("snuba-items");
@@ -459,12 +467,12 @@ mod tests {
459467

460468
#[test]
461469
fn poll_flushes_when_max_batch_size_reached() {
470+
init_with_schemas(&[("snuba", crate::SNUBA_SCHEMA)]).unwrap();
462471
let mut aggregator = OutcomesAggregator::new(
463472
Noop { last_message: None },
464473
1,
465474
Duration::from_millis(30_000),
466475
60,
467-
false,
468476
);
469477

470478
let partition = Partition::new(Topic::new("accepted-outcomes"), 0);
@@ -485,6 +493,7 @@ mod tests {
485493

486494
#[test]
487495
fn submit_returns_backpressure_when_message_carried_over() {
496+
init_with_schemas(&[("snuba", crate::SNUBA_SCHEMA)]).unwrap();
488497
struct RejectOnce {
489498
rejected: bool,
490499
}
@@ -517,7 +526,6 @@ mod tests {
517526
1, // flush after 1 bucket
518527
Duration::from_millis(30_000),
519528
60,
520-
false,
521529
);
522530

523531
let partition = Partition::new(Topic::new("test"), 0);
@@ -553,6 +561,7 @@ mod tests {
553561

554562
#[test]
555563
fn join_honors_timeout_when_message_stays_carried_over() {
564+
init_with_schemas(&[("snuba", crate::SNUBA_SCHEMA)]).unwrap();
556565
struct AlwaysReject;
557566
impl ProcessingStrategy<AggregatedOutcomesBatch> for AlwaysReject {
558567
fn poll(&mut self) -> Result<Option<CommitRequest>, StrategyError> {
@@ -574,7 +583,7 @@ mod tests {
574583
}
575584

576585
let mut aggregator =
577-
OutcomesAggregator::new(AlwaysReject, 1, Duration::from_millis(30_000), 60, false);
586+
OutcomesAggregator::new(AlwaysReject, 1, Duration::from_millis(30_000), 60);
578587
let partition = Partition::new(Topic::new("test"), 0);
579588
let payload = make_payload(6_000, 1, 2, 3, &[(4, 1)]);
580589

@@ -594,12 +603,14 @@ mod tests {
594603

595604
#[test]
596605
fn submit_uses_item_timestamp_when_enabled() {
606+
init_with_schemas(&[("snuba", crate::SNUBA_SCHEMA)]).unwrap();
607+
let _guard =
608+
override_options(&[("snuba", "consumer.use_item_timestamp", json!(true))]).unwrap();
597609
let mut aggregator = OutcomesAggregator::new(
598610
Noop { last_message: None },
599611
500,
600612
Duration::from_millis(2_000),
601613
60,
602-
true,
603614
);
604615

605616
let topic = Topic::new("snuba-items");
@@ -629,6 +640,9 @@ mod tests {
629640
trace_item.encode(&mut buf).unwrap();
630641
let payload = KafkaPayload::new(None, None, Some(buf));
631642

643+
// we need to poll first in order to get the new value (true)
644+
aggregator.poll().unwrap();
645+
632646
aggregator
633647
.submit(Message::new_broker_message(
634648
payload,
@@ -650,4 +664,82 @@ mod tests {
650664
Some(1)
651665
);
652666
}
667+
668+
#[test]
669+
fn poll_updates_use_item_timestamp_dynamically() {
670+
init_with_schemas(&[("snuba", crate::SNUBA_SCHEMA)]).unwrap();
671+
let mut aggregator = OutcomesAggregator::new(
672+
Noop { last_message: None },
673+
500,
674+
Duration::from_millis(30_000),
675+
60,
676+
);
677+
678+
let partition = Partition::new(Topic::new("snuba-items"), 0);
679+
680+
let mut buf = Vec::new();
681+
TraceItem {
682+
organization_id: 1,
683+
project_id: 2,
684+
received: Some(Timestamp {
685+
seconds: 1_700_000_000,
686+
nanos: 0,
687+
}),
688+
timestamp: Some(Timestamp {
689+
seconds: 1_700_000_060,
690+
nanos: 0,
691+
}),
692+
outcomes: Some(Outcomes {
693+
key_id: 3,
694+
category_count: vec![CategoryCount {
695+
data_category: 4,
696+
quantity: 1,
697+
}],
698+
}),
699+
..Default::default()
700+
}
701+
.encode(&mut buf)
702+
.unwrap();
703+
let payload = KafkaPayload::new(None, None, Some(buf));
704+
705+
let bucket_quantity = |aggregator: &OutcomesAggregator<Noop>, offset: u64| {
706+
let key = BucketKey {
707+
time_offset: offset,
708+
org_id: 1,
709+
project_id: 2,
710+
key_id: 3,
711+
category: 4,
712+
};
713+
aggregator.batch.buckets.get(&key).map(|s| s.quantity)
714+
};
715+
716+
let mut offset = 0;
717+
let mut do_submit = |aggregator: &mut OutcomesAggregator<Noop>| {
718+
aggregator.poll().unwrap();
719+
aggregator
720+
.submit(Message::new_broker_message(
721+
payload.clone(),
722+
partition,
723+
offset,
724+
Utc::now(),
725+
))
726+
.unwrap();
727+
offset += 1;
728+
};
729+
730+
// Enable item timestamp
731+
let guard =
732+
override_options(&[("snuba", "consumer.use_item_timestamp", json!(true))]).unwrap();
733+
do_submit(&mut aggregator);
734+
assert_eq!(bucket_quantity(&aggregator, 28_333_334), Some(1));
735+
assert_eq!(bucket_quantity(&aggregator, 28_333_333), None);
736+
737+
// Disable item timestamp
738+
drop(guard);
739+
let _guard =
740+
override_options(&[("snuba", "consumer.use_item_timestamp", json!(false))]).unwrap();
741+
do_submit(&mut aggregator);
742+
assert_eq!(bucket_quantity(&aggregator, 28_333_333), Some(1));
743+
assert_eq!(bucket_quantity(&aggregator, 28_333_334), Some(1)); // still present from first submit
744+
}
653745
}

0 commit comments

Comments
 (0)