Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(metrics): Extract user satisfaction as tag [INGEST-589] #1197

Merged
merged 10 commits into from
Mar 3, 2022
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

**Features**:

- Tag transaction metrics by user satisfaction. ([#1197](https://github.com/getsentry/relay/pull/1197))

**Internal**:

- Spread out metric aggregation over the aggregation window to avoid concentrated waves of metrics requests to the upstream every 10 seconds. Relay now applies jitter to `initial_delay` to spread out requests more evenly over time. ([#1185](https://github.com/getsentry/relay/pull/1185))
Expand Down
1 change: 1 addition & 0 deletions relay-general/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub use normalize::breakdowns::{
get_breakdown_measurements, BreakdownConfig, BreakdownsConfig, SpanOperationsConfig,
};
pub use normalize::normalize_dist;
pub use transactions::validate_timestamps;

/// The config for store.
#[derive(Serialize, Deserialize, Debug, Default)]
Expand Down
56 changes: 33 additions & 23 deletions relay-general/src/store/transactions.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,40 @@
use crate::processor::{ProcessValue, ProcessingState, Processor};
use crate::protocol::{Context, ContextInner, Event, EventType, Span};
use crate::protocol::{Context, ContextInner, Event, EventType, Span, Timestamp};
use crate::types::{Annotated, Meta, ProcessingAction, ProcessingResult};

/// Rejects transactions based on required fields.
pub struct TransactionsProcessor;

/// Returns start and end timestamps if they are both set and start <= end.
pub fn validate_timestamps(
transaction_event: &Event,
) -> Result<(&Timestamp, &Timestamp), ProcessingAction> {
match (
transaction_event.start_timestamp.value(),
transaction_event.timestamp.value(),
) {
(Some(start), Some(end)) => {
if *end < *start {
return Err(ProcessingAction::InvalidTransaction(
"end timestamp is smaller than start timestamp",
));
}
Ok((start, end))
}
(_, None) => {
// This invariant should be already guaranteed for regular error events.
Err(ProcessingAction::InvalidTransaction(
"timestamp hard-required for transaction events",
))
}
(None, _) => {
// XXX: Maybe copy timestamp over?
Err(ProcessingAction::InvalidTransaction(
"start_timestamp hard-required for transaction events",
))
}
}
}

impl Processor for TransactionsProcessor {
fn process_event(
&mut self,
Expand All @@ -27,27 +57,7 @@ impl Processor for TransactionsProcessor {
.set_value(Some("<unlabeled transaction>".to_owned()))
}

match (event.start_timestamp.value(), event.timestamp.value_mut()) {
(Some(start), Some(end)) => {
if *end < *start {
return Err(ProcessingAction::InvalidTransaction(
"end timestamp is smaller than start timestamp",
));
}
}
(_, None) => {
// This invariant should be already guaranteed for regular error events.
return Err(ProcessingAction::InvalidTransaction(
"timestamp hard-required for transaction events",
));
}
(None, _) => {
// XXX: Maybe copy timestamp over?
return Err(ProcessingAction::InvalidTransaction(
"start_timestamp hard-required for transaction events",
));
}
}
validate_timestamps(event)?;

let err_trace_context_required = Err(ProcessingAction::InvalidTransaction(
"trace context hard-required for transaction events",
Expand Down