Skip to content

Commit

Permalink
feat(protocol): Add transaction_info to events (#1330)
Browse files Browse the repository at this point in the history
Implements `transaction_info` containing the `source` field, which
indicates how the event's transaction name was obtained on the client.
This allows us to infer the quality of the transaction name and apply
server-side cleanup. Additionally, this adds the
`event.transaction_source` metric to measure how often each source
occurs in practice.
  • Loading branch information
jan-auer committed Jul 4, 2022
1 parent ed3a521 commit 135eb86
Show file tree
Hide file tree
Showing 9 changed files with 265 additions and 3 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
**Features**:

- Extract metrics also from trace-sampled transactions. ([#1317](https://github.com/getsentry/relay/pull/1317))

- Support `transaction_info` on event payloads. ([#1330](https://github.com/getsentry/relay/pull/1330))

**Bug Fixes**:

Expand Down
4 changes: 4 additions & 0 deletions py/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased

- Add `transaction_info` to event payloads, including the transaction's source and internal original transaction name. ([#1330](https://github.com/getsentry/relay/pull/1330))

## 0.8.13

- Add a data category constant for Replays. ([#1239](https://github.com/getsentry/relay/pull/1239))
Expand Down
7 changes: 6 additions & 1 deletion relay-general/src/protocol/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use crate::processor::ProcessValue;
use crate::protocol::{
Breadcrumb, Breakdowns, ClientSdkInfo, Contexts, Csp, DebugMeta, Exception, ExpectCt,
ExpectStaple, Fingerprint, Hpkp, LenientString, Level, LogEntry, Measurements, Metrics,
RelayInfo, Request, Span, Stacktrace, Tags, TemplateInfo, Thread, Timestamp, User, Values,
RelayInfo, Request, Span, Stacktrace, Tags, TemplateInfo, Thread, Timestamp, TransactionInfo,
User, Values,
};
use crate::types::{
Annotated, Array, Empty, ErrorKind, FromValue, IntoValue, Object, SkipSerialization, Value,
Expand Down Expand Up @@ -256,6 +257,10 @@ pub struct Event {
#[metastructure(max_chars = "culprit", trim_whitespace = "true")]
pub transaction: Annotated<String>,

/// Additional information about the name of the transaction.
#[metastructure(skip_serialization = "empty")]
pub transaction_info: Annotated<TransactionInfo>,

/// Time since the start of the transaction until the error occurred.
pub time_spent: Annotated<u64>,

Expand Down
2 changes: 2 additions & 0 deletions relay-general/src/protocol/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ mod stacktrace;
mod tags;
mod templateinfo;
mod thread;
mod transaction;
mod types;
mod user;
mod user_report;
Expand Down Expand Up @@ -68,6 +69,7 @@ pub use self::stacktrace::{Frame, FrameData, FrameVars, RawStacktrace, Stacktrac
pub use self::tags::{TagEntry, Tags};
pub use self::templateinfo::TemplateInfo;
pub use self::thread::{Thread, ThreadId};
pub use self::transaction::{TransactionInfo, TransactionSource};
pub use self::types::{
datetime_to_timestamp, Addr, AsPair, InvalidRegVal, IpAddr, JsonLenientString, LenientString,
Level, PairList, ParseLevelError, RegVal, Timestamp, Values,
Expand Down
157 changes: 157 additions & 0 deletions relay-general/src/protocol/transaction.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
use std::fmt;
use std::str::FromStr;

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

/// Describes how the name of the transaction was determined.
#[derive(Clone, Debug, Eq, PartialEq)]
#[cfg_attr(feature = "jsonschema", derive(schemars::JsonSchema))]
#[cfg_attr(feature = "jsonschema", schemars(rename_all = "kebab-case"))]
pub enum TransactionSource {
/// User-defined name set through `set_transaction_name`.
Custom,
/// Raw URL, potentially containing identifiers.
Url,
/// Parametrized URL or route.
Route,
/// Name of the view handling the request.
View,
/// Named after a software component, such as a function or class name.
Component,
/// Name of a background task (e.g. a Celery task).
Task,
/// This is the default value set by Relay for legacy SDKs.
Unknown,
/// Any other unknown source that is not explicitly defined above.
#[cfg_attr(feature = "jsonschema", schemars(skip))]
Other(String),
}

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

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"custom" => Ok(Self::Custom),
"url" => Ok(Self::Url),
"route" => Ok(Self::Route),
"view" => Ok(Self::View),
"component" => Ok(Self::Component),
"task" => Ok(Self::Task),
"unknown" => Ok(Self::Unknown),
s => Ok(Self::Other(s.to_owned())),
}
}
}

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),
}
}
}

impl Default for TransactionSource {
fn default() -> Self {
Self::Unknown
}
}

impl Empty for TransactionSource {
#[inline]
fn is_empty(&self) -> bool {
matches!(self, Self::Unknown)
}
}

impl FromValue for TransactionSource {
fn from_value(value: Annotated<Value>) -> Annotated<Self> {
match String::from_value(value) {
Annotated(Some(value), mut meta) => match value.parse() {
Ok(source) => Annotated(Some(source), meta),
Err(_) => {
meta.add_error(ErrorKind::InvalidData);
meta.set_original_value(Some(value));
Annotated(None, meta)
}
},
Annotated(None, meta) => Annotated(None, meta),
}
}
}

impl IntoValue for TransactionSource {
fn into_value(self) -> Value
where
Self: Sized,
{
Value::String(self.to_string())
}

fn serialize_payload<S>(&self, s: S, _behavior: SkipSerialization) -> Result<S::Ok, S::Error>
where
Self: Sized,
S: serde::Serializer,
{
serde::Serialize::serialize(&self.to_string(), s)
}
}

impl ProcessValue for TransactionSource {}

/// Additional information about the name of the transaction.
#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
#[cfg_attr(feature = "jsonschema", derive(JsonSchema))]
pub struct TransactionInfo {
/// Describes how the name of the transaction was determined.
///
/// This will be used by the server to decide whether or not to scrub identifiers from the
/// transaction name, or replace the entire name with a placeholder.
pub source: Annotated<TransactionSource>,

/// The unmodified transaction name as obtained by the source.
///
/// 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>,
}

#[cfg(test)]
mod tests {
use super::*;
use crate::testutils;

#[test]
fn test_other_source_roundtrip() {
let json = r#""something-new""#;
let source = Annotated::new(TransactionSource::Other("something-new".to_owned()));

testutils::assert_eq_dbg!(source, Annotated::from_json(json).unwrap());
testutils::assert_eq_str!(json, source.payload_to_json_pretty().unwrap());
}

#[test]
fn test_transaction_info_roundtrip() {
let json = r#"{
"source": "url",
"original": "/auth/login/john123/"
}"#;

let info = Annotated::new(TransactionInfo {
source: Annotated::new(TransactionSource::Url),
original: Annotated::new("/auth/login/john123/".to_owned()),
});

testutils::assert_eq_dbg!(info, Annotated::from_json(json).unwrap());
testutils::assert_eq_str!(json, info.to_json_pretty().unwrap());
}
}
57 changes: 57 additions & 0 deletions relay-general/tests/snapshots/test_fixtures__event_schema.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: relay-general/tests/test_fixtures.rs
assertion_line: 106
expression: event_json_schema()
---
{
Expand Down Expand Up @@ -347,6 +348,18 @@ expression: event_json_schema()
"null"
]
},
"transaction_info": {
"description": " Additional information about the name of the transaction.",
"default": null,
"anyOf": [
{
"$ref": "#/definitions/TransactionInfo"
},
{
"type": "null"
}
]
},
"type": {
"description": " Type of the event. Defaults to `default`.\n\n The event type determines how Sentry handles the event and has an impact on processing, rate\n limiting, and quotas. There are three fundamental classes of event types:\n\n - **Error monitoring events**: Processed and grouped into unique issues based on their\n exception stack traces and error messages.\n - **Security events**: Derived from Browser security violation reports and grouped into\n unique issues based on the endpoint and violation. SDKs do not send such events.\n - **Transaction events** (`transaction`): Contain operation spans and collected into traces\n for performance monitoring.\n\n Transactions must explicitly specify the `\"transaction\"` event type. In all other cases,\n Sentry infers the appropriate event type from the payload and overrides the stated type.\n SDKs should not send an event type other than for transactions.\n\n Example:\n\n ```json\n {\n \"type\": \"transaction\",\n \"spans\": []\n }\n ```",
"default": null,
Expand Down Expand Up @@ -2803,6 +2816,50 @@ expression: event_json_schema()
}
]
},
"TransactionInfo": {
"description": " Additional information about the name of the transaction.",
"anyOf": [
{
"type": "object",
"properties": {
"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,
"type": [
"string",
"null"
]
},
"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,
"anyOf": [
{
"$ref": "#/definitions/TransactionSource"
},
{
"type": "null"
}
]
}
},
"additionalProperties": false
}
]
},
"TransactionSource": {
"description": "Describes how the name of the transaction was determined.",
"type": "string",
"enum": [
"custom",
"url",
"route",
"view",
"component",
"task",
"unknown"
]
},
"User": {
"description": " Information about the user who triggered an event.\n\n ```json\n {\n \"user\": {\n \"id\": \"unique_id\",\n \"username\": \"my_user\",\n \"email\": \"foo@example.com\",\n \"ip_address\": \"127.0.0.1\",\n \"subscription\": \"basic\"\n }\n }\n ```",
"anyOf": [
Expand Down
17 changes: 16 additions & 1 deletion relay-server/src/actors/envelopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use relay_general::processor::{process_value, ProcessingState};
use relay_general::protocol::{
self, Breadcrumb, ClientReport, Csp, Event, EventId, EventType, ExpectCt, ExpectStaple, Hpkp,
IpAddr, LenientString, Metrics, RelayInfo, SecurityReportType, SessionAggregates,
SessionAttributes, SessionUpdate, Timestamp, UserReport, Values,
SessionAttributes, SessionUpdate, Timestamp, TransactionSource, UserReport, Values,
};
use relay_general::store::ClockDriftProcessor;
use relay_general::types::{Annotated, Array, FromValue, Object, ProcessingAction, Value};
Expand Down Expand Up @@ -1520,6 +1520,21 @@ impl EnvelopeProcessor {
}

event._metrics = Annotated::new(metrics);

if event.ty.value() == Some(&EventType::Transaction) {
let source = event
.transaction_info
.value()
.and_then(|info| info.source.value())
.unwrap_or(&TransactionSource::Unknown);

metric!(
counter(RelayCounters::EventTransactionSource) += 1,
source = &source.to_string(),
sdk = envelope.meta().client_name().unwrap_or("proprietary"),
platform = event.platform.as_str().unwrap_or("other"),
);
}
}

// TODO: Temporary workaround before processing. Experimental SDKs relied on a buggy
Expand Down
11 changes: 11 additions & 0 deletions relay-server/src/extractors/request_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,21 @@ pub struct RequestMeta<D = PartialDsn> {

impl<D> RequestMeta<D> {
/// Returns the client that sent this event (Sentry SDK identifier).
///
/// The client is formatted as `"sdk/version"`, for example `"raven-node/2.6.3"`.
pub fn client(&self) -> Option<&str> {
self.client.as_deref()
}

/// Returns the name of the client that sent the event without version.
///
/// If the client is not sent in standard format, this method returns `None`.
pub fn client_name(&self) -> Option<&str> {
let client = self.client()?;
let (name, _version) = client.split_once('/')?;
Some(name)
}

/// Returns the protocol version of the event payload.
pub fn version(&self) -> u16 {
self.version
Expand Down
11 changes: 11 additions & 0 deletions relay-server/src/statsd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,16 @@ pub enum RelayCounters {
/// This metric is tagged with:
/// - `version`: The event protocol version number defaulting to `7`.
EventProtocol,
/// The number of transaction events processed by the source of the transaction name.
///
/// This metric is tagged with:
/// - `platform`: The event's platform, such as `"javascript"`.
/// - `sdk`: The name of the Sentry SDK sending the transaction. This tag is only set for
/// Sentry's SDKs and defaults to "proprietary".
/// - `source`: The source of the transaction name on the client. See the [transaction source
/// documentation](https://develop.sentry.dev/sdk/event-payloads/properties/transaction_info/)
/// for all valid values.
EventTransactionSource,
/// Number of HTTP requests reaching Relay.
Requests,
/// Number of completed HTTP requests.
Expand Down Expand Up @@ -496,6 +506,7 @@ impl CounterMetric for RelayCounters {
#[cfg(feature = "processing")]
RelayCounters::ProcessingProduceError => "processing.produce.error",
RelayCounters::EventProtocol => "event.protocol",
RelayCounters::EventTransactionSource => "event.transaction_source",
RelayCounters::Requests => "requests",
RelayCounters::ResponsesStatusCodes => "responses.status_codes",
RelayCounters::EvictingStaleProjectCaches => "project_cache.eviction",
Expand Down

0 comments on commit 135eb86

Please sign in to comment.