Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- Add InvalidReplayEvent outcome. ([#1455](https://github.com/getsentry/relay/pull/1455))
- Add replay and replay-recording rate limiter. ([#1456](https://github.com/getsentry/relay/pull/1456))
- Support profiles tagged for many transactions. ([#1444](https://github.com/getsentry/relay/pull/1444)), ([#1463](https://github.com/getsentry/relay/pull/1463)), ([#1464](https://github.com/getsentry/relay/pull/1464))
- Track metrics for changes to the transaction name and DSC propagations. ([#1466](https://github.com/getsentry/relay/pull/1466))

**Features**:

Expand Down
20 changes: 20 additions & 0 deletions relay-general/src/protocol/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,26 @@ impl Event {
.map(|m| m.contains_key(module_name))
.unwrap_or(false)
}

pub fn sdk_name(&self) -> &str {
if let Some(client_sdk) = self.client_sdk.value() {
if let Some(name) = client_sdk.name.as_str() {
return name;
}
}

"unknown"
}

pub fn sdk_version(&self) -> &str {
if let Some(client_sdk) = self.client_sdk.value() {
if let Some(version) = client_sdk.version.as_str() {
return version;
}
}

"unknown"
}
}

#[cfg(test)]
Expand Down
72 changes: 59 additions & 13 deletions relay-general/src/protocol/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::fmt;
use std::str::FromStr;

use crate::processor::ProcessValue;
use crate::protocol::Timestamp;
use crate::types::{Annotated, Empty, ErrorKind, FromValue, IntoValue, SkipSerialization, Value};

/// Describes how the name of the transaction was determined.
Expand All @@ -28,6 +29,21 @@ pub enum TransactionSource {
Other(String),
}

impl TransactionSource {
pub fn as_str(&self) -> &str {
match self {
Self::Custom => "custom",
Self::Url => "url",
Self::Route => "route",
Self::View => "view",
Self::Component => "component",
Self::Task => "task",
Self::Unknown => "unknown",
Self::Other(ref s) => s,
}
}
}

impl FromStr for TransactionSource {
type Err = std::convert::Infallible;

Expand All @@ -47,16 +63,7 @@ impl FromStr for TransactionSource {

impl fmt::Display for TransactionSource {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Custom => write!(f, "custom"),
Self::Url => write!(f, "url"),
Self::Route => write!(f, "route"),
Self::View => write!(f, "view"),
Self::Component => write!(f, "component"),
Self::Task => write!(f, "task"),
Self::Unknown => write!(f, "unknown"),
Self::Other(s) => write!(f, "{}", s),
}
write!(f, "{}", self.as_str())
}
}

Expand Down Expand Up @@ -108,6 +115,21 @@ impl IntoValue for TransactionSource {

impl ProcessValue for TransactionSource {}

#[derive(Clone, Debug, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
#[cfg_attr(feature = "jsonschema", derive(JsonSchema))]
pub struct TransactionNameChange {
/// Describes how the previous transaction name was determined.
pub source: Annotated<TransactionSource>,

/// The number of propagations from the start of the transaction to this change.
pub propagations: Annotated<u64>,

/// Timestamp when the transaction name was changed.
///
/// This adheres to the event timestamp specification.
pub timestamp: Annotated<Timestamp>,
}

/// Additional information about the name of the transaction.
#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
#[cfg_attr(feature = "jsonschema", derive(JsonSchema))]
Expand All @@ -123,10 +145,20 @@ pub struct TransactionInfo {
/// This value will only be set if the transaction name was modified during event processing.
#[metastructure(max_chars = "culprit", trim_whitespace = "true")]
pub original: Annotated<String>,

/// A list of changes prior to the final transaction name.
///
/// This list must be empty if the transaction name is set at the beginning of the transaction
/// and never changed. There is no placeholder entry for the initial transaction name.
pub changes: Annotated<Vec<Annotated<TransactionNameChange>>>,

/// The total number of propagations during the transaction.
pub propagations: Annotated<u64>,
}

#[cfg(test)]
mod tests {
use chrono::{TimeZone, Utc};
use similar_asserts::assert_eq;

use super::*;
Expand All @@ -143,13 +175,27 @@ mod tests {
#[test]
fn test_transaction_info_roundtrip() {
let json = r#"{
"source": "url",
"original": "/auth/login/john123/"
"source": "route",
"original": "/auth/login/john123/",
"changes": [
{
"source": "url",
"propagations": 1,
"timestamp": 946684800.0
}
],
"propagations": 2
}"#;

let info = Annotated::new(TransactionInfo {
source: Annotated::new(TransactionSource::Url),
source: Annotated::new(TransactionSource::Route),
original: Annotated::new("/auth/login/john123/".to_owned()),
changes: Annotated::new(vec![Annotated::new(TransactionNameChange {
source: Annotated::new(TransactionSource::Url),
propagations: Annotated::new(1),
timestamp: Annotated::new(Utc.ymd(2000, 1, 1).and_hms(0, 0, 0).into()),
})]),
propagations: Annotated::new(2),
});

assert_eq!(info, Annotated::from_json(json).unwrap());
Expand Down
75 changes: 73 additions & 2 deletions relay-general/tests/snapshots/test_fixtures__event_schema.snap
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
source: relay-general/tests/test_fixtures.rs
assertion_line: 110
expression: event_json_schema()
expression: "relay_general::protocol::event_json_schema()"
---
{
"$schema": "http://json-schema.org/draft-07/schema#",
Expand Down Expand Up @@ -2983,6 +2982,24 @@ expression: event_json_schema()
{
"type": "object",
"properties": {
"changes": {
"description": " A list of changes prior to the final transaction name.\n\n This list must be empty if the transaction name is set at the beginning of the transaction\n and never changed. There is no placeholder entry for the initial transaction name.",
"default": null,
"type": [
"array",
"null"
],
"items": {
"anyOf": [
{
"$ref": "#/definitions/TransactionNameChange"
},
{
"type": "null"
}
]
}
},
"original": {
"description": " The unmodified transaction name as obtained by the source.\n\n This value will only be set if the transaction name was modified during event processing.",
"default": null,
Expand All @@ -2991,6 +3008,16 @@ expression: event_json_schema()
"null"
]
},
"propagations": {
"description": " The total number of propagations during the transaction.",
"default": null,
"type": [
"integer",
"null"
],
"format": "uint64",
"minimum": 0.0
},
"source": {
"description": " Describes how the name of the transaction was determined.\n\n This will be used by the server to decide whether or not to scrub identifiers from the\n transaction name, or replace the entire name with a placeholder.",
"default": null,
Expand All @@ -3008,6 +3035,50 @@ expression: event_json_schema()
}
]
},
"TransactionNameChange": {
"anyOf": [
{
"type": "object",
"properties": {
"propagations": {
"description": " The number of propagations from the start of the transaction to this change.",
"default": null,
"type": [
"integer",
"null"
],
"format": "uint64",
"minimum": 0.0
},
"source": {
"description": " Describes how the previous transaction name was determined.",
"default": null,
"anyOf": [
{
"$ref": "#/definitions/TransactionSource"
},
{
"type": "null"
}
]
},
"timestamp": {
"description": " Timestamp when the transaction name was changed.\n\n This adheres to the event timestamp specification.",
"default": null,
"anyOf": [
{
"$ref": "#/definitions/Timestamp"
},
{
"type": "null"
}
]
}
},
"additionalProperties": false
}
]
},
"TransactionSource": {
"description": "Describes how the name of the transaction was determined.",
"type": "string",
Expand Down
96 changes: 94 additions & 2 deletions relay-server/src/actors/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use relay_log::LogError;
use relay_metrics::{Bucket, Metric};
use relay_quotas::{DataCategory, RateLimits, ReasonCode};
use relay_redis::RedisPool;
use relay_sampling::RuleId;
use relay_sampling::{DynamicSamplingContext, RuleId};
use relay_statsd::metric;

use crate::actors::envelopes::{EnvelopeManager, SendEnvelope, SendEnvelopeError, SubmitEnvelope};
Expand All @@ -45,7 +45,7 @@ use crate::envelope::{AttachmentType, ContentType, Envelope, Item, ItemType};
use crate::metrics_extraction::sessions::{extract_session_metrics, SessionMetricsConfig};
use crate::metrics_extraction::transactions::extract_transaction_metrics;
use crate::service::{ServerError, REGISTRY};
use crate::statsd::{RelayCounters, RelayTimers};
use crate::statsd::{RelayCounters, RelayHistograms, RelayTimers};
use crate::utils::{
self, ChunkedFormDataAggregator, EnvelopeContext, ErrorBoundary, FormDataIter, SamplingResult,
};
Expand Down Expand Up @@ -344,6 +344,94 @@ fn outcome_from_profile_error(err: relay_profiling::ProfileError) -> Outcome {
Outcome::Invalid(discard_reason)
}

fn track_sampling_metrics(
project_state: &ProjectState,
context: &DynamicSamplingContext,
event: &Event,
) {
// We only collect this metric for the root transaction event, so ignore secondary projects.
if !project_state.is_matching_key(context.public_key) {
return;
}

let transaction_info = match event.transaction_info.value() {
Some(info) => info,
None => return,
};

let changes = match transaction_info.changes.value() {
Some(value) => value.as_slice(),
None => return,
};

let last_change = changes.last().and_then(|a| a.value());
let source = event.get_transaction_source().as_str();
let platform = event.platform.as_str().unwrap_or("other");
let sdk_name = event.sdk_name();
let sdk_version = event.sdk_version();

metric!(
histogram(RelayHistograms::DynamicSamplingChanges) = changes.len() as u64,
source = source,
platform = platform,
sdk_name = sdk_name,
sdk_version = sdk_version,
);

if let Some(&total) = transaction_info.propagations.value() {
// If there was no change, there were no propagations that happened with a wrong name.
let change = last_change
.and_then(|c| c.propagations.value())
.map_or(0, |v| *v);

metric!(
histogram(RelayHistograms::DynamicSamplingPropagationCount) = change,
source = source,
platform = platform,
sdk_name = sdk_name,
sdk_version = sdk_version,
);

let percentage = ((change as f64) / (total as f64)).min(1.0);
metric!(
histogram(RelayHistograms::DynamicSamplingPropagationPercentage) = percentage,
source = source,
platform = platform,
sdk_name = sdk_name,
sdk_version = sdk_version,
);
}

if let (Some(&start), Some(&change), Some(&end)) = (
event.start_timestamp.value(),
last_change.and_then(|c| c.timestamp.value()),
event.timestamp.value(),
) {
let delay_ms = (change - start).num_milliseconds();
if delay_ms >= 0 {
metric!(
histogram(RelayHistograms::DynamicSamplingChangeDuration) = delay_ms as u64,
source = source,
platform = platform,
sdk_name = sdk_name,
sdk_version = sdk_version,
);
}

let duration_ms = (end - start).num_milliseconds() as f64;
if delay_ms >= 0 && duration_ms >= 0.0 {
let percentage = ((delay_ms as f64) / duration_ms).min(1.0);
metric!(
histogram(RelayHistograms::DynamicSamplingChangePercentage) = percentage,
source = source,
platform = platform,
sdk_name = sdk_name,
sdk_version = sdk_version,
);
}
}
}

/// Synchronous service for processing envelopes.
pub struct EnvelopeProcessor {
config: Arc<Config>,
Expand Down Expand Up @@ -1625,6 +1713,10 @@ impl EnvelopeProcessor {
}
);
state.transaction_metrics_extracted = true;

if let Some(context) = state.envelope.sampling_context() {
track_sampling_metrics(&state.project_state, context, event);
}
}
Ok(())
}
Expand Down
Loading