From 559cb77f17796dc36e1417ac1e25d1179ea341ad Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Tue, 17 Oct 2023 17:26:28 -0400 Subject: [PATCH 01/59] feat(otel): Add an endpoint to ingest OpenTelemetry spans --- Cargo.lock | 12 ++++ credentials.json | 5 -- relay-dynamic-config/src/feature.rs | 3 + relay-server/Cargo.toml | 1 + relay-server/src/actors/processor.rs | 24 ++++++++ relay-server/src/endpoints/mod.rs | 2 + relay-server/src/endpoints/spans.rs | 59 ++++++++++++++++++ relay-spans/Cargo.toml | 17 ++++++ relay-spans/src/lib.rs | 90 ++++++++++++++++++++++++++++ 9 files changed, 208 insertions(+), 5 deletions(-) delete mode 100644 credentials.json create mode 100644 relay-server/src/endpoints/spans.rs create mode 100644 relay-spans/Cargo.toml create mode 100644 relay-spans/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index e40cbbb76c..a91b6f3e91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3888,6 +3888,7 @@ dependencies = [ "relay-redis", "relay-replays", "relay-sampling", + "relay-spans", "relay-statsd", "relay-system", "relay-test", @@ -3911,6 +3912,17 @@ dependencies = [ "zstd", ] +[[package]] +name = "relay-spans" +version = "23.9.1" +dependencies = [ + "enumset", + "relay-event-schema", + "relay-protocol", + "serde", + "serde_json", +] + [[package]] name = "relay-statsd" version = "23.9.1" diff --git a/credentials.json b/credentials.json deleted file mode 100644 index 596e187bcd..0000000000 --- a/credentials.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "secret_key": "EZHJ2G0BC0sJFs5sD8j1I-tSosLAD7jtEwM28lnEZ7o", - "public_key": "e5S5Il0IYcgHM1bQ5-5Z7k8AY0yCsajt2g0onLAH0SU", - "id": "dad490a4-cb19-4b90-8d64-e23a252a9e79" -} diff --git a/relay-dynamic-config/src/feature.rs b/relay-dynamic-config/src/feature.rs index 7e8de391b2..0a41946b2a 100644 --- a/relay-dynamic-config/src/feature.rs +++ b/relay-dynamic-config/src/feature.rs @@ -31,6 +31,9 @@ pub enum Feature { /// Enable extracting resource spans. #[serde(rename = "projects:span-metrics-extraction-resource")] SpanMetricsExtractionResource, + /// Enable OTel span ingestion. + #[serde(rename = "organizations:otel-span-ingestion")] + OTelSpanIngestion, /// Deprecated, still forwarded for older downstream Relays. #[serde(rename = "organizations:transaction-name-mark-scrubbed-as-sanitized")] diff --git a/relay-server/Cargo.toml b/relay-server/Cargo.toml index b76c2d0b8c..dea31e7011 100644 --- a/relay-server/Cargo.toml +++ b/relay-server/Cargo.toml @@ -78,6 +78,7 @@ relay-quotas = { path = "../relay-quotas" } relay-redis = { path = "../relay-redis" } relay-replays = { path = "../relay-replays" } relay-sampling = { path = "../relay-sampling" } +relay-spans = { path = "../relay-spans" } relay-statsd = { path = "../relay-statsd" } relay-system = { path = "../relay-system" } reqwest = { version = "0.11.1", features = [ diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 9e7969fee9..8981af0858 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2427,6 +2427,29 @@ impl EnvelopeProcessorService { Ok(span) } + #[cfg(feature = "processing")] + fn process_spans(&self, state: &mut ProcessEnvelopeState) { + let otel_span_ingestion_enabled = + state.project_state.has_feature(Feature::OTelSpanIngestion); + state.managed_envelope.retain_items(|item| match item.ty() { + ItemType::Span if !otel_span_ingestion_enabled => ItemAction::DropSilently, + ItemType::Span => match Annotated::::from_json_bytes(&item.payload()) { + Ok(span) => match self.validate_span(span) { + Ok(_) => ItemAction::Keep, + Err(e) => { + relay_log::error!("Invalid span: {e}"); + ItemAction::DropSilently + } + }, + Err(e) => { + relay_log::error!("Invalid span: {e}"); + ItemAction::DropSilently + } + }, + _ => ItemAction::Keep, + }); + } + /// Computes the sampling decision on the incoming event fn run_dynamic_sampling(&self, state: &mut ProcessEnvelopeState) { // Running dynamic sampling involves either: @@ -2668,6 +2691,7 @@ impl EnvelopeProcessorService { self.process_user_reports(state); self.process_replays(state)?; self.filter_profiles(state); + self.process_spans(state); if state.creates_event() { // Some envelopes only create events in processing relays; for example, unreal events. diff --git a/relay-server/src/endpoints/mod.rs b/relay-server/src/endpoints/mod.rs index e887ae10f9..6d7104c841 100644 --- a/relay-server/src/endpoints/mod.rs +++ b/relay-server/src/endpoints/mod.rs @@ -19,6 +19,7 @@ mod outcomes; mod project_configs; mod public_keys; mod security_report; +mod spans; mod statics; #[cfg(feature = "dashboard")] mod stats; @@ -83,6 +84,7 @@ where .route("/api/:project_id/minidump/", minidump::route(config)) .route("/api/:project_id/events/:event_id/attachments/", attachments::route(config)) .route("/api/:project_id/unreal/:sentry_key/", unreal::route(config)) + .route("/api/:project_id/spans/", spans::route(config)) .route_layer(middlewares::cors()); let router = Router::new(); diff --git a/relay-server/src/endpoints/spans.rs b/relay-server/src/endpoints/spans.rs new file mode 100644 index 0000000000..aa46edd14b --- /dev/null +++ b/relay-server/src/endpoints/spans.rs @@ -0,0 +1,59 @@ +use axum::extract; +use axum::http::{Request, StatusCode}; +use axum::response::IntoResponse; +use axum::routing::{post, MethodRouter}; +use axum::RequestExt; +use bytes::Bytes; + +use relay_config::Config; +use relay_event_schema::protocol::{EventId, Span as EventSpan}; +use relay_protocol::Annotated; +use relay_spans::Span; + +use crate::endpoints::common::{self, BadStoreRequest}; +use crate::envelope::{ContentType, Envelope, Item, ItemType}; +use crate::extractors::{RawContentType, RequestMeta}; +use crate::service::ServiceState; + +async fn handle( + state: ServiceState, + content_type: RawContentType, + meta: RequestMeta, + request: Request, +) -> axum::response::Result +where + B: axum::body::HttpBody + Send + 'static, + B::Data: Send + Into, + B::Error: Into, +{ + if !content_type.as_ref().starts_with("application/json") { + return Ok(StatusCode::ACCEPTED); + } + + // Parse OTel span. + let payload: String = request.extract().await?; + let span: Annotated = + Annotated::from_json(payload.as_str()).map_err(BadStoreRequest::InvalidJson)?; + + // Convert to an Event span. + let event_span: Annotated = Annotated::new(span.value().unwrap().into()); + + // Pack into an envelope for further processing. + let span_json = event_span.to_json().map_err(BadStoreRequest::InvalidJson)?; + let mut item = Item::new(ItemType::Span); + item.set_payload(ContentType::Json, span_json); + let mut envelope = Envelope::from_request(Some(EventId::new()), meta); + envelope.add_item(item); + common::handle_envelope(&state, envelope).await?; + + Ok(StatusCode::ACCEPTED) +} + +pub fn route(config: &Config) -> MethodRouter +where + B: axum::body::HttpBody + Send + 'static, + B::Data: Send + Into, + B::Error: Into, +{ + post(handle).route_layer(extract::DefaultBodyLimit::max(config.max_span_size())) +} diff --git a/relay-spans/Cargo.toml b/relay-spans/Cargo.toml new file mode 100644 index 0000000000..c14a5efe39 --- /dev/null +++ b/relay-spans/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "relay-spans" +authors = ["Sentry "] +description = "Event normalization and processing" +homepage = "https://getsentry.github.io/relay/" +repository = "https://github.com/getsentry/relay" +version = "23.9.1" +edition = "2021" +license-file = "../LICENSE" +publish = false + +[dependencies] +enumset = "1.0.4" +relay-event-schema = { path = "../relay-event-schema" } +relay-protocol = { path = "../relay-protocol" } +serde = { workspace = true } +serde_json = { workspace = true } diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs new file mode 100644 index 0000000000..ccf8aa5151 --- /dev/null +++ b/relay-spans/src/lib.rs @@ -0,0 +1,90 @@ +#![allow(dead_code, unused_variables)] +use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Object, Value}; + +use relay_event_schema::protocol::{Span as EventSpan, SpanId, Timestamp, TraceId}; + +#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue)] +#[metastructure(process_func = "process_span", value_type = "Span")] +pub struct Span { + #[metastructure(required = "true")] + pub end_time: Annotated, + #[metastructure(required = "true")] + pub start_time: Annotated, + #[metastructure(pii = "maybe")] + pub name: Annotated, + #[metastructure(required = "true")] + pub span_id: Annotated, + #[metastructure(required = "true")] + pub trace_id: Annotated, + #[metastructure(pii = "maybe")] + pub attributes: Annotated>, + #[metastructure(additional_properties, retain = "true", pii = "maybe")] + pub other: Object, +} + +impl From<&Span> for EventSpan { + fn from(from: &Span) -> Self { + let mut span = EventSpan { + description: from.name.clone(), + span_id: from.span_id.clone(), + trace_id: from.trace_id.clone(), + ..Default::default() + }; + let mut sentry_tags: Object = Default::default(); + let mut remaining_attributes: Object = Default::default(); + + if let Some(attributes) = from.attributes.clone().value_mut() { + if let Some(environment) = attributes.remove::("sentry.environment") { + if let Some(env) = environment.value() { + sentry_tags.insert( + "environment".into(), + Annotated::new(env.as_str().unwrap().to_string()), + ); + } + } + + for (key, value) in attributes { + remaining_attributes.insert(key.into(), value.clone()); + } + } + + span.data = Annotated::new(remaining_attributes); + span.sentry_tags = Annotated::new(sentry_tags); + + span + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_span() { + let json = r#"{ + "trace_id": "75E054B31042428F874AAB5188E3D666", + "span_id": "deadbeefdeadbeef", + "span_kind": "somekind", + "name": "GET https://google.com/api", + "start_time": 1697490227.0, + "end_time": 1697490237.0, + "attributes": { + "op": "http.client", + "sentry.environment": "test" + } + }"#; + let otel_span: Annotated = Annotated::from_json(json).unwrap(); + let event_span: Annotated = Annotated::new(otel_span.value().unwrap().into()); + assert_eq!( + event_span + .value() + .unwrap() + .sentry_tags + .value() + .unwrap() + .get("environment") + .unwrap(), + &Annotated::new("test".to_string()) + ); + } +} From 4b09e95f6875a3b4a55286b7ea9f313713076557 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Tue, 17 Oct 2023 17:35:27 -0400 Subject: [PATCH 02/59] Move process_spans under the processing macro --- relay-server/src/actors/processor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 8981af0858..26aea8e781 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2691,7 +2691,6 @@ impl EnvelopeProcessorService { self.process_user_reports(state); self.process_replays(state)?; self.filter_profiles(state); - self.process_spans(state); if state.creates_event() { // Some envelopes only create events in processing relays; for example, unreal events. @@ -2733,6 +2732,7 @@ impl EnvelopeProcessorService { // We need the event parsed in order to set the profile context on it self.process_profiles(state); self.process_check_ins(state); + self.process_spans(state); }); if state.has_event() { From c86a98d01a25ba20c653e1a5aad1c6a8ac8cf956 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Wed, 18 Oct 2023 18:57:57 -0400 Subject: [PATCH 03/59] Ingest a real OTLP span schema --- Cargo.lock | 51 +++-- relay-server/src/actors/processor.rs | 4 +- relay-server/src/endpoints/spans.rs | 28 +-- relay-spans/Cargo.toml | 2 + relay-spans/src/lib.rs | 268 +++++++++++++++++++++------ 5 files changed, 252 insertions(+), 101 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8f779c8513..de3a300a61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -175,7 +175,7 @@ checksum = "b9ccdd8f2a161be9bd5c023df56f1b2a0bd1d83872ae53b71a84a12c9bf6e842" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -259,7 +259,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -857,7 +857,7 @@ checksum = "83fdaf97f4804dcebfa5862639bc9ce4121e82140bec2a987ac5140294865b5b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -924,7 +924,7 @@ dependencies = [ "ident_case", "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -935,7 +935,7 @@ checksum = "836a9bbc7ad63342d6d6e7b815ccab164bc77a2d95d84bc3117a8c0d5c98e2d5" dependencies = [ "darling_core", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -1028,7 +1028,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml 0.9.17", - "syn 2.0.16", + "syn 2.0.32", "walkdir", ] @@ -1220,7 +1220,7 @@ dependencies = [ "darling", "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -1442,7 +1442,7 @@ checksum = "89ca545a94061b6365f2c7355b4b32bd20df3ff95f02da9329b34ccc3bd6ee72" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -2921,7 +2921,7 @@ dependencies = [ "pest_meta", "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -3157,9 +3157,9 @@ checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" [[package]] name = "quote" -version = "1.0.26" +version = "1.0.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4424af4bf778aae2051a77b60283332f386554255d722233d09fbfc7e30da2fc" +checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae" dependencies = [ "proc-macro2", ] @@ -3916,11 +3916,13 @@ dependencies = [ name = "relay-spans" version = "23.9.1" dependencies = [ + "chrono", "enumset", "relay-event-schema", "relay-protocol", "serde", "serde_json", + "serde_repr", ] [[package]] @@ -4103,7 +4105,7 @@ dependencies = [ "proc-macro2", "quote", "rust-embed-utils", - "syn 2.0.16", + "syn 2.0.32", "walkdir", ] @@ -4484,7 +4486,7 @@ checksum = "4c614d17805b093df4b147b51339e7e44bf05ef59fba1e45d83500bcfb4d8585" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -4518,6 +4520,17 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_repr" +version = "0.1.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8725e1dfadb3a50f7e5ce0b1a540466f6ed3fe7a0fca2ac2b8b831d31316bd00" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.32", +] + [[package]] name = "serde_test" version = "1.0.152" @@ -5042,9 +5055,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.16" +version = "2.0.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a6f671d4b5ffdb8eadec19c0ae67fe2639df8684bd7bc4b83d986b8db549cf01" +checksum = "239814284fd6f1a4ffe4ca893952cdd93c224b6a1571c9a9eadd670295c0c9e2" dependencies = [ "proc-macro2", "quote", @@ -5118,7 +5131,7 @@ checksum = "f9456a42c5b0d803c8cd86e73dd7cc9edd429499f37a3550d286d5e86720569f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -5230,7 +5243,7 @@ checksum = "630bdcf245f78637c13ec01ffae6187cca34625e8c63150d424b59e55af2675e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", ] [[package]] @@ -5708,7 +5721,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", "wasm-bindgen-shared", ] @@ -5742,7 +5755,7 @@ checksum = "54681b18a46765f095758388f2d0cf16eb8d4169b639ab575a8f5693af210c7b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.32", "wasm-bindgen-backend", "wasm-bindgen-shared", ] diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 26aea8e781..66c03c7845 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2429,8 +2429,8 @@ impl EnvelopeProcessorService { #[cfg(feature = "processing")] fn process_spans(&self, state: &mut ProcessEnvelopeState) { - let otel_span_ingestion_enabled = - state.project_state.has_feature(Feature::OTelSpanIngestion); + let otel_span_ingestion_enabled = true; + //state.project_state.has_feature(Feature::OTelSpanIngestion); state.managed_envelope.retain_items(|item| match item.ty() { ItemType::Span if !otel_span_ingestion_enabled => ItemAction::DropSilently, ItemType::Span => match Annotated::::from_json_bytes(&item.payload()) { diff --git a/relay-server/src/endpoints/spans.rs b/relay-server/src/endpoints/spans.rs index aa46edd14b..d34c8d61a5 100644 --- a/relay-server/src/endpoints/spans.rs +++ b/relay-server/src/endpoints/spans.rs @@ -1,8 +1,7 @@ use axum::extract; -use axum::http::{Request, StatusCode}; +use axum::http::StatusCode; use axum::response::IntoResponse; use axum::routing::{post, MethodRouter}; -use axum::RequestExt; use bytes::Bytes; use relay_config::Config; @@ -12,31 +11,18 @@ use relay_spans::Span; use crate::endpoints::common::{self, BadStoreRequest}; use crate::envelope::{ContentType, Envelope, Item, ItemType}; -use crate::extractors::{RawContentType, RequestMeta}; +use crate::extractors::RequestMeta; use crate::service::ServiceState; -async fn handle( +async fn handle( state: ServiceState, - content_type: RawContentType, meta: RequestMeta, - request: Request, -) -> axum::response::Result -where - B: axum::body::HttpBody + Send + 'static, - B::Data: Send + Into, - B::Error: Into, -{ - if !content_type.as_ref().starts_with("application/json") { - return Ok(StatusCode::ACCEPTED); - } - - // Parse OTel span. - let payload: String = request.extract().await?; - let span: Annotated = - Annotated::from_json(payload.as_str()).map_err(BadStoreRequest::InvalidJson)?; + extract::Json(span): extract::Json, +) -> Result { + println!("{:#?}", span); // Convert to an Event span. - let event_span: Annotated = Annotated::new(span.value().unwrap().into()); + let event_span: Annotated = Annotated::new(span.into()); // Pack into an envelope for further processing. let span_json = event_span.to_json().map_err(BadStoreRequest::InvalidJson)?; diff --git a/relay-spans/Cargo.toml b/relay-spans/Cargo.toml index c14a5efe39..1809a07504 100644 --- a/relay-spans/Cargo.toml +++ b/relay-spans/Cargo.toml @@ -10,8 +10,10 @@ license-file = "../LICENSE" publish = false [dependencies] +chrono.workspace = true enumset = "1.0.4" relay-event-schema = { path = "../relay-event-schema" } relay-protocol = { path = "../relay-protocol" } serde = { workspace = true } serde_json = { workspace = true } +serde_repr = "0.1.16" diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index ccf8aa5151..bef7d449eb 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -1,80 +1,230 @@ -#![allow(dead_code, unused_variables)] -use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Object, Value}; +use chrono::{TimeZone, Utc}; + +use serde::Deserialize; +use serde_repr::Deserialize_repr; use relay_event_schema::protocol::{Span as EventSpan, SpanId, Timestamp, TraceId}; -#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue)] -#[metastructure(process_func = "process_span", value_type = "Span")] +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] pub struct Span { - #[metastructure(required = "true")] - pub end_time: Annotated, - #[metastructure(required = "true")] - pub start_time: Annotated, - #[metastructure(pii = "maybe")] - pub name: Annotated, - #[metastructure(required = "true")] - pub span_id: Annotated, - #[metastructure(required = "true")] - pub trace_id: Annotated, - #[metastructure(pii = "maybe")] - pub attributes: Annotated>, - #[metastructure(additional_properties, retain = "true", pii = "maybe")] - pub other: Object, -} - -impl From<&Span> for EventSpan { - fn from(from: &Span) -> Self { - let mut span = EventSpan { - description: from.name.clone(), - span_id: from.span_id.clone(), - trace_id: from.trace_id.clone(), - ..Default::default() - }; - let mut sentry_tags: Object = Default::default(); - let mut remaining_attributes: Object = Default::default(); - - if let Some(attributes) = from.attributes.clone().value_mut() { - if let Some(environment) = attributes.remove::("sentry.environment") { - if let Some(env) = environment.value() { - sentry_tags.insert( - "environment".into(), - Annotated::new(env.as_str().unwrap().to_string()), - ); - } - } - - for (key, value) in attributes { - remaining_attributes.insert(key.into(), value.clone()); - } + pub trace_id: String, + pub span_id: String, + pub trace_state: Option, + pub parent_span_id: String, + pub name: String, + pub kind: SpanKind, + pub start_time_unix_nano: i64, + pub end_time_unix_nano: i64, + pub attributes: Vec, + pub dropped_attributes_count: u32, + pub events: Vec, + pub dropped_events_count: u32, + pub links: Vec, + pub dropped_links_count: u32, + pub status: Option, +} + +#[derive(Debug, Deserialize)] +pub struct Event { + pub time_unix_nano: u64, + pub name: String, + pub attributes: Vec, + pub dropped_attributes_count: u32, +} + +#[derive(Debug, Deserialize)] +pub struct Link { + pub trace_id: String, + pub span_id: String, + pub trace_state: String, + pub attributes: Vec, + pub dropped_attributes_count: u32, +} + +#[derive(Debug, Deserialize_repr)] +#[repr(u32)] +pub enum SpanKind { + Unspecified = 0, + Internal = 1, + Server = 2, + Client = 3, + Producer = 4, + Consumer = 5, +} + +impl SpanKind { + pub fn as_str_name(&self) -> &'static str { + match self { + SpanKind::Unspecified => "SPAN_KIND_UNSPECIFIED", + SpanKind::Internal => "SPAN_KIND_INTERNAL", + SpanKind::Server => "SPAN_KIND_SERVER", + SpanKind::Client => "SPAN_KIND_CLIENT", + SpanKind::Producer => "SPAN_KIND_PRODUCER", + SpanKind::Consumer => "SPAN_KIND_CONSUMER", } + } + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "SPAN_KIND_UNSPECIFIED" => Some(Self::Unspecified), + "SPAN_KIND_INTERNAL" => Some(Self::Internal), + "SPAN_KIND_SERVER" => Some(Self::Server), + "SPAN_KIND_CLIENT" => Some(Self::Client), + "SPAN_KIND_PRODUCER" => Some(Self::Producer), + "SPAN_KIND_CONSUMER" => Some(Self::Consumer), + _ => None, + } + } +} - span.data = Annotated::new(remaining_attributes); - span.sentry_tags = Annotated::new(sentry_tags); +#[derive(Debug, Deserialize)] +pub struct Status { + pub message: Option, + pub code: StatusCode, +} - span +#[derive(Debug, Deserialize_repr)] +#[repr(u32)] +pub enum StatusCode { + Unset = 0, + Ok = 1, + Error = 2, +} + +impl StatusCode { + pub fn as_str_name(&self) -> &'static str { + match self { + StatusCode::Unset => "STATUS_CODE_UNSET", + StatusCode::Ok => "STATUS_CODE_OK", + StatusCode::Error => "STATUS_CODE_ERROR", + } + } + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "STATUS_CODE_UNSET" => Some(Self::Unset), + "STATUS_CODE_OK" => Some(Self::Ok), + "STATUS_CODE_ERROR" => Some(Self::Error), + _ => None, + } + } +} + +#[derive(Debug, Deserialize)] +pub struct AnyValue { + pub value: Option, +} + +#[derive(Debug, Deserialize)] +pub enum Value { + StringValue(String), + BoolValue(bool), + IntValue(i64), + DoubleValue(f64), + ArrayValue(ArrayValue), + KvlistValue(KeyValueList), + BytesValue(Vec), +} + +#[derive(Debug, Deserialize)] +pub struct ArrayValue { + pub values: Vec, +} + +#[derive(Debug, Deserialize)] +pub struct KeyValueList { + pub values: Vec, +} + +#[derive(Debug, Deserialize)] +pub struct KeyValue { + pub key: String, + pub value: Option, +} + +#[derive(Debug, Deserialize)] +pub struct InstrumentationScope { + pub name: String, + pub version: String, + pub attributes: Vec, + pub dropped_attributes_count: u32, +} + +impl From for EventSpan { + fn from(from: Span) -> Self { + let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); + let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); + let exclusive_time = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; + Self { + description: from.name.into(), + exclusive_time: exclusive_time.into(), + parent_span_id: SpanId(from.parent_span_id).into(), + span_id: SpanId(from.span_id).into(), + start_timestamp: Timestamp(start_timestamp).into(), + timestamp: Timestamp(end_timestamp).into(), + trace_id: TraceId(from.trace_id).into(), + ..Default::default() + } } } #[cfg(test)] mod tests { use super::*; + use relay_protocol::Annotated; #[test] fn parse_span() { let json = r#"{ - "trace_id": "75E054B31042428F874AAB5188E3D666", - "span_id": "deadbeefdeadbeef", - "span_kind": "somekind", - "name": "GET https://google.com/api", - "start_time": 1697490227.0, - "end_time": 1697490237.0, - "attributes": { - "op": "http.client", - "sentry.environment": "test" - } + "traceId": "89143b0763095bd9c9955e8175d1fb23", + "spanId": "e342abb1214ca181", + "parentSpanId": "0c7a7dea069bf5a6", + "name": "middleware - fastify -> @fastify/multipart", + "kind": 1, + "startTimeUnixNano": 1697620454980000000, + "endTimeUnixNano": 1697620454980078800, + "attributes": [ + { + "key": "fastify.type", + "value": { + "stringValue": "middleware" + } + }, + { + "key": "plugin.name", + "value": { + "stringValue": "fastify -> @fastify/multipart" + } + }, + { + "key": "hook.name", + "value": { + "stringValue": "onResponse" + } + }, + { + "key": "sentry.sample_rate", + "value": { + "intValue": 1 + } + }, + { + "key": "sentry.parentSampled", + "value": { + "boolValue": true + } + } + ], + "droppedAttributesCount": 0, + "events": [], + "droppedEventsCount": 0, + "status": { + "code": 0 + }, + "links": [], + "droppedLinksCount": 0 }"#; - let otel_span: Annotated = Annotated::from_json(json).unwrap(); - let event_span: Annotated = Annotated::new(otel_span.value().unwrap().into()); + let otel_span: Span = serde_json::from_str(json).unwrap(); + let event_span: Annotated = Annotated::new(otel_span.into()); assert_eq!( event_span .value() From a70485972f18f5e958e8732a5cba7ac04e27ba16 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Wed, 18 Oct 2023 19:05:32 -0400 Subject: [PATCH 04/59] Handle parent_span_id and segment_id --- relay-spans/src/lib.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index bef7d449eb..24c807a441 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -11,7 +11,7 @@ pub struct Span { pub trace_id: String, pub span_id: String, pub trace_state: Option, - pub parent_span_id: String, + pub parent_span_id: Option, pub name: String, pub kind: SpanKind, pub start_time_unix_nano: i64, @@ -154,16 +154,23 @@ impl From for EventSpan { let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); let exclusive_time = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; - Self { + let mut span = EventSpan { description: from.name.into(), exclusive_time: exclusive_time.into(), - parent_span_id: SpanId(from.parent_span_id).into(), span_id: SpanId(from.span_id).into(), start_timestamp: Timestamp(start_timestamp).into(), timestamp: Timestamp(end_timestamp).into(), trace_id: TraceId(from.trace_id).into(), ..Default::default() + }; + if let Some(parent_span_id) = from.parent_span_id { + span.is_segment = false.into(); + span.parent_span_id = SpanId(parent_span_id).into(); + } else { + span.is_segment = true.into(); + span.segment_id = span.span_id.clone(); } + span } } From 24bca6be6dc1e39e68360cd89678c4f3e2e9ee7d Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Thu, 19 Oct 2023 13:03:20 -0400 Subject: [PATCH 05/59] Emit OtelSpan from the HTTP handler and parse them when processing --- relay-server/src/actors/processor.rs | 41 ++++++++++++++++++++------- relay-server/src/endpoints/spans.rs | 24 ++++++++-------- relay-server/src/envelope.rs | 10 +++++-- relay-server/src/utils/rate_limits.rs | 1 + relay-server/src/utils/sizes.rs | 5 ++++ 5 files changed, 56 insertions(+), 25 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 66c03c7845..ea809e691d 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -1662,6 +1662,7 @@ impl EnvelopeProcessorService { ItemType::ReplayRecording => false, ItemType::CheckIn => false, ItemType::Span => false, + ItemType::OtelSpan => false, // Without knowing more, `Unknown` items are allowed to be repeated ItemType::Unknown(_) => false, @@ -2429,25 +2430,43 @@ impl EnvelopeProcessorService { #[cfg(feature = "processing")] fn process_spans(&self, state: &mut ProcessEnvelopeState) { - let otel_span_ingestion_enabled = true; - //state.project_state.has_feature(Feature::OTelSpanIngestion); + let mut items: Vec = Vec::new(); state.managed_envelope.retain_items(|item| match item.ty() { - ItemType::Span if !otel_span_ingestion_enabled => ItemAction::DropSilently, - ItemType::Span => match Annotated::::from_json_bytes(&item.payload()) { - Ok(span) => match self.validate_span(span) { - Ok(_) => ItemAction::Keep, + ItemType::OtelSpan => { + let span: relay_spans::Span = match serde_json::from_slice(&item.payload()) { + Ok(span) => span, + Err(e) => { + relay_log::error!("Invalid span: {e}"); + return ItemAction::DropSilently; + } + }; + let event_span: Annotated = Annotated::new(span.into()); + match self.validate_span(event_span) { + Ok(span) => { + let payload: Bytes = match span.to_json() { + Ok(payload) => payload.into(), + Err(e) => { + relay_log::error!("Invalid span: {e}"); + return ItemAction::DropSilently; + } + }; + let mut new_item = Item::new(ItemType::Span); + new_item.set_payload(ContentType::Json, payload); + items.push(new_item); + ItemAction::DropSilently + } Err(e) => { relay_log::error!("Invalid span: {e}"); ItemAction::DropSilently } - }, - Err(e) => { - relay_log::error!("Invalid span: {e}"); - ItemAction::DropSilently } - }, + } _ => ItemAction::Keep, }); + let envelope = state.managed_envelope.envelope_mut(); + for item in items { + envelope.add_item(item); + } } /// Computes the sampling decision on the incoming event diff --git a/relay-server/src/endpoints/spans.rs b/relay-server/src/endpoints/spans.rs index d34c8d61a5..1593837163 100644 --- a/relay-server/src/endpoints/spans.rs +++ b/relay-server/src/endpoints/spans.rs @@ -5,29 +5,29 @@ use axum::routing::{post, MethodRouter}; use bytes::Bytes; use relay_config::Config; -use relay_event_schema::protocol::{EventId, Span as EventSpan}; -use relay_protocol::Annotated; -use relay_spans::Span; +use relay_event_schema::protocol::EventId; use crate::endpoints::common::{self, BadStoreRequest}; use crate::envelope::{ContentType, Envelope, Item, ItemType}; -use crate::extractors::RequestMeta; +use crate::extractors::{RawContentType, RequestMeta}; use crate::service::ServiceState; async fn handle( state: ServiceState, + content_type: RawContentType, meta: RequestMeta, - extract::Json(span): extract::Json, + body: Bytes, ) -> Result { - println!("{:#?}", span); + if !content_type.as_ref().starts_with("application/json") { + return Ok(StatusCode::UNSUPPORTED_MEDIA_TYPE); + } - // Convert to an Event span. - let event_span: Annotated = Annotated::new(span.into()); + if body.is_empty() { + return Err(BadStoreRequest::EmptyBody); + } - // Pack into an envelope for further processing. - let span_json = event_span.to_json().map_err(BadStoreRequest::InvalidJson)?; - let mut item = Item::new(ItemType::Span); - item.set_payload(ContentType::Json, span_json); + let mut item = Item::new(ItemType::OtelSpan); + item.set_payload(ContentType::Json, body); let mut envelope = Envelope::from_request(Some(EventId::new()), meta); envelope.add_item(item); common::handle_envelope(&state, envelope).await?; diff --git a/relay-server/src/envelope.rs b/relay-server/src/envelope.rs index acad7f1cb6..9d2af3e7a0 100644 --- a/relay-server/src/envelope.rs +++ b/relay-server/src/envelope.rs @@ -111,6 +111,8 @@ pub enum ItemType { CheckIn, /// A standalone span. Span, + /// A standalone OpenTelemetry span. + OtelSpan, /// A new item type that is yet unknown by this version of Relay. /// /// By default, items of this type are forwarded without modification. Processing Relays and @@ -154,6 +156,7 @@ impl fmt::Display for ItemType { Self::ReplayRecording => write!(f, "replay_recording"), Self::CheckIn => write!(f, "check_in"), Self::Span => write!(f, "span"), + Self::OtelSpan => write!(f, "otel_span"), Self::Unknown(s) => s.fmt(f), } } @@ -555,7 +558,8 @@ impl Item { ItemType::ClientReport => None, ItemType::CheckIn => Some(DataCategory::Monitor), ItemType::Unknown(_) => None, - ItemType::Span => None, // No outcomes, for now + ItemType::Span => None, // No outcomes, for now + ItemType::OtelSpan => None, // No outcomes, for now } } @@ -715,7 +719,8 @@ impl Item { | ItemType::ReplayRecording | ItemType::Profile | ItemType::CheckIn - | ItemType::Span => false, + | ItemType::Span + | ItemType::OtelSpan => false, // The unknown item type can observe any behavior, most likely there are going to be no // item types added that create events. @@ -746,6 +751,7 @@ impl Item { ItemType::Profile => true, ItemType::CheckIn => false, ItemType::Span => false, + ItemType::OtelSpan => false, // Since this Relay cannot interpret the semantics of this item, it does not know // whether it requires an event or not. Depending on the strategy, this can cause two diff --git a/relay-server/src/utils/rate_limits.rs b/relay-server/src/utils/rate_limits.rs index 2f6abf6de2..7a7b1926d9 100644 --- a/relay-server/src/utils/rate_limits.rs +++ b/relay-server/src/utils/rate_limits.rs @@ -108,6 +108,7 @@ fn infer_event_category(item: &Item) -> Option { ItemType::ClientReport => None, ItemType::CheckIn => None, ItemType::Span => None, + ItemType::OtelSpan => None, ItemType::Unknown(_) => None, } } diff --git a/relay-server/src/utils/sizes.rs b/relay-server/src/utils/sizes.rs index c73fc41f70..c89772f3dc 100644 --- a/relay-server/src/utils/sizes.rs +++ b/relay-server/src/utils/sizes.rs @@ -68,6 +68,11 @@ pub fn check_envelope_size_limits(config: &Config, envelope: &Envelope) -> bool return false; } } + ItemType::OtelSpan => { + if item.len() > config.max_span_size() { + return false; + } + } ItemType::Unknown(_) => (), } } From 05fda2077ab77a03f86bc1a8df6de46788cfea76 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Thu, 19 Oct 2023 13:03:42 -0400 Subject: [PATCH 06/59] Filter spans if the feature is not active --- relay-server/src/actors/processor.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index ea809e691d..82e0c0b98f 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2428,6 +2428,18 @@ impl EnvelopeProcessorService { Ok(span) } + fn filter_spans(&self, state: &mut ProcessEnvelopeState) { + let otel_span_ingestion_enabled = + state.project_state.has_feature(Feature::OTelSpanIngestion); + state.managed_envelope.retain_items(|item| match item.ty() { + ItemType::OtelSpan if !otel_span_ingestion_enabled => { + relay_log::warn!("dropping span because feature is disabled"); + ItemAction::DropSilently + } + _ => ItemAction::Keep, + }); + } + #[cfg(feature = "processing")] fn process_spans(&self, state: &mut ProcessEnvelopeState) { let mut items: Vec = Vec::new(); @@ -2710,6 +2722,7 @@ impl EnvelopeProcessorService { self.process_user_reports(state); self.process_replays(state)?; self.filter_profiles(state); + self.filter_spans(state); if state.creates_event() { // Some envelopes only create events in processing relays; for example, unreal events. From 8302275a0b17ca7328741d04a970560018849af4 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Thu, 19 Oct 2023 13:04:11 -0400 Subject: [PATCH 07/59] Parse attributes' values with the right type --- relay-spans/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index 24c807a441..aa79670b16 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -115,6 +115,7 @@ pub struct AnyValue { } #[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] pub enum Value { StringValue(String), BoolValue(bool), @@ -138,7 +139,7 @@ pub struct KeyValueList { #[derive(Debug, Deserialize)] pub struct KeyValue { pub key: String, - pub value: Option, + pub value: Option, } #[derive(Debug, Deserialize)] From 319ab239cae8da88b4803eabd02af0d0db5a3881 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Thu, 19 Oct 2023 20:48:29 -0400 Subject: [PATCH 08/59] Use default value instead of Option --- relay-spans/src/lib.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index aa79670b16..da8ddd590e 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -10,7 +10,9 @@ use relay_event_schema::protocol::{Span as EventSpan, SpanId, Timestamp, TraceId pub struct Span { pub trace_id: String, pub span_id: String, - pub trace_state: Option, + #[serde(default, skip_serializing_if = "String::is_empty")] + pub trace_state: String, + #[serde(default, skip_serializing_if = "String::is_empty")] pub parent_span_id: Option, pub name: String, pub kind: SpanKind, @@ -22,7 +24,8 @@ pub struct Span { pub dropped_events_count: u32, pub links: Vec, pub dropped_links_count: u32, - pub status: Option, + #[serde(default)] + pub status: Status, } #[derive(Debug, Deserialize)] @@ -77,15 +80,17 @@ impl SpanKind { } } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Default)] pub struct Status { - pub message: Option, + #[serde(default, skip_serializing_if = "String::is_empty")] + pub message: String, pub code: StatusCode, } -#[derive(Debug, Deserialize_repr)] +#[derive(Debug, Deserialize_repr, Default)] #[repr(u32)] pub enum StatusCode { + #[default] Unset = 0, Ok = 1, Error = 2, @@ -109,14 +114,9 @@ impl StatusCode { } } -#[derive(Debug, Deserialize)] -pub struct AnyValue { - pub value: Option, -} - #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] -pub enum Value { +pub enum AnyValue { StringValue(String), BoolValue(bool), IntValue(i64), @@ -139,7 +139,7 @@ pub struct KeyValueList { #[derive(Debug, Deserialize)] pub struct KeyValue { pub key: String, - pub value: Option, + pub value: AnyValue, } #[derive(Debug, Deserialize)] From 3b00b09a8803d45a971b74b4a47850bf513c746a Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Thu, 19 Oct 2023 22:34:44 -0400 Subject: [PATCH 09/59] Add data and status fields --- Cargo.lock | 1 + relay-spans/Cargo.toml | 5 +- relay-spans/src/lib.rs | 127 +++++++++++++++++++++++++++----- relay-spans/src/status_codes.rs | 40 ++++++++++ 4 files changed, 152 insertions(+), 21 deletions(-) create mode 100644 relay-spans/src/status_codes.rs diff --git a/Cargo.lock b/Cargo.lock index de3a300a61..ee588884dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3918,6 +3918,7 @@ version = "23.9.1" dependencies = [ "chrono", "enumset", + "once_cell", "relay-event-schema", "relay-protocol", "serde", diff --git a/relay-spans/Cargo.toml b/relay-spans/Cargo.toml index 1809a07504..bcaad45872 100644 --- a/relay-spans/Cargo.toml +++ b/relay-spans/Cargo.toml @@ -12,8 +12,9 @@ publish = false [dependencies] chrono.workspace = true enumset = "1.0.4" +once_cell.workspace = true relay-event-schema = { path = "../relay-event-schema" } relay-protocol = { path = "../relay-protocol" } -serde = { workspace = true } -serde_json = { workspace = true } +serde.workspace = true +serde_json.workspace = true serde_repr = "0.1.16" diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index da8ddd590e..bebfee80ab 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -1,11 +1,15 @@ -use chrono::{TimeZone, Utc}; +use std::str::FromStr; +use chrono::{TimeZone, Utc}; use serde::Deserialize; use serde_repr::Deserialize_repr; -use relay_event_schema::protocol::{Span as EventSpan, SpanId, Timestamp, TraceId}; +use relay_event_schema::protocol::{Span as EventSpan, SpanId, SpanStatus, Timestamp, TraceId}; +use relay_protocol::{Annotated, Object, Value}; -#[derive(Debug, Deserialize)] +mod status_codes; + +#[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Span { pub trace_id: String, @@ -28,7 +32,7 @@ pub struct Span { pub status: Status, } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] pub struct Event { pub time_unix_nano: u64, pub name: String, @@ -36,7 +40,7 @@ pub struct Event { pub dropped_attributes_count: u32, } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] pub struct Link { pub trace_id: String, pub span_id: String, @@ -45,7 +49,7 @@ pub struct Link { pub dropped_attributes_count: u32, } -#[derive(Debug, Deserialize_repr)] +#[derive(Clone, Debug, Deserialize_repr)] #[repr(u32)] pub enum SpanKind { Unspecified = 0, @@ -80,14 +84,14 @@ impl SpanKind { } } -#[derive(Debug, Deserialize, Default)] +#[derive(Clone, Debug, Deserialize, Default)] pub struct Status { #[serde(default, skip_serializing_if = "String::is_empty")] pub message: String, pub code: StatusCode, } -#[derive(Debug, Deserialize_repr, Default)] +#[derive(Clone, Debug, Deserialize_repr, Default, PartialEq)] #[repr(u32)] pub enum StatusCode { #[default] @@ -114,29 +118,52 @@ impl StatusCode { } } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub enum AnyValue { - StringValue(String), + ArrayValue(ArrayValue), BoolValue(bool), - IntValue(i64), + BytesValue(Vec), DoubleValue(f64), - ArrayValue(ArrayValue), + IntValue(i64), KvlistValue(KeyValueList), - BytesValue(Vec), + StringValue(String), } -#[derive(Debug, Deserialize)] +impl AnyValue { + pub fn to_i64(self) -> Option { + match self { + AnyValue::IntValue(v) => Some(v), + _ => None, + } + } + + pub fn to_string(self) -> Option { + match self { + AnyValue::StringValue(v) => Some(v), + AnyValue::BoolValue(v) => Some(v.to_string()), + AnyValue::IntValue(v) => Some(v.to_string()), + AnyValue::DoubleValue(v) => Some(v.to_string()), + AnyValue::BytesValue(v) => match String::from_utf8(v) { + Ok(v) => Some(v), + Err(_) => None, + }, + _ => None, + } + } +} + +#[derive(Clone, Debug, Deserialize)] pub struct ArrayValue { pub values: Vec, } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] pub struct KeyValueList { pub values: Vec, } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] pub struct KeyValue { pub key: String, pub value: AnyValue, @@ -155,15 +182,39 @@ impl From for EventSpan { let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); let exclusive_time = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; + let mut attributes: Object = Object::new(); + for attribute in from.attributes.clone() { + match attribute.value { + AnyValue::ArrayValue(_) => todo!(), + AnyValue::BoolValue(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + AnyValue::BytesValue(_) => todo!(), + AnyValue::DoubleValue(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + AnyValue::IntValue(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + AnyValue::KvlistValue(_) => todo!(), + AnyValue::StringValue(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + }; + } let mut span = EventSpan { - description: from.name.into(), + data: attributes.into(), + description: from.name.clone().into(), exclusive_time: exclusive_time.into(), - span_id: SpanId(from.span_id).into(), + span_id: SpanId(from.span_id.clone()).into(), start_timestamp: Timestamp(start_timestamp).into(), timestamp: Timestamp(end_timestamp).into(), - trace_id: TraceId(from.trace_id).into(), + trace_id: TraceId(from.trace_id.clone()).into(), ..Default::default() }; + if let Ok(status) = SpanStatus::from_str(from.sentry_status()) { + span.status = status.into(); + } if let Some(parent_span_id) = from.parent_span_id { span.is_segment = false.into(); span.parent_span_id = SpanId(parent_span_id).into(); @@ -175,6 +226,44 @@ impl From for EventSpan { } } +impl Span { + pub fn sentry_status(&self) -> &'static str { + let status_code = self.status.code.clone(); + + if status_code == StatusCode::Unset || status_code == StatusCode::Ok { + return "ok"; + } + + if let Some(code) = self + .attributes + .clone() + .into_iter() + .find(|a| a.key == "http.status_code") + { + if let Some(code_value) = code.value.to_i64() { + if let Some(sentry_status) = status_codes::HTTP.get(&code_value) { + return sentry_status; + } + } + } + + if let Some(code) = self + .attributes + .clone() + .into_iter() + .find(|a| a.key == "rpc.grpc.status_code") + { + if let Some(code_value) = code.value.to_i64() { + if let Some(sentry_status) = status_codes::GRPC.get(&code_value) { + return sentry_status; + } + } + } + + "unknown_error" + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/relay-spans/src/status_codes.rs b/relay-spans/src/status_codes.rs new file mode 100644 index 0000000000..0678a447e6 --- /dev/null +++ b/relay-spans/src/status_codes.rs @@ -0,0 +1,40 @@ +use std::collections::BTreeMap; + +use once_cell::sync::Lazy; + +pub static HTTP: Lazy> = Lazy::new(|| { + BTreeMap::from([ + (400, "failed_precondition"), + (401, "unauthenticated"), + (403, "permission_denied"), + (404, "not_found"), + (409, "aborted"), + (429, "resource_exhausted"), + (499, "cancelled"), + (500, "internal_error"), + (501, "unimplemented"), + (503, "unavailable"), + (504, "deadline_exceeded"), + ]) +}); + +pub static GRPC: Lazy> = Lazy::new(|| { + BTreeMap::from([ + (1, "cancelled"), + (2, "unknown_error"), + (3, "invalid_argument"), + (4, "deadline_exceeded"), + (5, "not_found"), + (6, "already_exists"), + (7, "permission_denied"), + (8, "resource_exhausted"), + (9, "failed_precondition"), + (10, "aborted"), + (11, "out_of_range"), + (12, "unimplemented"), + (13, "internal_error"), + (14, "unavailable"), + (15, "data_loss"), + (16, "unauthenticated"), + ]) +}); From 7e44fb03b58723cac120e1de1c7c6cecbea4532b Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Thu, 19 Oct 2023 22:51:59 -0400 Subject: [PATCH 10/59] Reorganize the crate --- relay-spans/src/lib.rs | 337 +------------------------------------- relay-spans/src/span.rs | 350 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 352 insertions(+), 335 deletions(-) create mode 100644 relay-spans/src/span.rs diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index bebfee80ab..14216d94b1 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -1,337 +1,4 @@ -use std::str::FromStr; - -use chrono::{TimeZone, Utc}; -use serde::Deserialize; -use serde_repr::Deserialize_repr; - -use relay_event_schema::protocol::{Span as EventSpan, SpanId, SpanStatus, Timestamp, TraceId}; -use relay_protocol::{Annotated, Object, Value}; +pub use crate::span::Span; +mod span; mod status_codes; - -#[derive(Clone, Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct Span { - pub trace_id: String, - pub span_id: String, - #[serde(default, skip_serializing_if = "String::is_empty")] - pub trace_state: String, - #[serde(default, skip_serializing_if = "String::is_empty")] - pub parent_span_id: Option, - pub name: String, - pub kind: SpanKind, - pub start_time_unix_nano: i64, - pub end_time_unix_nano: i64, - pub attributes: Vec, - pub dropped_attributes_count: u32, - pub events: Vec, - pub dropped_events_count: u32, - pub links: Vec, - pub dropped_links_count: u32, - #[serde(default)] - pub status: Status, -} - -#[derive(Clone, Debug, Deserialize)] -pub struct Event { - pub time_unix_nano: u64, - pub name: String, - pub attributes: Vec, - pub dropped_attributes_count: u32, -} - -#[derive(Clone, Debug, Deserialize)] -pub struct Link { - pub trace_id: String, - pub span_id: String, - pub trace_state: String, - pub attributes: Vec, - pub dropped_attributes_count: u32, -} - -#[derive(Clone, Debug, Deserialize_repr)] -#[repr(u32)] -pub enum SpanKind { - Unspecified = 0, - Internal = 1, - Server = 2, - Client = 3, - Producer = 4, - Consumer = 5, -} - -impl SpanKind { - pub fn as_str_name(&self) -> &'static str { - match self { - SpanKind::Unspecified => "SPAN_KIND_UNSPECIFIED", - SpanKind::Internal => "SPAN_KIND_INTERNAL", - SpanKind::Server => "SPAN_KIND_SERVER", - SpanKind::Client => "SPAN_KIND_CLIENT", - SpanKind::Producer => "SPAN_KIND_PRODUCER", - SpanKind::Consumer => "SPAN_KIND_CONSUMER", - } - } - pub fn from_str_name(value: &str) -> ::core::option::Option { - match value { - "SPAN_KIND_UNSPECIFIED" => Some(Self::Unspecified), - "SPAN_KIND_INTERNAL" => Some(Self::Internal), - "SPAN_KIND_SERVER" => Some(Self::Server), - "SPAN_KIND_CLIENT" => Some(Self::Client), - "SPAN_KIND_PRODUCER" => Some(Self::Producer), - "SPAN_KIND_CONSUMER" => Some(Self::Consumer), - _ => None, - } - } -} - -#[derive(Clone, Debug, Deserialize, Default)] -pub struct Status { - #[serde(default, skip_serializing_if = "String::is_empty")] - pub message: String, - pub code: StatusCode, -} - -#[derive(Clone, Debug, Deserialize_repr, Default, PartialEq)] -#[repr(u32)] -pub enum StatusCode { - #[default] - Unset = 0, - Ok = 1, - Error = 2, -} - -impl StatusCode { - pub fn as_str_name(&self) -> &'static str { - match self { - StatusCode::Unset => "STATUS_CODE_UNSET", - StatusCode::Ok => "STATUS_CODE_OK", - StatusCode::Error => "STATUS_CODE_ERROR", - } - } - pub fn from_str_name(value: &str) -> ::core::option::Option { - match value { - "STATUS_CODE_UNSET" => Some(Self::Unset), - "STATUS_CODE_OK" => Some(Self::Ok), - "STATUS_CODE_ERROR" => Some(Self::Error), - _ => None, - } - } -} - -#[derive(Clone, Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -pub enum AnyValue { - ArrayValue(ArrayValue), - BoolValue(bool), - BytesValue(Vec), - DoubleValue(f64), - IntValue(i64), - KvlistValue(KeyValueList), - StringValue(String), -} - -impl AnyValue { - pub fn to_i64(self) -> Option { - match self { - AnyValue::IntValue(v) => Some(v), - _ => None, - } - } - - pub fn to_string(self) -> Option { - match self { - AnyValue::StringValue(v) => Some(v), - AnyValue::BoolValue(v) => Some(v.to_string()), - AnyValue::IntValue(v) => Some(v.to_string()), - AnyValue::DoubleValue(v) => Some(v.to_string()), - AnyValue::BytesValue(v) => match String::from_utf8(v) { - Ok(v) => Some(v), - Err(_) => None, - }, - _ => None, - } - } -} - -#[derive(Clone, Debug, Deserialize)] -pub struct ArrayValue { - pub values: Vec, -} - -#[derive(Clone, Debug, Deserialize)] -pub struct KeyValueList { - pub values: Vec, -} - -#[derive(Clone, Debug, Deserialize)] -pub struct KeyValue { - pub key: String, - pub value: AnyValue, -} - -#[derive(Debug, Deserialize)] -pub struct InstrumentationScope { - pub name: String, - pub version: String, - pub attributes: Vec, - pub dropped_attributes_count: u32, -} - -impl From for EventSpan { - fn from(from: Span) -> Self { - let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); - let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); - let exclusive_time = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; - let mut attributes: Object = Object::new(); - for attribute in from.attributes.clone() { - match attribute.value { - AnyValue::ArrayValue(_) => todo!(), - AnyValue::BoolValue(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); - } - AnyValue::BytesValue(_) => todo!(), - AnyValue::DoubleValue(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); - } - AnyValue::IntValue(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); - } - AnyValue::KvlistValue(_) => todo!(), - AnyValue::StringValue(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); - } - }; - } - let mut span = EventSpan { - data: attributes.into(), - description: from.name.clone().into(), - exclusive_time: exclusive_time.into(), - span_id: SpanId(from.span_id.clone()).into(), - start_timestamp: Timestamp(start_timestamp).into(), - timestamp: Timestamp(end_timestamp).into(), - trace_id: TraceId(from.trace_id.clone()).into(), - ..Default::default() - }; - if let Ok(status) = SpanStatus::from_str(from.sentry_status()) { - span.status = status.into(); - } - if let Some(parent_span_id) = from.parent_span_id { - span.is_segment = false.into(); - span.parent_span_id = SpanId(parent_span_id).into(); - } else { - span.is_segment = true.into(); - span.segment_id = span.span_id.clone(); - } - span - } -} - -impl Span { - pub fn sentry_status(&self) -> &'static str { - let status_code = self.status.code.clone(); - - if status_code == StatusCode::Unset || status_code == StatusCode::Ok { - return "ok"; - } - - if let Some(code) = self - .attributes - .clone() - .into_iter() - .find(|a| a.key == "http.status_code") - { - if let Some(code_value) = code.value.to_i64() { - if let Some(sentry_status) = status_codes::HTTP.get(&code_value) { - return sentry_status; - } - } - } - - if let Some(code) = self - .attributes - .clone() - .into_iter() - .find(|a| a.key == "rpc.grpc.status_code") - { - if let Some(code_value) = code.value.to_i64() { - if let Some(sentry_status) = status_codes::GRPC.get(&code_value) { - return sentry_status; - } - } - } - - "unknown_error" - } -} - -#[cfg(test)] -mod tests { - use super::*; - use relay_protocol::Annotated; - - #[test] - fn parse_span() { - let json = r#"{ - "traceId": "89143b0763095bd9c9955e8175d1fb23", - "spanId": "e342abb1214ca181", - "parentSpanId": "0c7a7dea069bf5a6", - "name": "middleware - fastify -> @fastify/multipart", - "kind": 1, - "startTimeUnixNano": 1697620454980000000, - "endTimeUnixNano": 1697620454980078800, - "attributes": [ - { - "key": "fastify.type", - "value": { - "stringValue": "middleware" - } - }, - { - "key": "plugin.name", - "value": { - "stringValue": "fastify -> @fastify/multipart" - } - }, - { - "key": "hook.name", - "value": { - "stringValue": "onResponse" - } - }, - { - "key": "sentry.sample_rate", - "value": { - "intValue": 1 - } - }, - { - "key": "sentry.parentSampled", - "value": { - "boolValue": true - } - } - ], - "droppedAttributesCount": 0, - "events": [], - "droppedEventsCount": 0, - "status": { - "code": 0 - }, - "links": [], - "droppedLinksCount": 0 - }"#; - let otel_span: Span = serde_json::from_str(json).unwrap(); - let event_span: Annotated = Annotated::new(otel_span.into()); - assert_eq!( - event_span - .value() - .unwrap() - .sentry_tags - .value() - .unwrap() - .get("environment") - .unwrap(), - &Annotated::new("test".to_string()) - ); - } -} diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs new file mode 100644 index 0000000000..bb832b8e3f --- /dev/null +++ b/relay-spans/src/span.rs @@ -0,0 +1,350 @@ +use std::str::FromStr; + +use chrono::{TimeZone, Utc}; +use serde::Deserialize; +use serde_repr::Deserialize_repr; + +use relay_event_schema::protocol::{Span as EventSpan, SpanId, SpanStatus, Timestamp, TraceId}; +use relay_protocol::{Annotated, Object, Value}; + +use crate::status_codes; + +#[derive(Clone, Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct Span { + pub trace_id: String, + pub span_id: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + pub trace_state: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + pub parent_span_id: Option, + pub name: String, + pub kind: SpanKind, + pub start_time_unix_nano: i64, + pub end_time_unix_nano: i64, + pub attributes: Vec, + pub dropped_attributes_count: u32, + pub events: Vec, + pub dropped_events_count: u32, + pub links: Vec, + pub dropped_links_count: u32, + #[serde(default)] + pub status: Status, +} + +#[derive(Clone, Debug, Deserialize)] +pub struct Event { + pub time_unix_nano: u64, + pub name: String, + pub attributes: Vec, + pub dropped_attributes_count: u32, +} + +#[derive(Clone, Debug, Deserialize)] +pub struct Link { + pub trace_id: String, + pub span_id: String, + pub trace_state: String, + pub attributes: Vec, + pub dropped_attributes_count: u32, +} + +#[derive(Clone, Debug, Deserialize_repr)] +#[repr(u32)] +pub enum SpanKind { + Unspecified = 0, + Internal = 1, + Server = 2, + Client = 3, + Producer = 4, + Consumer = 5, +} + +impl SpanKind { + pub fn as_str_name(&self) -> &'static str { + match self { + SpanKind::Unspecified => "SPAN_KIND_UNSPECIFIED", + SpanKind::Internal => "SPAN_KIND_INTERNAL", + SpanKind::Server => "SPAN_KIND_SERVER", + SpanKind::Client => "SPAN_KIND_CLIENT", + SpanKind::Producer => "SPAN_KIND_PRODUCER", + SpanKind::Consumer => "SPAN_KIND_CONSUMER", + } + } + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "SPAN_KIND_UNSPECIFIED" => Some(Self::Unspecified), + "SPAN_KIND_INTERNAL" => Some(Self::Internal), + "SPAN_KIND_SERVER" => Some(Self::Server), + "SPAN_KIND_CLIENT" => Some(Self::Client), + "SPAN_KIND_PRODUCER" => Some(Self::Producer), + "SPAN_KIND_CONSUMER" => Some(Self::Consumer), + _ => None, + } + } +} + +#[derive(Clone, Debug, Deserialize, Default)] +pub struct Status { + #[serde(default, skip_serializing_if = "String::is_empty")] + pub message: String, + pub code: StatusCode, +} + +#[derive(Clone, Debug, Deserialize_repr, Default, PartialEq)] +#[repr(u32)] +pub enum StatusCode { + #[default] + Unset = 0, + Ok = 1, + Error = 2, +} + +impl StatusCode { + pub fn as_str_name(&self) -> &'static str { + match self { + StatusCode::Unset => "STATUS_CODE_UNSET", + StatusCode::Ok => "STATUS_CODE_OK", + StatusCode::Error => "STATUS_CODE_ERROR", + } + } + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "STATUS_CODE_UNSET" => Some(Self::Unset), + "STATUS_CODE_OK" => Some(Self::Ok), + "STATUS_CODE_ERROR" => Some(Self::Error), + _ => None, + } + } +} + +#[derive(Clone, Debug, Deserialize)] +pub enum AnyValue { + #[serde(rename = "arrayValue")] + Array(ArrayValue), + #[serde(rename = "boolValue")] + Bool(bool), + #[serde(rename = "bytesValue")] + Bytes(Vec), + #[serde(rename = "doubleValue")] + Double(f64), + #[serde(rename = "intValue")] + Int(i64), + #[serde(rename = "kvlistValue")] + Kvlist(KeyValueList), + #[serde(rename = "stringValue")] + String(String), +} + +impl AnyValue { + pub fn to_i64(&self) -> Option { + match self { + AnyValue::Int(v) => Some(*v), + _ => None, + } + } + + pub fn to_string(&self) -> Option { + match self { + AnyValue::String(v) => Some(v.clone()), + AnyValue::Bool(v) => Some(v.to_string()), + AnyValue::Int(v) => Some(v.to_string()), + AnyValue::Double(v) => Some(v.to_string()), + AnyValue::Bytes(v) => match String::from_utf8(v.clone()) { + Ok(v) => Some(v), + Err(_) => None, + }, + _ => None, + } + } +} + +#[derive(Clone, Debug, Deserialize)] +pub struct ArrayValue { + pub values: Vec, +} + +#[derive(Clone, Debug, Deserialize)] +pub struct KeyValueList { + pub values: Vec, +} + +#[derive(Clone, Debug, Deserialize)] +pub struct KeyValue { + pub key: String, + pub value: AnyValue, +} + +#[derive(Debug, Deserialize)] +pub struct InstrumentationScope { + pub name: String, + pub version: String, + pub attributes: Vec, + pub dropped_attributes_count: u32, +} + +impl From for EventSpan { + fn from(from: Span) -> Self { + let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); + let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); + let exclusive_time = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; + let mut attributes: Object = Object::new(); + for attribute in from.attributes.clone() { + match attribute.value { + AnyValue::Array(_) => todo!(), + AnyValue::Bool(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + AnyValue::Bytes(_) => todo!(), + AnyValue::Double(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + AnyValue::Int(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + AnyValue::Kvlist(_) => todo!(), + AnyValue::String(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + }; + } + let mut span = EventSpan { + data: attributes.into(), + description: from.name.clone().into(), + exclusive_time: exclusive_time.into(), + span_id: SpanId(from.span_id.clone()).into(), + start_timestamp: Timestamp(start_timestamp).into(), + timestamp: Timestamp(end_timestamp).into(), + trace_id: TraceId(from.trace_id.clone()).into(), + ..Default::default() + }; + if let Ok(status) = SpanStatus::from_str(from.sentry_status()) { + span.status = status.into(); + } + if let Some(parent_span_id) = from.parent_span_id { + span.is_segment = false.into(); + span.parent_span_id = SpanId(parent_span_id).into(); + } else { + span.is_segment = true.into(); + span.segment_id = span.span_id.clone(); + } + span + } +} + +impl Span { + pub fn sentry_status(&self) -> &'static str { + let status_code = self.status.code.clone(); + + if status_code == StatusCode::Unset || status_code == StatusCode::Ok { + return "ok"; + } + + if let Some(code) = self + .attributes + .clone() + .into_iter() + .find(|a| a.key == "http.status_code") + { + if let Some(code_value) = code.value.to_i64() { + if let Some(sentry_status) = status_codes::HTTP.get(&code_value) { + return sentry_status; + } + } + } + + if let Some(code) = self + .attributes + .clone() + .into_iter() + .find(|a| a.key == "rpc.grpc.status_code") + { + if let Some(code_value) = code.value.to_i64() { + if let Some(sentry_status) = status_codes::GRPC.get(&code_value) { + return sentry_status; + } + } + } + + "unknown_error" + } +} + +#[cfg(test)] +mod tests { + use super::*; + use relay_protocol::Annotated; + + #[test] + fn parse_span() { + let json = r#"{ + "traceId": "89143b0763095bd9c9955e8175d1fb23", + "spanId": "e342abb1214ca181", + "parentSpanId": "0c7a7dea069bf5a6", + "name": "middleware - fastify -> @fastify/multipart", + "kind": 1, + "startTimeUnixNano": 1697620454980000000, + "endTimeUnixNano": 1697620454980078800, + "attributes": [ + { + "key": "sentry.environment", + "value": { + "stringValue": "test" + } + }, + { + "key": "fastify.type", + "value": { + "stringValue": "middleware" + } + }, + { + "key": "plugin.name", + "value": { + "stringValue": "fastify -> @fastify/multipart" + } + }, + { + "key": "hook.name", + "value": { + "stringValue": "onResponse" + } + }, + { + "key": "sentry.sample_rate", + "value": { + "intValue": 1 + } + }, + { + "key": "sentry.parentSampled", + "value": { + "boolValue": true + } + } + ], + "droppedAttributesCount": 0, + "events": [], + "droppedEventsCount": 0, + "status": { + "code": 0 + }, + "links": [], + "droppedLinksCount": 0 + }"#; + let otel_span: Span = serde_json::from_str(json).unwrap(); + let event_span: Annotated = Annotated::new(otel_span.into()); + assert_eq!( + event_span + .value() + .unwrap() + .data + .value() + .unwrap() + .get("sentry.environment") + .unwrap() + .as_str(), + Some("test") + ); + } +} From 7ed09c6a05b5ab9ec0d871c8a9f878001a9ee9a1 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 08:27:17 -0400 Subject: [PATCH 11/59] Add docs auto-generation Co-authored-by: Joris Bayer --- relay-spans/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index 14216d94b1..f034b09262 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -1,3 +1,9 @@ +#![warn(missing_docs)] +#![doc( + html_logo_url = "https://raw.githubusercontent.com/getsentry/relay/master/artwork/relay-icon.png", + html_favicon_url = "https://raw.githubusercontent.com/getsentry/relay/master/artwork/relay-icon.png" +)] + pub use crate::span::Span; mod span; From 1f6f25cfbb86b263bf94145e3f39d161523f7233 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 08:29:43 -0400 Subject: [PATCH 12/59] Add a link to the schema we're implementing --- relay-spans/src/span.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index bb832b8e3f..007888a5ae 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -9,6 +9,8 @@ use relay_protocol::{Annotated, Object, Value}; use crate::status_codes; +/// This is a serde implementation of https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/trace/v1/trace.proto. + #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Span { From 77bc3c584c3540f4c31f3f5c9bb5ac2d7b48b7e3 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 08:30:27 -0400 Subject: [PATCH 13/59] Omit EventId as it's set later on if needed --- relay-server/src/endpoints/spans.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/relay-server/src/endpoints/spans.rs b/relay-server/src/endpoints/spans.rs index 1593837163..5d69435651 100644 --- a/relay-server/src/endpoints/spans.rs +++ b/relay-server/src/endpoints/spans.rs @@ -5,7 +5,6 @@ use axum::routing::{post, MethodRouter}; use bytes::Bytes; use relay_config::Config; -use relay_event_schema::protocol::EventId; use crate::endpoints::common::{self, BadStoreRequest}; use crate::envelope::{ContentType, Envelope, Item, ItemType}; @@ -28,7 +27,7 @@ async fn handle( let mut item = Item::new(ItemType::OtelSpan); item.set_payload(ContentType::Json, body); - let mut envelope = Envelope::from_request(Some(EventId::new()), meta); + let mut envelope = Envelope::from_request(None, meta); envelope.add_item(item); common::handle_envelope(&state, envelope).await?; From 517c80782d7d3d193ea3e80810f3f98a8771daea Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 08:32:21 -0400 Subject: [PATCH 14/59] Move span related code right after the span struct --- relay-spans/src/span.rs | 174 ++++++++++++++++++++-------------------- 1 file changed, 87 insertions(+), 87 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 007888a5ae..01d0b16621 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -34,6 +34,93 @@ pub struct Span { pub status: Status, } +impl Span { + pub fn sentry_status(&self) -> &'static str { + let status_code = self.status.code.clone(); + + if status_code == StatusCode::Unset || status_code == StatusCode::Ok { + return "ok"; + } + + if let Some(code) = self + .attributes + .clone() + .into_iter() + .find(|a| a.key == "http.status_code") + { + if let Some(code_value) = code.value.to_i64() { + if let Some(sentry_status) = status_codes::HTTP.get(&code_value) { + return sentry_status; + } + } + } + + if let Some(code) = self + .attributes + .clone() + .into_iter() + .find(|a| a.key == "rpc.grpc.status_code") + { + if let Some(code_value) = code.value.to_i64() { + if let Some(sentry_status) = status_codes::GRPC.get(&code_value) { + return sentry_status; + } + } + } + + "unknown_error" + } +} + +impl From for EventSpan { + fn from(from: Span) -> Self { + let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); + let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); + let exclusive_time = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; + let mut attributes: Object = Object::new(); + for attribute in from.attributes.clone() { + match attribute.value { + AnyValue::Array(_) => todo!(), + AnyValue::Bool(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + AnyValue::Bytes(_) => todo!(), + AnyValue::Double(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + AnyValue::Int(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + AnyValue::Kvlist(_) => todo!(), + AnyValue::String(v) => { + attributes.insert(attribute.key, Annotated::new(v.into())); + } + }; + } + let mut span = EventSpan { + data: attributes.into(), + description: from.name.clone().into(), + exclusive_time: exclusive_time.into(), + span_id: SpanId(from.span_id.clone()).into(), + start_timestamp: Timestamp(start_timestamp).into(), + timestamp: Timestamp(end_timestamp).into(), + trace_id: TraceId(from.trace_id.clone()).into(), + ..Default::default() + }; + if let Ok(status) = SpanStatus::from_str(from.sentry_status()) { + span.status = status.into(); + } + if let Some(parent_span_id) = from.parent_span_id { + span.is_segment = false.into(); + span.parent_span_id = SpanId(parent_span_id).into(); + } else { + span.is_segment = true.into(); + span.segment_id = span.span_id.clone(); + } + span + } +} + #[derive(Clone, Debug, Deserialize)] pub struct Event { pub time_unix_nano: u64, @@ -185,93 +272,6 @@ pub struct InstrumentationScope { pub dropped_attributes_count: u32, } -impl From for EventSpan { - fn from(from: Span) -> Self { - let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); - let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); - let exclusive_time = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; - let mut attributes: Object = Object::new(); - for attribute in from.attributes.clone() { - match attribute.value { - AnyValue::Array(_) => todo!(), - AnyValue::Bool(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); - } - AnyValue::Bytes(_) => todo!(), - AnyValue::Double(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); - } - AnyValue::Int(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); - } - AnyValue::Kvlist(_) => todo!(), - AnyValue::String(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); - } - }; - } - let mut span = EventSpan { - data: attributes.into(), - description: from.name.clone().into(), - exclusive_time: exclusive_time.into(), - span_id: SpanId(from.span_id.clone()).into(), - start_timestamp: Timestamp(start_timestamp).into(), - timestamp: Timestamp(end_timestamp).into(), - trace_id: TraceId(from.trace_id.clone()).into(), - ..Default::default() - }; - if let Ok(status) = SpanStatus::from_str(from.sentry_status()) { - span.status = status.into(); - } - if let Some(parent_span_id) = from.parent_span_id { - span.is_segment = false.into(); - span.parent_span_id = SpanId(parent_span_id).into(); - } else { - span.is_segment = true.into(); - span.segment_id = span.span_id.clone(); - } - span - } -} - -impl Span { - pub fn sentry_status(&self) -> &'static str { - let status_code = self.status.code.clone(); - - if status_code == StatusCode::Unset || status_code == StatusCode::Ok { - return "ok"; - } - - if let Some(code) = self - .attributes - .clone() - .into_iter() - .find(|a| a.key == "http.status_code") - { - if let Some(code_value) = code.value.to_i64() { - if let Some(sentry_status) = status_codes::HTTP.get(&code_value) { - return sentry_status; - } - } - } - - if let Some(code) = self - .attributes - .clone() - .into_iter() - .find(|a| a.key == "rpc.grpc.status_code") - { - if let Some(code_value) = code.value.to_i64() { - if let Some(sentry_status) = status_codes::GRPC.get(&code_value) { - return sentry_status; - } - } - } - - "unknown_error" - } -} - #[cfg(test)] mod tests { use super::*; From 30b4f138d406988ea69c7efbcedf1cb68e46626b Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 08:40:14 -0400 Subject: [PATCH 15/59] Remove unwanted Option --- relay-spans/src/span.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 01d0b16621..145f7a755f 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -19,7 +19,7 @@ pub struct Span { #[serde(default, skip_serializing_if = "String::is_empty")] pub trace_state: String, #[serde(default, skip_serializing_if = "String::is_empty")] - pub parent_span_id: Option, + pub parent_span_id: String, pub name: String, pub kind: SpanKind, pub start_time_unix_nano: i64, From dc9f60329ffd5820237a17d19af871bcd61b98cd Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 08:42:27 -0400 Subject: [PATCH 16/59] Use better error messages --- relay-server/src/actors/processor.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 82e0c0b98f..f8c2f9c58f 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2448,7 +2448,7 @@ impl EnvelopeProcessorService { let span: relay_spans::Span = match serde_json::from_slice(&item.payload()) { Ok(span) => span, Err(e) => { - relay_log::error!("Invalid span: {e}"); + relay_log::error!(error = &e as &dyn Error, "failed to parse OTel span"); return ItemAction::DropSilently; } }; @@ -2458,7 +2458,10 @@ impl EnvelopeProcessorService { let payload: Bytes = match span.to_json() { Ok(payload) => payload.into(), Err(e) => { - relay_log::error!("Invalid span: {e}"); + relay_log::error!( + error = &e as &dyn Error, + "Failed to serialize OTel span" + ); return ItemAction::DropSilently; } }; From 6ddebe07ed145f6009cc51db032cfa6f49810134 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 08:43:11 -0400 Subject: [PATCH 17/59] Rename feature flag --- relay-dynamic-config/src/feature.rs | 2 +- relay-server/src/actors/processor.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/relay-dynamic-config/src/feature.rs b/relay-dynamic-config/src/feature.rs index 0a41946b2a..f1a48a51d0 100644 --- a/relay-dynamic-config/src/feature.rs +++ b/relay-dynamic-config/src/feature.rs @@ -33,7 +33,7 @@ pub enum Feature { SpanMetricsExtractionResource, /// Enable OTel span ingestion. #[serde(rename = "organizations:otel-span-ingestion")] - OTelSpanIngestion, + OtelSpanIngestion, /// Deprecated, still forwarded for older downstream Relays. #[serde(rename = "organizations:transaction-name-mark-scrubbed-as-sanitized")] diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index f8c2f9c58f..d48033d388 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2430,7 +2430,7 @@ impl EnvelopeProcessorService { fn filter_spans(&self, state: &mut ProcessEnvelopeState) { let otel_span_ingestion_enabled = - state.project_state.has_feature(Feature::OTelSpanIngestion); + state.project_state.has_feature(Feature::OtelSpanIngestion); state.managed_envelope.retain_items(|item| match item.ty() { ItemType::OtelSpan if !otel_span_ingestion_enabled => { relay_log::warn!("dropping span because feature is disabled"); From fe7630069f863efb4aff37d31fa6a40bc802aac4 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 08:50:13 -0400 Subject: [PATCH 18/59] Remove unneeded structs --- relay-spans/src/span.rs | 61 ++++------------------------------------- 1 file changed, 6 insertions(+), 55 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 145f7a755f..3a22f76f90 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -14,8 +14,11 @@ use crate::status_codes; #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Span { + /// Trace ID pub trace_id: String, + /// Span ID pub span_id: String, + /// Trace state #[serde(default, skip_serializing_if = "String::is_empty")] pub trace_state: String, #[serde(default, skip_serializing_if = "String::is_empty")] @@ -102,7 +105,9 @@ impl From for EventSpan { description: from.name.clone().into(), exclusive_time: exclusive_time.into(), span_id: SpanId(from.span_id.clone()).into(), + parent_span_id: SpanId(from.parent_span_id.clone()).into(), start_timestamp: Timestamp(start_timestamp).into(), + is_segment: from.parent_span_id.is_empty().into(), timestamp: Timestamp(end_timestamp).into(), trace_id: TraceId(from.trace_id.clone()).into(), ..Default::default() @@ -110,11 +115,7 @@ impl From for EventSpan { if let Ok(status) = SpanStatus::from_str(from.sentry_status()) { span.status = status.into(); } - if let Some(parent_span_id) = from.parent_span_id { - span.is_segment = false.into(); - span.parent_span_id = SpanId(parent_span_id).into(); - } else { - span.is_segment = true.into(); + if from.parent_span_id.is_empty() { span.segment_id = span.span_id.clone(); } span @@ -149,30 +150,6 @@ pub enum SpanKind { Consumer = 5, } -impl SpanKind { - pub fn as_str_name(&self) -> &'static str { - match self { - SpanKind::Unspecified => "SPAN_KIND_UNSPECIFIED", - SpanKind::Internal => "SPAN_KIND_INTERNAL", - SpanKind::Server => "SPAN_KIND_SERVER", - SpanKind::Client => "SPAN_KIND_CLIENT", - SpanKind::Producer => "SPAN_KIND_PRODUCER", - SpanKind::Consumer => "SPAN_KIND_CONSUMER", - } - } - pub fn from_str_name(value: &str) -> ::core::option::Option { - match value { - "SPAN_KIND_UNSPECIFIED" => Some(Self::Unspecified), - "SPAN_KIND_INTERNAL" => Some(Self::Internal), - "SPAN_KIND_SERVER" => Some(Self::Server), - "SPAN_KIND_CLIENT" => Some(Self::Client), - "SPAN_KIND_PRODUCER" => Some(Self::Producer), - "SPAN_KIND_CONSUMER" => Some(Self::Consumer), - _ => None, - } - } -} - #[derive(Clone, Debug, Deserialize, Default)] pub struct Status { #[serde(default, skip_serializing_if = "String::is_empty")] @@ -189,24 +166,6 @@ pub enum StatusCode { Error = 2, } -impl StatusCode { - pub fn as_str_name(&self) -> &'static str { - match self { - StatusCode::Unset => "STATUS_CODE_UNSET", - StatusCode::Ok => "STATUS_CODE_OK", - StatusCode::Error => "STATUS_CODE_ERROR", - } - } - pub fn from_str_name(value: &str) -> ::core::option::Option { - match value { - "STATUS_CODE_UNSET" => Some(Self::Unset), - "STATUS_CODE_OK" => Some(Self::Ok), - "STATUS_CODE_ERROR" => Some(Self::Error), - _ => None, - } - } -} - #[derive(Clone, Debug, Deserialize)] pub enum AnyValue { #[serde(rename = "arrayValue")] @@ -264,14 +223,6 @@ pub struct KeyValue { pub value: AnyValue, } -#[derive(Debug, Deserialize)] -pub struct InstrumentationScope { - pub name: String, - pub version: String, - pub attributes: Vec, - pub dropped_attributes_count: u32, -} - #[cfg(test)] mod tests { use super::*; From 74c93d65e9dafedc07ea98366ab7f4d04f261e5e Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 11:40:59 -0400 Subject: [PATCH 19/59] Use macro to access map value --- relay-spans/src/span.rs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 3a22f76f90..99e6babb56 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -226,7 +226,7 @@ pub struct KeyValue { #[cfg(test)] mod tests { use super::*; - use relay_protocol::Annotated; + use relay_protocol::{get_path, Annotated}; #[test] fn parse_span() { @@ -288,16 +288,8 @@ mod tests { let otel_span: Span = serde_json::from_str(json).unwrap(); let event_span: Annotated = Annotated::new(otel_span.into()); assert_eq!( - event_span - .value() - .unwrap() - .data - .value() - .unwrap() - .get("sentry.environment") - .unwrap() - .as_str(), - Some("test") + get_path!(event_span.data["sentry.environment"]), + Some(&Annotated::new("test".into())) ); } } From 28cc43ddd24014fe2a85b2ff71dfca0a825387c5 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 11:58:34 -0400 Subject: [PATCH 20/59] Add documentation --- relay-spans/src/lib.rs | 2 ++ relay-spans/src/span.rs | 52 ++++++++++++++++++++++++++++++--- relay-spans/src/status_codes.rs | 4 +++ 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index f034b09262..bda115c0f3 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -1,3 +1,5 @@ +//! Structs and functions needed to ingest OpenTelemetry spans. + #![warn(missing_docs)] #![doc( html_logo_url = "https://raw.githubusercontent.com/getsentry/relay/master/artwork/relay-icon.png", diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 99e6babb56..cb81854a86 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -10,34 +10,78 @@ use relay_protocol::{Annotated, Object, Value}; use crate::status_codes; /// This is a serde implementation of https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/trace/v1/trace.proto. - +/// A Span represents a single operation performed by a single component of the system. #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Span { - /// Trace ID + /// A unique identifier for a trace. All spans from the same trace share + /// the same `trace_id`. The ID is a 16-byte array. An ID with all zeroes OR + /// of length other than 16 bytes is considered invalid (empty string in OTLP/JSON + /// is zero-length and thus is also invalid). + /// + /// This field is required. pub trace_id: String, - /// Span ID + /// A unique identifier for a span within a trace, assigned when the span + /// is created. The ID is an 8-byte array. An ID with all zeroes OR of length + /// other than 8 bytes is considered invalid (empty string in OTLP/JSON + /// is zero-length and thus is also invalid). + /// + /// This field is required. pub span_id: String, - /// Trace state + /// trace_state conveys information about request position in multiple distributed tracing graphs. + /// It is a trace_state in w3c-trace-context format: + /// See also for more details about this field. #[serde(default, skip_serializing_if = "String::is_empty")] pub trace_state: String, + /// The `span_id` of this span's parent span. If this is a root span, then this + /// field must be empty. The ID is an 8-byte array. #[serde(default, skip_serializing_if = "String::is_empty")] pub parent_span_id: String, + /// A description of the span's operation. + /// + /// For example, the name can be a qualified method name or a file name + /// and a line number where the operation is called. A best practice is to use + /// the same display name at the same call point in an application. + /// This makes it easier to correlate spans in different traces. + /// + /// This field is semantically required to be set to non-empty string. + /// Empty value is equivalent to an unknown span name. + /// + /// This field is required. pub name: String, + /// Distinguishes between spans generated in a particular context. For example, + /// two spans with the same name may be distinguished using `CLIENT` (caller) + /// and `SERVER` (callee) to identify queueing latency associated with the span. pub kind: SpanKind, + /// Timestamp when the span started in nanoseconds. pub start_time_unix_nano: i64, + /// Timestamp when the span ended in nanoseconds. pub end_time_unix_nano: i64, + /// Arbitrary additional data on a span, like `extra` on the top-level event. pub attributes: Vec, + /// dropped_attributes_count is the number of attributes that were discarded. Attributes + /// can be discarded because their keys are too long or because there are too many + /// attributes. If this value is 0, then no attributes were dropped. pub dropped_attributes_count: u32, + /// events is a collection of Event items.`` pub events: Vec, + /// dropped_events_count is the number of dropped events. If the value is 0, then no + /// events were dropped. pub dropped_events_count: u32, + /// links is a collection of Links, which are references from this span to a span + /// in the same or different trace. pub links: Vec, + /// links is a collection of Links, which are references from this span to a span + /// in the same or different trace. pub dropped_links_count: u32, + /// An optional final status for this span. Semantically when Status isn't set, it means + /// span's status code is unset, i.e. assume STATUS_CODE_UNSET (code = 0). #[serde(default)] pub status: Status, } impl Span { + /// sentry_status() returns a status as defined by Sentry based on the span status. pub fn sentry_status(&self) -> &'static str { let status_code = self.status.code.clone(); diff --git a/relay-spans/src/status_codes.rs b/relay-spans/src/status_codes.rs index 0678a447e6..a55586135d 100644 --- a/relay-spans/src/status_codes.rs +++ b/relay-spans/src/status_codes.rs @@ -2,6 +2,8 @@ use std::collections::BTreeMap; use once_cell::sync::Lazy; +/// HTTP maps some HTTP codes to Sentry's span statuses. +/// See possible mapping in https://develop.sentry.dev/sdk/event-payloads/span/. pub static HTTP: Lazy> = Lazy::new(|| { BTreeMap::from([ (400, "failed_precondition"), @@ -18,6 +20,8 @@ pub static HTTP: Lazy> = Lazy::new(|| { ]) }); +/// GRPC maps some GRPC codes to Sentry's span statuses. +/// See description in grpc documentation. pub static GRPC: Lazy> = Lazy::new(|| { BTreeMap::from([ (1, "cancelled"), From 7f63d7c80401129211b3142f43f9d4b15c298f4c Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 17:56:52 -0400 Subject: [PATCH 21/59] Fix links --- relay-spans/src/span.rs | 4 ++-- relay-spans/src/status_codes.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index cb81854a86..e96e13f1db 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -9,7 +9,7 @@ use relay_protocol::{Annotated, Object, Value}; use crate::status_codes; -/// This is a serde implementation of https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/trace/v1/trace.proto. +/// This is a serde implementation of . /// A Span represents a single operation performed by a single component of the system. #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] @@ -29,7 +29,7 @@ pub struct Span { /// This field is required. pub span_id: String, /// trace_state conveys information about request position in multiple distributed tracing graphs. - /// It is a trace_state in w3c-trace-context format: + /// It is a trace_state in w3c-trace-context format: . /// See also for more details about this field. #[serde(default, skip_serializing_if = "String::is_empty")] pub trace_state: String, diff --git a/relay-spans/src/status_codes.rs b/relay-spans/src/status_codes.rs index a55586135d..9fb0643cfb 100644 --- a/relay-spans/src/status_codes.rs +++ b/relay-spans/src/status_codes.rs @@ -3,7 +3,7 @@ use std::collections::BTreeMap; use once_cell::sync::Lazy; /// HTTP maps some HTTP codes to Sentry's span statuses. -/// See possible mapping in https://develop.sentry.dev/sdk/event-payloads/span/. +/// See possible mapping in . pub static HTTP: Lazy> = Lazy::new(|| { BTreeMap::from([ (400, "failed_precondition"), From fa4e1ca14b726c53620f6dfab52beb3698e866d7 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 18:16:15 -0400 Subject: [PATCH 22/59] Add more documentation and use default where appropriate --- relay-spans/src/span.rs | 142 +++++++++++++++++++++++++++++++++++----- 1 file changed, 124 insertions(+), 18 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index e96e13f1db..eb6d077e47 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -18,25 +18,35 @@ pub struct Span { /// the same `trace_id`. The ID is a 16-byte array. An ID with all zeroes OR /// of length other than 16 bytes is considered invalid (empty string in OTLP/JSON /// is zero-length and thus is also invalid). - /// - /// This field is required. pub trace_id: String, /// A unique identifier for a span within a trace, assigned when the span /// is created. The ID is an 8-byte array. An ID with all zeroes OR of length /// other than 8 bytes is considered invalid (empty string in OTLP/JSON /// is zero-length and thus is also invalid). - /// - /// This field is required. pub span_id: String, /// trace_state conveys information about request position in multiple distributed tracing graphs. /// It is a trace_state in w3c-trace-context format: . /// See also for more details about this field. - #[serde(default, skip_serializing_if = "String::is_empty")] + #[serde(default)] pub trace_state: String, /// The `span_id` of this span's parent span. If this is a root span, then this /// field must be empty. The ID is an 8-byte array. - #[serde(default, skip_serializing_if = "String::is_empty")] + #[serde(default)] pub parent_span_id: String, + /// Flags, a bit field. 8 least significant bits are the trace + /// flags as defined in W3C Trace Context specification. Readers + /// MUST not assume that 24 most significant bits will be zero. + /// To read the 8-bit W3C trace flag, use `flags & SPAN_FLAGS_TRACE_FLAGS_MASK`. + /// + /// When creating span messages, if the message is logically forwarded from another source + /// with an equivalent flags fields (i.e., usually another OTLP span message), the field SHOULD + /// be copied as-is. If creating from a source that does not have an equivalent flags field + /// (such as a runtime representation of an OpenTelemetry span), the high 24 bits MUST + /// be set to zero. + /// + /// See https://www.w3.org/TR/trace-context-2/#trace-flags for the flag definitions. + #[serde(default)] + pub flags: u32, /// A description of the span's operation. /// /// For example, the name can be a qualified method name or a file name @@ -46,33 +56,59 @@ pub struct Span { /// /// This field is semantically required to be set to non-empty string. /// Empty value is equivalent to an unknown span name. - /// - /// This field is required. pub name: String, /// Distinguishes between spans generated in a particular context. For example, /// two spans with the same name may be distinguished using `CLIENT` (caller) /// and `SERVER` (callee) to identify queueing latency associated with the span. + #[serde(default)] pub kind: SpanKind, - /// Timestamp when the span started in nanoseconds. + /// start_time_unix_nano is the start time of the span. On the client side, this is the time + /// kept by the local machine where the span execution starts. On the server side, this + /// is the time when the server's application handler starts running. + /// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. + /// + /// This field is semantically required and it is expected that end_time >= start_time. pub start_time_unix_nano: i64, - /// Timestamp when the span ended in nanoseconds. + /// end_time_unix_nano is the end time of the span. On the client side, this is the time + /// kept by the local machine where the span execution ends. On the server side, this + /// is the time when the server application handler stops running. + /// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. + /// + /// This field is semantically required and it is expected that end_time >= start_time. pub end_time_unix_nano: i64, - /// Arbitrary additional data on a span, like `extra` on the top-level event. + /// attributes is a collection of key/value pairs. Note, global attributes + /// like server name can be set using the resource API. Examples of attributes: + /// + /// "/http/user_agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36" + /// "/http/server_latency": 300 + /// "example.com/myattribute": true + /// "example.com/score": 10.239 + /// + /// The OpenTelemetry API specification further restricts the allowed value types: + /// + /// Attribute keys MUST be unique (it is not allowed to have more than one + /// attribute with the same key). + #[serde(default)] pub attributes: Vec, /// dropped_attributes_count is the number of attributes that were discarded. Attributes /// can be discarded because their keys are too long or because there are too many /// attributes. If this value is 0, then no attributes were dropped. + #[serde(default)] pub dropped_attributes_count: u32, /// events is a collection of Event items.`` + #[serde(default)] pub events: Vec, /// dropped_events_count is the number of dropped events. If the value is 0, then no /// events were dropped. + #[serde(default)] pub dropped_events_count: u32, /// links is a collection of Links, which are references from this span to a span /// in the same or different trace. + #[serde(default)] pub links: Vec, /// links is a collection of Links, which are references from this span to a span /// in the same or different trace. + #[serde(default)] pub dropped_links_count: u32, /// An optional final status for this span. Semantically when Status isn't set, it means /// span's status code is unset, i.e. assume STATUS_CODE_UNSET (code = 0). @@ -166,50 +202,104 @@ impl From for EventSpan { } } +/// Event is a time-stamped annotation of the span, consisting of user-supplied +/// text description and key-value pairs. #[derive(Clone, Debug, Deserialize)] pub struct Event { - pub time_unix_nano: u64, - pub name: String, + /// attributes is a collection of attribute key/value pairs on the event. + /// Attribute keys MUST be unique (it is not allowed to have more than one + /// attribute with the same key). + #[serde(default)] pub attributes: Vec, + /// dropped_attributes_count is the number of dropped attributes. If the value is 0, + /// then no attributes were dropped. + #[serde(default)] pub dropped_attributes_count: u32, + /// name of the event. + /// This field is semantically required to be set to non-empty string. + pub name: String, + /// time_unix_nano is the time the event occurred. + #[serde(default)] + pub time_unix_nano: u64, } #[derive(Clone, Debug, Deserialize)] pub struct Link { - pub trace_id: String, - pub span_id: String, - pub trace_state: String, + /// attributes is a collection of attribute key/value pairs on the link. + /// Attribute keys MUST be unique (it is not allowed to have more than one + /// attribute with the same key). + #[serde(default)] pub attributes: Vec, + /// dropped_attributes_count is the number of dropped attributes. If the value is 0, + /// then no attributes were dropped. + #[serde(default)] pub dropped_attributes_count: u32, + /// A unique identifier for the linked span. The ID is an 8-byte array. + #[serde(default)] + pub span_id: String, + /// A unique identifier of a trace that this linked span is part of. The ID is a + /// 16-byte array. + #[serde(default)] + pub trace_id: String, + /// The trace_state associated with the link. + #[serde(default)] + pub trace_state: String, } -#[derive(Clone, Debug, Deserialize_repr)] +#[derive(Clone, Default, Debug, Deserialize_repr)] #[repr(u32)] pub enum SpanKind { + /// Unspecified. Do NOT use as default. + /// Implementations MAY assume SpanKind to be INTERNAL when receiving UNSPECIFIED. Unspecified = 0, + /// Indicates that the span represents an internal operation within an application, + /// as opposed to an operation happening at the boundaries. Default value. + #[default] Internal = 1, + /// Indicates that the span covers server-side handling of an RPC or other + /// remote network request. Server = 2, + /// Indicates that the span describes a request to some remote service. Client = 3, + /// Indicates that the span describes a producer sending a message to a broker. + /// Unlike CLIENT and SERVER, there is often no direct critical path latency relationship + /// between producer and consumer spans. A PRODUCER span ends when the message was accepted + /// by the broker while the logical processing of the message might span a much longer time. Producer = 4, + /// Indicates that the span describes consumer receiving a message from a broker. + /// Like the PRODUCER kind, there is often no direct critical path latency relationship + /// between producer and consumer spans. Consumer = 5, } #[derive(Clone, Debug, Deserialize, Default)] pub struct Status { - #[serde(default, skip_serializing_if = "String::is_empty")] + /// A developer-facing human readable error message. + #[serde(default)] pub message: String, + /// The status code. + #[serde(default)] pub code: StatusCode, } #[derive(Clone, Debug, Deserialize_repr, Default, PartialEq)] #[repr(u32)] pub enum StatusCode { + /// The default status. #[default] Unset = 0, + /// The Span has been validated by an Application developer or Operator to + /// have completed successfully. Ok = 1, + /// The Span contains an error. Error = 2, } +/// AnyValue is used to represent any type of attribute value. AnyValue may contain a +/// primitive value such as a string or integer or it may contain an arbitrary nested +/// object containing arrays, key-value lists and primitives. +/// The value is one of the listed fields. It is valid for all values to be unspecified +/// in which case this AnyValue is considered to be "empty". #[derive(Clone, Debug, Deserialize)] pub enum AnyValue { #[serde(rename = "arrayValue")] @@ -251,16 +341,32 @@ impl AnyValue { } } +/// ArrayValue is a list of AnyValue messages. We need ArrayValue as a message +/// since oneof in AnyValue does not allow repeated fields. #[derive(Clone, Debug, Deserialize)] pub struct ArrayValue { + /// Array of values. The array may be empty (contain 0 elements). + #[serde(default)] pub values: Vec, } +/// KeyValueList is a list of KeyValue messages. We need KeyValueList as a message +/// +/// since `oneof` in AnyValue does not allow repeated fields. Everywhere else where we need +/// a list of KeyValue messages (e.g. in Span) we use `repeated KeyValue` directly to +/// avoid unnecessary extra wrapping (which slows down the protocol). The 2 approaches +/// are semantically equivalent. #[derive(Clone, Debug, Deserialize)] pub struct KeyValueList { + /// A collection of key/value pairs of key-value pairs. The list may be empty (may + /// contain 0 elements). + /// The keys MUST be unique (it is not allowed to have more than one + /// value with the same key). pub values: Vec, } +/// KeyValue is a key-value pair that is used to store Span attributes, Link +/// attributes, etc. #[derive(Clone, Debug, Deserialize)] pub struct KeyValue { pub key: String, From b196ac774379482c73837e67e491477b889944ac Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 20 Oct 2023 18:47:32 -0400 Subject: [PATCH 23/59] Fix link and remove examples trying to be parsed during tests --- relay-spans/src/span.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index eb6d077e47..4d2c3c499a 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -44,7 +44,7 @@ pub struct Span { /// (such as a runtime representation of an OpenTelemetry span), the high 24 bits MUST /// be set to zero. /// - /// See https://www.w3.org/TR/trace-context-2/#trace-flags for the flag definitions. + /// See for the flag definitions. #[serde(default)] pub flags: u32, /// A description of the span's operation. @@ -77,12 +77,7 @@ pub struct Span { /// This field is semantically required and it is expected that end_time >= start_time. pub end_time_unix_nano: i64, /// attributes is a collection of key/value pairs. Note, global attributes - /// like server name can be set using the resource API. Examples of attributes: - /// - /// "/http/user_agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36" - /// "/http/server_latency": 300 - /// "example.com/myattribute": true - /// "example.com/score": 10.239 + /// like server name can be set using the resource API. /// /// The OpenTelemetry API specification further restricts the allowed value types: /// From 1831a2b485ee43d41f55034a973b5c85ada6ba5e Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 6 Nov 2023 08:23:22 -0500 Subject: [PATCH 24/59] Populate the received field --- relay-spans/src/span.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 4d2c3c499a..53cfdfae1c 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -152,10 +152,10 @@ impl Span { impl From for EventSpan { fn from(from: Span) -> Self { - let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); let exclusive_time = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; let mut attributes: Object = Object::new(); + let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); for attribute in from.attributes.clone() { match attribute.value { AnyValue::Array(_) => todo!(), @@ -179,10 +179,11 @@ impl From for EventSpan { data: attributes.into(), description: from.name.clone().into(), exclusive_time: exclusive_time.into(), - span_id: SpanId(from.span_id.clone()).into(), + is_segment: from.parent_span_id.is_empty().into(), parent_span_id: SpanId(from.parent_span_id.clone()).into(), + received: Timestamp(Utc::now()).into(), + span_id: SpanId(from.span_id.clone()).into(), start_timestamp: Timestamp(start_timestamp).into(), - is_segment: from.parent_span_id.is_empty().into(), timestamp: Timestamp(end_timestamp).into(), trace_id: TraceId(from.trace_id.clone()).into(), ..Default::default() From b94d80f53f6ee3a72aa36628ab1dd1708b2bea94 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 6 Nov 2023 08:43:20 -0500 Subject: [PATCH 25/59] Convert OTel attributes to Sentry tags --- Cargo.lock | 2 +- relay-spans/src/lib.rs | 1 + relay-spans/src/otel_to_sentry_tags.rs | 35 ++++++++++++++++++++++++++ relay-spans/src/span.rs | 14 ++++++++--- 4 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 relay-spans/src/otel_to_sentry_tags.rs diff --git a/Cargo.lock b/Cargo.lock index bf4b43f399..4882694305 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4576,7 +4576,7 @@ checksum = "8725e1dfadb3a50f7e5ce0b1a540466f6ed3fe7a0fca2ac2b8b831d31316bd00" dependencies = [ "proc-macro2", "quote", - "syn 2.0.32", + "syn 2.0.38", ] [[package]] diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index bda115c0f3..615b650509 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -8,5 +8,6 @@ pub use crate::span::Span; +mod otel_to_sentry_tags; mod span; mod status_codes; diff --git a/relay-spans/src/otel_to_sentry_tags.rs b/relay-spans/src/otel_to_sentry_tags.rs new file mode 100644 index 0000000000..45fbd4b157 --- /dev/null +++ b/relay-spans/src/otel_to_sentry_tags.rs @@ -0,0 +1,35 @@ +use std::collections::BTreeMap; + +use once_cell::sync::Lazy; + +pub static OTEL_TO_SENTRY_TAGS: Lazy> = Lazy::new(|| { + BTreeMap::from([ + ("sentry.release", "release"), + ("sentry.environment", "environment"), + ("sentry.origin", "origin"), + ("sentry.op", "op"), + ("sentry.source", "source"), + ("sentry.sample_rate", "sample_rate"), + ("enduser.id", "user.id"), + ("sentry.user.username", "user.username"), + ("sentry.user.email", "user.email"), + ("sentry.user.ip_address", "user.ip_address"), + ("sentry.user.segment", "user.segment"), + ("sentry.user.geo.city", "user.geo.city"), + ("sentry.user.geo.country_code", "user.geo.country_code"), + ("sentry.user.geo.region", "user.geo.region"), + ("http.request.method", "request.method"), + ("url.full", "request.url"), + ("url.query_string", "request.query_string"), + ("http.request.cookies", "request.cookies"), + ( + "http.request.headers.content-type", + "request.headers.content-type", + ), + ("http.request.env", "request.env"), + ("sentry.sdk.name", "sdk.name"), + ("sentry.sdk.version", "sdk.version"), + ("sentry.sdk.integrations", "sdk.integrations"), + ("sentry.sdk.packages", "sdk.packages"), + ]) +}); diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 53cfdfae1c..76623ed534 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -7,6 +7,7 @@ use serde_repr::Deserialize_repr; use relay_event_schema::protocol::{Span as EventSpan, SpanId, SpanStatus, Timestamp, TraceId}; use relay_protocol::{Annotated, Object, Value}; +use crate::otel_to_sentry_tags::OTEL_TO_SENTRY_TAGS; use crate::status_codes; /// This is a serde implementation of . @@ -157,21 +158,26 @@ impl From for EventSpan { let mut attributes: Object = Object::new(); let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); for attribute in from.attributes.clone() { + let key: String = if let Some(key) = OTEL_TO_SENTRY_TAGS.get(attribute.key.as_str()) { + key.to_string() + } else { + attribute.key + }; match attribute.value { AnyValue::Array(_) => todo!(), AnyValue::Bool(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); + attributes.insert(key, Annotated::new(v.into())); } AnyValue::Bytes(_) => todo!(), AnyValue::Double(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); + attributes.insert(key, Annotated::new(v.into())); } AnyValue::Int(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); + attributes.insert(key, Annotated::new(v.into())); } AnyValue::Kvlist(_) => todo!(), AnyValue::String(v) => { - attributes.insert(attribute.key, Annotated::new(v.into())); + attributes.insert(key, Annotated::new(v.into())); } }; } From ce73994a059870162b1d2c73f218990b822e0023 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 6 Nov 2023 08:48:56 -0500 Subject: [PATCH 26/59] Add a CHANGELOG entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab5de5bc54..39b320db80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Allow advanced scrubbing expressions for datascrubbing safe fields. ([#2670](https://github.com/getsentry/relay/pull/2670)) - Track when a span was received. ([#2688](https://github.com/getsentry/relay/pull/2688)) - Add context for NEL (Network Error Logging) reports to the event schema. ([#2421](https://github.com/getsentry/relay/pull/2421)) +- Ingest OpenTelemetry spans via HTTP or an envelope. ([#2620](https://github.com/getsentry/relay/pull/2620)) **Bug Fixes**: From aa8874a7e6a4f0fe8901bf015bcf1a76d666ed21 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Thu, 9 Nov 2023 10:37:10 -0500 Subject: [PATCH 27/59] Simplify span handling --- relay-server/src/actors/processor.rs | 37 +++++++--------------------- 1 file changed, 9 insertions(+), 28 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 45839ac4ba..18a01ac21c 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2507,36 +2507,17 @@ impl EnvelopeProcessorService { let mut items: Vec = Vec::new(); state.managed_envelope.retain_items(|item| match item.ty() { ItemType::OtelSpan => { - let span: relay_spans::Span = match serde_json::from_slice(&item.payload()) { - Ok(span) => span, - Err(e) => { - relay_log::error!(error = &e as &dyn Error, "failed to parse OTel span"); - return ItemAction::DropSilently; - } - }; - let event_span: Annotated = Annotated::new(span.into()); - match self.validate_span(event_span) { - Ok(span) => { - let payload: Bytes = match span.to_json() { - Ok(payload) => payload.into(), - Err(e) => { - relay_log::error!( - error = &e as &dyn Error, - "Failed to serialize OTel span" - ); - return ItemAction::DropSilently; - } - }; - let mut new_item = Item::new(ItemType::Span); - new_item.set_payload(ContentType::Json, payload); - items.push(new_item); - ItemAction::DropSilently - } - Err(e) => { - relay_log::error!("Invalid span: {e}"); - ItemAction::DropSilently + if let Ok(relay_span) = serde_json::from_slice::(&item.payload()) + { + if let Ok(span) = self.validate_span(Annotated::new(relay_span.into())) { + if let Ok(payload) = span.to_json() { + let mut new_item = Item::new(ItemType::Span); + new_item.set_payload(ContentType::Json, payload); + items.push(new_item); + } } } + ItemAction::DropSilently } _ => ItemAction::Keep, }); From 8de70ef779f10a506610fb6ce127a7f518d50c1b Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 13 Nov 2023 11:28:26 -0500 Subject: [PATCH 28/59] Add missing entry --- relay-server/src/envelope.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/relay-server/src/envelope.rs b/relay-server/src/envelope.rs index 28dba8b8d3..1341ac2a88 100644 --- a/relay-server/src/envelope.rs +++ b/relay-server/src/envelope.rs @@ -195,6 +195,7 @@ impl std::str::FromStr for ItemType { "replay_recording" => Self::ReplayRecording, "check_in" => Self::CheckIn, "span" => Self::Span, + "otel_span" => Self::OtelSpan, other => Self::Unknown(other.to_owned()), }) } From 3cb8069f6b64f7bcd8ce5702c8cec85cb7fd27cb Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 13 Nov 2023 11:33:15 -0500 Subject: [PATCH 29/59] Rename Span to OtelSpan --- relay-server/src/actors/processor.rs | 3 ++- relay-spans/src/lib.rs | 2 +- relay-spans/src/span.rs | 12 ++++++------ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 18a01ac21c..69e0c79f30 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2507,7 +2507,8 @@ impl EnvelopeProcessorService { let mut items: Vec = Vec::new(); state.managed_envelope.retain_items(|item| match item.ty() { ItemType::OtelSpan => { - if let Ok(relay_span) = serde_json::from_slice::(&item.payload()) + if let Ok(relay_span) = + serde_json::from_slice::(&item.payload()) { if let Ok(span) = self.validate_span(Annotated::new(relay_span.into())) { if let Ok(payload) = span.to_json() { diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index 615b650509..d15c4c79f8 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -6,7 +6,7 @@ html_favicon_url = "https://raw.githubusercontent.com/getsentry/relay/master/artwork/relay-icon.png" )] -pub use crate::span::Span; +pub use crate::span::OtelSpan; mod otel_to_sentry_tags; mod span; diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 76623ed534..9cdc856199 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -14,7 +14,7 @@ use crate::status_codes; /// A Span represents a single operation performed by a single component of the system. #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct Span { +pub struct OtelSpan { /// A unique identifier for a trace. All spans from the same trace share /// the same `trace_id`. The ID is a 16-byte array. An ID with all zeroes OR /// of length other than 16 bytes is considered invalid (empty string in OTLP/JSON @@ -112,7 +112,7 @@ pub struct Span { pub status: Status, } -impl Span { +impl OtelSpan { /// sentry_status() returns a status as defined by Sentry based on the span status. pub fn sentry_status(&self) -> &'static str { let status_code = self.status.code.clone(); @@ -151,8 +151,8 @@ impl Span { } } -impl From for EventSpan { - fn from(from: Span) -> Self { +impl From for EventSpan { + fn from(from: OtelSpan) -> Self { let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); let exclusive_time = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; let mut attributes: Object = Object::new(); @@ -437,10 +437,10 @@ mod tests { "links": [], "droppedLinksCount": 0 }"#; - let otel_span: Span = serde_json::from_str(json).unwrap(); + let otel_span: OtelSpan = serde_json::from_str(json).unwrap(); let event_span: Annotated = Annotated::new(otel_span.into()); assert_eq!( - get_path!(event_span.data["sentry.environment"]), + get_path!(event_span.data["environment"]), Some(&Annotated::new("test".into())) ); } From da542e97ba6189aeac47080444d613a3001a3482 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 26 Nov 2023 22:31:37 -0500 Subject: [PATCH 30/59] Extract metrics from standalone spans only --- relay-server/src/actors/processor.rs | 29 ++++++++++++++++++++++ relay-server/src/metrics_extraction/mod.rs | 3 +-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 12e16ad698..f9ecae5fa6 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2515,9 +2515,33 @@ impl EnvelopeProcessorService { }); } + fn extract_span_metrics(&self, state: &mut ProcessEnvelopeState) { + let span_metrics_extraction_config = match state.project_state.config.metric_extraction { + ErrorBoundary::Ok(ref config) if config.is_enabled() => config, + _ => return, + }; + for item in state.managed_envelope.envelope().items() { + if item.ty() != &ItemType::OtelSpan { + continue; + } + if let Ok(otel_span) = serde_json::from_slice::(&item.payload()) + { + if let Ok(event_span) = self.validate_span(Annotated::new(otel_span.into())) { + if let Some(span) = event_span.value() { + crate::metrics_extraction::generic::extract_metrics( + span, + span_metrics_extraction_config, + ); + } + } + } + } + } + #[cfg(feature = "processing")] fn process_spans(&self, state: &mut ProcessEnvelopeState) { let mut items: Vec = Vec::new(); + state.managed_envelope.retain_items(|item| match item.ty() { ItemType::OtelSpan => { if let Ok(relay_span) = @@ -2820,6 +2844,11 @@ impl EnvelopeProcessorService { }); } + // Extract span metrics from standalone spans only + if self.inner.config.processing_enabled() || state.sampling_result.should_drop() { + self.extract_span_metrics(state); + } + if_processing!({ self.enforce_quotas(state)?; self.process_profiles(state); diff --git a/relay-server/src/metrics_extraction/mod.rs b/relay-server/src/metrics_extraction/mod.rs index 5af977d6fe..4980fe0107 100644 --- a/relay-server/src/metrics_extraction/mod.rs +++ b/relay-server/src/metrics_extraction/mod.rs @@ -1,9 +1,8 @@ use relay_common::time::UnixTimestamp; use relay_metrics::Bucket; -mod generic; - pub mod event; +pub mod generic; pub mod sessions; pub mod transactions; From f68651d99bc7112359cc6dfab198e8ac58f6985e Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 26 Nov 2023 23:01:44 -0500 Subject: [PATCH 31/59] Accept EventSpan as well --- relay-dynamic-config/src/feature.rs | 4 +- relay-server/src/actors/processor.rs | 82 +++++++++++++++++++--------- relay-server/src/envelope.rs | 13 ++++- 3 files changed, 68 insertions(+), 31 deletions(-) diff --git a/relay-dynamic-config/src/feature.rs b/relay-dynamic-config/src/feature.rs index 59167b7446..6bad026b81 100644 --- a/relay-dynamic-config/src/feature.rs +++ b/relay-dynamic-config/src/feature.rs @@ -34,8 +34,8 @@ pub enum Feature { #[serde(rename = "organizations:profiling")] Profiling, /// Enable OTel span ingestion. - #[serde(rename = "organizations:otel-span-ingestion")] - OtelSpanIngestion, + #[serde(rename = "organizations:standalone-span-ingestion")] + StandaloneSpanIngestion, /// Deprecated, still forwarded for older downstream Relays. #[serde(rename = "organizations:transaction-name-mark-scrubbed-as-sanitized")] diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 344f20a2e0..4eb433a654 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2506,12 +2506,17 @@ impl EnvelopeProcessorService { } fn filter_spans(&self, state: &mut ProcessEnvelopeState) { - let otel_span_ingestion_enabled = - state.project_state.has_feature(Feature::OtelSpanIngestion); + let standalone_span_ingestion_enabled = state + .project_state + .has_feature(Feature::StandaloneSpanIngestion); state.managed_envelope.retain_items(|item| match item.ty() { - ItemType::OtelSpan if !otel_span_ingestion_enabled => { - relay_log::warn!("dropping span because feature is disabled"); - ItemAction::DropSilently + ItemType::OtelSpan | ItemType::EventSpan => { + if !standalone_span_ingestion_enabled { + relay_log::warn!("dropping span because feature is disabled"); + ItemAction::DropSilently + } else { + ItemAction::Keep + } } _ => ItemAction::Keep, }); @@ -2522,19 +2527,31 @@ impl EnvelopeProcessorService { ErrorBoundary::Ok(ref config) if config.is_enabled() => config, _ => return, }; - for item in state.managed_envelope.envelope().items() { - if item.ty() != &ItemType::OtelSpan { - continue; + let extract_span_metrics = |span: Annotated| { + let span = match self.validate_span(span) { + Ok(span) => span, + Err(e) => { + relay_log::error!("Invalid span: {e}"); + return; + } + }; + if let Some(span) = span.value() { + crate::metrics_extraction::generic::extract_metrics( + span, + span_metrics_extraction_config, + ); } - if let Ok(otel_span) = serde_json::from_slice::(&item.payload()) - { - if let Ok(event_span) = self.validate_span(Annotated::new(otel_span.into())) { - if let Some(span) = event_span.value() { - crate::metrics_extraction::generic::extract_metrics( - span, - span_metrics_extraction_config, - ); - } + }; + for item in state.managed_envelope.envelope().items() { + if item.ty() == &ItemType::OtelSpan { + if let Ok(otel_span) = + serde_json::from_slice::(&item.payload()) + { + extract_span_metrics(Annotated::new(otel_span.into())); + } + } else if item.ty() == &ItemType::EventSpan { + if let Ok(event_span) = Annotated::::from_json_bytes(&item.payload()) { + extract_span_metrics(event_span); } } } @@ -2543,19 +2560,32 @@ impl EnvelopeProcessorService { #[cfg(feature = "processing")] fn process_spans(&self, state: &mut ProcessEnvelopeState) { let mut items: Vec = Vec::new(); - + let mut add_span = |span: Annotated| { + let span = match self.validate_span(span) { + Ok(span) => span, + Err(e) => { + relay_log::error!("Invalid span: {e}"); + return; + } + }; + if let Ok(payload) = span.to_json() { + let mut item = Item::new(ItemType::Span); + item.set_payload(ContentType::Json, payload); + items.push(item); + } + }; state.managed_envelope.retain_items(|item| match item.ty() { ItemType::OtelSpan => { - if let Ok(relay_span) = + if let Ok(otel_span) = serde_json::from_slice::(&item.payload()) { - if let Ok(span) = self.validate_span(Annotated::new(relay_span.into())) { - if let Ok(payload) = span.to_json() { - let mut new_item = Item::new(ItemType::Span); - new_item.set_payload(ContentType::Json, payload); - items.push(new_item); - } - } + add_span(Annotated::new(otel_span.into())); + } + ItemAction::DropSilently + } + ItemType::EventSpan => { + if let Ok(event_span) = Annotated::::from_json_bytes(&item.payload()) { + add_span(event_span); } ItemAction::DropSilently } diff --git a/relay-server/src/envelope.rs b/relay-server/src/envelope.rs index 1341ac2a88..6b16128883 100644 --- a/relay-server/src/envelope.rs +++ b/relay-server/src/envelope.rs @@ -116,6 +116,8 @@ pub enum ItemType { Span, /// A standalone OpenTelemetry span. OtelSpan, + /// A standalone span. + EventSpan, /// UserReport as an Event UserReportV2, /// A new item type that is yet unknown by this version of Relay. @@ -165,6 +167,7 @@ impl fmt::Display for ItemType { Self::CheckIn => write!(f, "check_in"), Self::Span => write!(f, "span"), Self::OtelSpan => write!(f, "otel_span"), + Self::EventSpan => write!(f, "event_span"), Self::Unknown(s) => s.fmt(f), } } @@ -196,6 +199,7 @@ impl std::str::FromStr for ItemType { "check_in" => Self::CheckIn, "span" => Self::Span, "otel_span" => Self::OtelSpan, + "event_span" => Self::EventSpan, other => Self::Unknown(other.to_owned()), }) } @@ -580,8 +584,9 @@ impl Item { ItemType::ClientReport => None, ItemType::CheckIn => Some(DataCategory::Monitor), ItemType::Unknown(_) => None, - ItemType::Span => None, // No outcomes, for now - ItemType::OtelSpan => None, // No outcomes, for now + ItemType::Span => None, // No outcomes, for now + ItemType::OtelSpan => None, // No outcomes, for now + ItemType::EventSpan => None, // No outcomes, for now } } @@ -756,7 +761,8 @@ impl Item { | ItemType::Profile | ItemType::CheckIn | ItemType::Span - | ItemType::OtelSpan => false, + | ItemType::OtelSpan + | ItemType::EventSpan => false, // The unknown item type can observe any behavior, most likely there are going to be no // item types added that create events. @@ -791,6 +797,7 @@ impl Item { ItemType::CheckIn => false, ItemType::Span => false, ItemType::OtelSpan => false, + ItemType::EventSpan => false, // Since this Relay cannot interpret the semantics of this item, it does not know // whether it requires an event or not. Depending on the strategy, this can cause two From d6e7b602535608492b4e460a4509476110c43e10 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 26 Nov 2023 23:16:32 -0500 Subject: [PATCH 32/59] Validate timestamps and IDs on the span --- relay-server/src/actors/processor.rs | 37 +++++++++++++++++++++------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 4eb433a654..f7e141a297 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2456,10 +2456,7 @@ impl EnvelopeProcessorService { add_span(transaction_span.into()); } - /// Helper for [`Self::extract_spans`]. - /// /// We do not extract spans with missing fields if those fields are required on the Kafka topic. - #[cfg(feature = "processing")] fn validate_span(&self, mut span: Annotated) -> Result, anyhow::Error> { let inner = span .value_mut() @@ -2469,14 +2466,36 @@ impl EnvelopeProcessorService { ref exclusive_time, ref mut tags, ref mut sentry_tags, + ref mut start_timestamp, + ref mut timestamp, + ref mut span_id, + ref mut trace_id, .. } = inner; - // The following required fields are already validated by the `TransactionsProcessor`: - // - `timestamp` - // - `start_timestamp` - // - `trace_id` - // - `span_id` - // + + trace_id + .value() + .ok_or(anyhow::anyhow!("span is missing trace_id"))?; + span_id + .value() + .ok_or(anyhow::anyhow!("span is missing span_id"))?; + + match (start_timestamp.value(), timestamp.value()) { + (Some(start), Some(end)) => { + if end < start { + return Err(anyhow::anyhow!( + "end timestamp is smaller than start timestamp" + )); + } + } + (_, None) => { + return Err(anyhow::anyhow!("timestamp hard-required for spans")); + } + (None, _) => { + return Err(anyhow::anyhow!("start_timestamp hard-required for spans")); + } + } + // `is_segment` is set by `extract_span`. exclusive_time .value() From 51c6ebfc7d90830663ffb3b378c7865d16d08b3f Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 26 Nov 2023 23:19:39 -0500 Subject: [PATCH 33/59] Fix EventSpan support --- relay-server/src/actors/processor.rs | 1 + relay-server/src/utils/rate_limits.rs | 1 + relay-server/src/utils/sizes.rs | 7 +------ 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index f7e141a297..a79cb91912 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -1718,6 +1718,7 @@ impl EnvelopeProcessorService { ItemType::CheckIn => false, ItemType::Span => false, ItemType::OtelSpan => false, + ItemType::EventSpan => false, // Without knowing more, `Unknown` items are allowed to be repeated ItemType::Unknown(_) => false, diff --git a/relay-server/src/utils/rate_limits.rs b/relay-server/src/utils/rate_limits.rs index e6f37971ae..b4f33e3be5 100644 --- a/relay-server/src/utils/rate_limits.rs +++ b/relay-server/src/utils/rate_limits.rs @@ -111,6 +111,7 @@ fn infer_event_category(item: &Item) -> Option { ItemType::CheckIn => None, ItemType::Span => None, ItemType::OtelSpan => None, + ItemType::EventSpan => None, ItemType::Unknown(_) => None, } } diff --git a/relay-server/src/utils/sizes.rs b/relay-server/src/utils/sizes.rs index 3851e0c332..736d1f6d93 100644 --- a/relay-server/src/utils/sizes.rs +++ b/relay-server/src/utils/sizes.rs @@ -65,12 +65,7 @@ pub fn check_envelope_size_limits(config: &Config, envelope: &Envelope) -> bool ItemType::UserReport => (), ItemType::Statsd => (), ItemType::MetricBuckets => (), - ItemType::Span => { - if item.len() > config.max_span_size() { - return false; - } - } - ItemType::OtelSpan => { + ItemType::Span | ItemType::OtelSpan | ItemType::EventSpan => { if item.len() > config.max_span_size() { return false; } From 7f8157f8148b3f73dcf168a3ff27467322d21894 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 26 Nov 2023 23:26:55 -0500 Subject: [PATCH 34/59] Fix import --- relay-server/src/actors/processor.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index a79cb91912..91d0937b44 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -33,7 +33,7 @@ use relay_event_schema::protocol::{ Breadcrumb, ClientReport, Contexts, Csp, Event, EventType, ExpectCt, ExpectStaple, Hpkp, IpAddr, LenientString, Metrics, NetworkReportError, OtelContext, ProfileContext, RelayInfo, Replay, SecurityReportType, SessionAggregates, SessionAttributes, SessionStatus, SessionUpdate, - Timestamp, TraceContext, UserReport, Values, + Span, Timestamp, TraceContext, UserReport, Values, }; use relay_filter::FilterStatKey; use relay_metrics::aggregator::AggregatorConfig; @@ -58,7 +58,6 @@ use { crate::actors::project_cache::UpdateRateLimits, crate::utils::{EnvelopeLimiter, MetricsLimiter}, relay_event_normalization::{span, StoreConfig, StoreProcessor}, - relay_event_schema::protocol::Span, relay_metrics::Aggregator, relay_quotas::{RateLimitingError, RedisRateLimiter}, relay_redis::RedisPool, From 0b7beebcadfad09100ba0b52ca8b51d98c48ff76 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 26 Nov 2023 23:30:50 -0500 Subject: [PATCH 35/59] Make sure everything is behind the feature flag --- relay-server/src/actors/processor.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 91d0937b44..d5dcdcf2b6 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2542,6 +2542,12 @@ impl EnvelopeProcessorService { } fn extract_span_metrics(&self, state: &mut ProcessEnvelopeState) { + if state + .project_state + .has_feature(Feature::StandaloneSpanIngestion) + { + return; + } let span_metrics_extraction_config = match state.project_state.config.metric_extraction { ErrorBoundary::Ok(ref config) if config.is_enabled() => config, _ => return, @@ -2593,7 +2599,13 @@ impl EnvelopeProcessorService { items.push(item); } }; + let standalone_span_ingestion_enabled = state + .project_state + .has_feature(Feature::StandaloneSpanIngestion); + state.managed_envelope.retain_items(|item| match item.ty() { + ItemType::OtelSpan if !standalone_span_ingestion_enabled => ItemAction::DropSilently, + ItemType::EventSpan if !standalone_span_ingestion_enabled => ItemAction::DropSilently, ItemType::OtelSpan => { if let Ok(otel_span) = serde_json::from_slice::(&item.payload()) From 36904cc7a452f95212f77f106f809490749f7805 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 26 Nov 2023 23:46:47 -0500 Subject: [PATCH 36/59] Remove todos on attribute value handling --- relay-spans/src/span.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 9cdc856199..136fdc17c9 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -164,18 +164,22 @@ impl From for EventSpan { attribute.key }; match attribute.value { - AnyValue::Array(_) => todo!(), + AnyValue::Array(_) => {} AnyValue::Bool(v) => { attributes.insert(key, Annotated::new(v.into())); } - AnyValue::Bytes(_) => todo!(), + AnyValue::Bytes(v) => { + if let Ok(v) = String::from_utf8(v.clone()) { + attributes.insert(key, Annotated::new(v.into())); + } + } AnyValue::Double(v) => { attributes.insert(key, Annotated::new(v.into())); } AnyValue::Int(v) => { attributes.insert(key, Annotated::new(v.into())); } - AnyValue::Kvlist(_) => todo!(), + AnyValue::Kvlist(_) => {} AnyValue::String(v) => { attributes.insert(key, Annotated::new(v.into())); } From e93226238b8c72c60b8b7037a58ca3a9c3d2adc3 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Mon, 27 Nov 2023 17:17:50 +0100 Subject: [PATCH 37/59] test: integration --- relay-server/src/actors/processor.rs | 4 + tests/integration/fixtures/__init__.py | 22 +++- tests/integration/fixtures/processing.py | 10 ++ tests/integration/test_store.py | 137 ++++++++++++++++++++++- 4 files changed, 170 insertions(+), 3 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index e1bc0af3ac..b594c593e6 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2352,12 +2352,16 @@ impl EnvelopeProcessorService { serde_json::from_slice::(&item.payload()) { add_span(Annotated::new(otel_span.into())); + } else { + relay_log::debug!("Failed to parse OTel span"); } ItemAction::DropSilently } ItemType::EventSpan => { if let Ok(event_span) = Annotated::::from_json_bytes(&item.payload()) { add_span(event_span); + } else { + relay_log::debug!("Failed to parse span"); } ItemAction::DropSilently } diff --git a/tests/integration/fixtures/__init__.py b/tests/integration/fixtures/__init__.py index d7471e6336..2f6d4ff51b 100644 --- a/tests/integration/fixtures/__init__.py +++ b/tests/integration/fixtures/__init__.py @@ -162,7 +162,6 @@ def send_nel_event( dsn_key_idx=0, dsn_key=None, ): - if payload is None: payload = [ { @@ -204,6 +203,27 @@ def send_nel_event( return + def send_otel_span( + self, + project_id, + payload, + headers=None, + dsn_key_idx=0, + dsn_key=None, + ): + headers = { + "Content-Type": "application/json", + **(headers or {}), + } + + if dsn_key is None: + dsn_key = self.get_dsn_public_key(project_id, dsn_key_idx) + + url = f"/api/{project_id}/spans/?sentry_key={dsn_key}" + + response = self.post(url, headers=headers, json=payload) + response.raise_for_status() + def send_options(self, project_id, headers=None, dsn_key_idx=0): headers = { "X-Sentry-Auth": self.get_auth_header(project_id, dsn_key_idx), diff --git a/tests/integration/fixtures/processing.py b/tests/integration/fixtures/processing.py index abec32ae44..f7b91ae2a6 100644 --- a/tests/integration/fixtures/processing.py +++ b/tests/integration/fixtures/processing.py @@ -435,3 +435,13 @@ def get_span(self): assert message.error() is None return json.loads(message.value()) + + def get_spans(self, timeout=None, max_attempts=100): + for _ in range(max_attempts): + message = self.poll(timeout=timeout) + + if message is None: + return + else: + assert message.error() is None + yield json.loads(message.value()) diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 67d6607ee8..4b7a739939 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -8,8 +8,10 @@ import pytest from datetime import datetime, timedelta, timezone -from requests.exceptions import HTTPError + from flask import abort, Response +from requests.exceptions import HTTPError +from sentry_sdk.envelope import Envelope, Item, PayloadRef def test_store(mini_sentry, relay_chain): @@ -1209,7 +1211,7 @@ def test_invalid_project_id(mini_sentry, relay): pytest.raises(queue.Empty, lambda: mini_sentry.captured_events.get(timeout=1)) -def test_spans( +def test_span_extraction( mini_sentry, relay_with_processing, spans_consumer, @@ -1299,3 +1301,134 @@ def test_spans( } spans_consumer.assert_empty() + + +def test_span_ingestion( + mini_sentry, + relay_with_processing, + spans_consumer, + metrics_consumer, +): + spans_consumer = spans_consumer() + metrics_consumer = metrics_consumer() + + relay = relay_with_processing() + project_id = 42 + project_config = mini_sentry.add_full_project_config(project_id) + project_config["config"]["features"] = [ + "organizations:standalone-span-ingestion", + # "projects:span-metrics-extraction", + # "projects:span-metrics-extraction-all-modules", + ] + + # 1 - Send OTel span and sentry span via envelope + envelope = Envelope() + envelope.add_item( + Item( + type="otel_span", + payload=PayloadRef( + bytes=json.dumps( + { + "traceId": "89143b0763095bd9c9955e8175d1fb23", + "spanId": "e342abb1214ca181", + "name": "my 1st OTel span", + "startTimeUnixNano": 1697620454980000000, + "endTimeUnixNano": 1697620454980078800, + } + ).encode() + ), + ) + ) + envelope.add_item( + Item( + type="span", + payload=PayloadRef( + bytes=json.dumps( + { + "op": "before_first_display", + "span_id": "bd429c44b67a3eb1", + "start_timestamp": 1597976300.0000000, + "timestamp": 1597976302.0000000, + "trace_id": "ff62a8b040f340bda5d830223def1d81", + }, + ).encode() + ), + ) + ) + relay.send_envelope(project_id, envelope) + + # 2 - Send OTel span via endpoint + relay.send_otel_span( + project_id, + { + "traceId": "89143b0763095bd9c9955e8175d1fb24", + "spanId": "e342abb1214ca182", + "name": "my 2nd OTel span", + "startTimeUnixNano": 1697620454981000000, + "endTimeUnixNano": 1697620454981078800, + }, + ) + + spans = list(spans_consumer.get_spans()) + for span in spans: + del span["start_time"] + span["span"].pop("received", None) + + spans.sort( + key=lambda msg: msg["span"].get("description", "") + ) # endpoint might overtake envelope + + assert spans == [ + { + "organization_id": 1, + "project_id": 42, + "retention_days": 90, + "span": { + "op": "before_first_display", + "span_id": "bd429c44b67a3eb1", + "start_timestamp": 1597976300.0, + "timestamp": 1597976302.0, + "trace_id": "ff62a8b040f340bda5d830223def1d81", + }, + }, + { + "organization_id": 1, + "project_id": 42, + "retention_days": 90, + "span": { + "data": {}, + "description": "my 1st OTel span", + "exclusive_time": 0.0788, + "is_segment": True, + "parent_span_id": "", + "segment_id": "e342abb1214ca181", + "span_id": "e342abb1214ca181", + "start_timestamp": 1697620454.98, + "status": "ok", + "timestamp": 1697620454.980079, + "trace_id": "89143b0763095bd9c9955e8175d1fb23", + }, + }, + { + "organization_id": 1, + "project_id": 42, + "retention_days": 90, + "span": { + "data": {}, + "description": "my 2nd OTel span", + "exclusive_time": 0.0788, + "is_segment": True, + "parent_span_id": "", + "segment_id": "e342abb1214ca182", + "span_id": "e342abb1214ca182", + "start_timestamp": 1697620454.981, + "status": "ok", + "timestamp": 1697620454.981079, + "trace_id": "89143b0763095bd9c9955e8175d1fb24", + }, + }, + ] + + metrics = list(metrics_consumer.get_metrics()) + + assert metrics == ["TODO"] From 648b921028a68e257c799b5699bcff1ecadcbcdf Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 27 Nov 2023 12:52:13 -0500 Subject: [PATCH 38/59] Revert EventSpan addition --- relay-server/src/actors/processor.rs | 15 ++++----------- relay-server/src/envelope.rs | 13 +++---------- relay-server/src/utils/rate_limits.rs | 1 - relay-server/src/utils/sizes.rs | 2 +- 4 files changed, 8 insertions(+), 23 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index b594c593e6..222a7d117f 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -1458,7 +1458,6 @@ impl EnvelopeProcessorService { ItemType::CheckIn => false, ItemType::Span => false, ItemType::OtelSpan => false, - ItemType::EventSpan => false, // Without knowing more, `Unknown` items are allowed to be repeated ItemType::Unknown(_) => false, @@ -2110,12 +2109,6 @@ impl EnvelopeProcessorService { #[cfg(feature = "processing")] fn extract_spans(&self, state: &mut ProcessEnvelopeState) { - // For now, drop any spans submitted by the SDK. - state.managed_envelope.retain_items(|item| match item.ty() { - ItemType::Span => ItemAction::DropSilently, - _ => ItemAction::Keep, - }); - // Only extract spans from transactions (not errors). if state.event_type() != Some(EventType::Transaction) { return; @@ -2270,7 +2263,7 @@ impl EnvelopeProcessorService { .project_state .has_feature(Feature::StandaloneSpanIngestion); state.managed_envelope.retain_items(|item| match item.ty() { - ItemType::OtelSpan | ItemType::EventSpan => { + ItemType::OtelSpan | ItemType::Span => { if !standalone_span_ingestion_enabled { relay_log::warn!("dropping span because feature is disabled"); ItemAction::DropSilently @@ -2315,7 +2308,7 @@ impl EnvelopeProcessorService { { extract_span_metrics(Annotated::new(otel_span.into())); } - } else if item.ty() == &ItemType::EventSpan { + } else if item.ty() == &ItemType::Span { if let Ok(event_span) = Annotated::::from_json_bytes(&item.payload()) { extract_span_metrics(event_span); } @@ -2346,7 +2339,7 @@ impl EnvelopeProcessorService { state.managed_envelope.retain_items(|item| match item.ty() { ItemType::OtelSpan if !standalone_span_ingestion_enabled => ItemAction::DropSilently, - ItemType::EventSpan if !standalone_span_ingestion_enabled => ItemAction::DropSilently, + ItemType::Span if !standalone_span_ingestion_enabled => ItemAction::DropSilently, ItemType::OtelSpan => { if let Ok(otel_span) = serde_json::from_slice::(&item.payload()) @@ -2357,7 +2350,7 @@ impl EnvelopeProcessorService { } ItemAction::DropSilently } - ItemType::EventSpan => { + ItemType::Span => { if let Ok(event_span) = Annotated::::from_json_bytes(&item.payload()) { add_span(event_span); } else { diff --git a/relay-server/src/envelope.rs b/relay-server/src/envelope.rs index 67032f2b8c..f861b21ee0 100644 --- a/relay-server/src/envelope.rs +++ b/relay-server/src/envelope.rs @@ -118,8 +118,6 @@ pub enum ItemType { Span, /// A standalone OpenTelemetry span. OtelSpan, - /// A standalone span. - EventSpan, /// UserReport as an Event UserReportV2, /// A new item type that is yet unknown by this version of Relay. @@ -170,7 +168,6 @@ impl fmt::Display for ItemType { Self::CheckIn => write!(f, "check_in"), Self::Span => write!(f, "span"), Self::OtelSpan => write!(f, "otel_span"), - Self::EventSpan => write!(f, "event_span"), Self::Unknown(s) => s.fmt(f), } } @@ -203,7 +200,6 @@ impl std::str::FromStr for ItemType { "check_in" => Self::CheckIn, "span" => Self::Span, "otel_span" => Self::OtelSpan, - "event_span" => Self::EventSpan, other => Self::Unknown(other.to_owned()), }) } @@ -588,9 +584,8 @@ impl Item { ItemType::ClientReport => None, ItemType::CheckIn => Some(DataCategory::Monitor), ItemType::Unknown(_) => None, - ItemType::Span => None, // No outcomes, for now - ItemType::OtelSpan => None, // No outcomes, for now - ItemType::EventSpan => None, // No outcomes, for now + ItemType::Span => None, // No outcomes, for now + ItemType::OtelSpan => None, // No outcomes, for now } } @@ -766,8 +761,7 @@ impl Item { | ItemType::Profile | ItemType::CheckIn | ItemType::Span - | ItemType::OtelSpan - | ItemType::EventSpan => false, + | ItemType::OtelSpan => false, // The unknown item type can observe any behavior, most likely there are going to be no // item types added that create events. @@ -803,7 +797,6 @@ impl Item { ItemType::CheckIn => false, ItemType::Span => false, ItemType::OtelSpan => false, - ItemType::EventSpan => false, // Since this Relay cannot interpret the semantics of this item, it does not know // whether it requires an event or not. Depending on the strategy, this can cause two diff --git a/relay-server/src/utils/rate_limits.rs b/relay-server/src/utils/rate_limits.rs index 979801c086..f72eac8823 100644 --- a/relay-server/src/utils/rate_limits.rs +++ b/relay-server/src/utils/rate_limits.rs @@ -112,7 +112,6 @@ fn infer_event_category(item: &Item) -> Option { ItemType::CheckIn => None, ItemType::Span => None, ItemType::OtelSpan => None, - ItemType::EventSpan => None, ItemType::Unknown(_) => None, } } diff --git a/relay-server/src/utils/sizes.rs b/relay-server/src/utils/sizes.rs index 294b9462d3..9580d5196a 100644 --- a/relay-server/src/utils/sizes.rs +++ b/relay-server/src/utils/sizes.rs @@ -66,7 +66,7 @@ pub fn check_envelope_size_limits(config: &Config, envelope: &Envelope) -> bool ItemType::Statsd => (), ItemType::MetricBuckets => (), ItemType::MetricMeta => (), - ItemType::Span | ItemType::OtelSpan | ItemType::EventSpan => { + ItemType::Span | ItemType::OtelSpan => { if item.len() > config.max_span_size() { return false; } From 210504a044b747613c1a579b17e6d4e72a62a0be Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 27 Nov 2023 13:10:33 -0500 Subject: [PATCH 39/59] Streamline deserialization, normalization and metrics extraction --- relay-server/src/actors/processor.rs | 54 +++++----------------------- 1 file changed, 9 insertions(+), 45 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 222a7d117f..521fdf326f 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2191,6 +2191,7 @@ impl EnvelopeProcessorService { } /// We do not extract spans with missing fields if those fields are required on the Kafka topic. + #[cfg(feature = "processing")] fn validate_span(&self, mut span: Annotated) -> Result, anyhow::Error> { let inner = span .value_mut() @@ -2275,59 +2276,28 @@ impl EnvelopeProcessorService { }); } - fn extract_span_metrics(&self, state: &mut ProcessEnvelopeState) { - if state - .project_state - .has_feature(Feature::StandaloneSpanIngestion) - { - return; - } + #[cfg(feature = "processing")] + fn process_spans(&self, state: &mut ProcessEnvelopeState) { let span_metrics_extraction_config = match state.project_state.config.metric_extraction { ErrorBoundary::Ok(ref config) if config.is_enabled() => config, _ => return, }; - let extract_span_metrics = |span: Annotated| { - let span = match self.validate_span(span) { + let mut items: Vec = Vec::new(); + let mut add_span = |annotated_span: Annotated| { + let validated_span = match self.validate_span(annotated_span) { Ok(span) => span, Err(e) => { relay_log::error!("Invalid span: {e}"); return; } }; - if let Some(span) = span.value() { + if let Some(span_value) = validated_span.value() { crate::metrics_extraction::generic::extract_metrics( - span, + span_value, span_metrics_extraction_config, ); } - }; - for item in state.managed_envelope.envelope().items() { - if item.ty() == &ItemType::OtelSpan { - if let Ok(otel_span) = - serde_json::from_slice::(&item.payload()) - { - extract_span_metrics(Annotated::new(otel_span.into())); - } - } else if item.ty() == &ItemType::Span { - if let Ok(event_span) = Annotated::::from_json_bytes(&item.payload()) { - extract_span_metrics(event_span); - } - } - } - } - - #[cfg(feature = "processing")] - fn process_spans(&self, state: &mut ProcessEnvelopeState) { - let mut items: Vec = Vec::new(); - let mut add_span = |span: Annotated| { - let span = match self.validate_span(span) { - Ok(span) => span, - Err(e) => { - relay_log::error!("Invalid span: {e}"); - return; - } - }; - if let Ok(payload) = span.to_json() { + if let Ok(payload) = validated_span.to_json() { let mut item = Item::new(ItemType::Span); item.set_payload(ContentType::Json, payload); items.push(item); @@ -2336,7 +2306,6 @@ impl EnvelopeProcessorService { let standalone_span_ingestion_enabled = state .project_state .has_feature(Feature::StandaloneSpanIngestion); - state.managed_envelope.retain_items(|item| match item.ty() { ItemType::OtelSpan if !standalone_span_ingestion_enabled => ItemAction::DropSilently, ItemType::Span if !standalone_span_ingestion_enabled => ItemAction::DropSilently, @@ -2662,11 +2631,6 @@ impl EnvelopeProcessorService { }); } - // Extract span metrics from standalone spans only - if self.inner.config.processing_enabled() || state.sampling_result.should_drop() { - self.extract_span_metrics(state); - } - if_processing!({ self.enforce_quotas(state)?; self.process_profiles(state); From e8dc41af83f27756e5d1b86f5dd5ed34a3e95d37 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 27 Nov 2023 13:14:56 -0500 Subject: [PATCH 40/59] Fix import --- relay-server/src/actors/processor.rs | 3 ++- tests/integration/test_store.py | 13 +++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 521fdf326f..f36740f35a 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -29,7 +29,7 @@ use relay_event_schema::processor::{self, process_value, ProcessingAction, Proce use relay_event_schema::protocol::{ Breadcrumb, ClientReport, Contexts, Csp, Event, EventType, ExpectCt, ExpectStaple, Hpkp, IpAddr, LenientString, Metrics, NetworkReportError, OtelContext, ProfileContext, RelayInfo, - Replay, SecurityReportType, Span, Timestamp, TraceContext, UserReport, Values, + Replay, SecurityReportType, Timestamp, TraceContext, UserReport, Values, }; use relay_filter::FilterStatKey; use relay_metrics::aggregator::AggregatorConfig; @@ -54,6 +54,7 @@ use { crate::actors::project_cache::UpdateRateLimits, crate::utils::{EnvelopeLimiter, MetricsLimiter}, relay_event_normalization::{span, StoreConfig, StoreProcessor}, + relay_event_schema::protocol::Span, relay_metrics::{Aggregator, RedisMetricMetaStore}, relay_quotas::{RateLimitingError, RedisRateLimiter}, relay_redis::RedisPool, diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 4b7a739939..b1ce7cd92d 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1,15 +1,14 @@ import json import os import queue -from time import sleep -import uuid import socket import threading -import pytest +import uuid from datetime import datetime, timedelta, timezone +from time import sleep - -from flask import abort, Response +import pytest +from flask import Response, abort from requests.exceptions import HTTPError from sentry_sdk.envelope import Envelope, Item, PayloadRef @@ -230,8 +229,8 @@ def test_store_buffer_size(mini_sentry, relay): def test_store_max_concurrent_requests(mini_sentry, relay): - from time import sleep from threading import Semaphore + from time import sleep processing_store = False store_count = Semaphore() @@ -1317,8 +1316,6 @@ def test_span_ingestion( project_config = mini_sentry.add_full_project_config(project_id) project_config["config"]["features"] = [ "organizations:standalone-span-ingestion", - # "projects:span-metrics-extraction", - # "projects:span-metrics-extraction-all-modules", ] # 1 - Send OTel span and sentry span via envelope From a7c6608738ec928d9fc2790e7d6413329db9bcaa Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 27 Nov 2023 14:53:01 -0500 Subject: [PATCH 41/59] Do not return if the config is disabled --- relay-server/src/actors/processor.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index f36740f35a..85edfe5f31 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2280,8 +2280,8 @@ impl EnvelopeProcessorService { #[cfg(feature = "processing")] fn process_spans(&self, state: &mut ProcessEnvelopeState) { let span_metrics_extraction_config = match state.project_state.config.metric_extraction { - ErrorBoundary::Ok(ref config) if config.is_enabled() => config, - _ => return, + ErrorBoundary::Ok(ref config) if config.is_enabled() => Some(config), + _ => None, }; let mut items: Vec = Vec::new(); let mut add_span = |annotated_span: Annotated| { @@ -2292,11 +2292,10 @@ impl EnvelopeProcessorService { return; } }; - if let Some(span_value) = validated_span.value() { - crate::metrics_extraction::generic::extract_metrics( - span_value, - span_metrics_extraction_config, - ); + if let Some(config) = span_metrics_extraction_config { + if let Some(span_value) = validated_span.value() { + crate::metrics_extraction::generic::extract_metrics(span_value, config); + } } if let Ok(payload) = validated_span.to_json() { let mut item = Item::new(ItemType::Span); From 5900d0f307e86872d44aa944994bed03974206ba Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 27 Nov 2023 16:25:02 -0500 Subject: [PATCH 42/59] Add exclusive_time to spans in ingestion integration tests --- relay-server/src/actors/processor.rs | 18 +++++++----------- tests/integration/test_store.py | 2 ++ 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 85edfe5f31..c1ecc1b438 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2288,7 +2288,7 @@ impl EnvelopeProcessorService { let validated_span = match self.validate_span(annotated_span) { Ok(span) => span, Err(e) => { - relay_log::error!("Invalid span: {e}"); + relay_log::error!("invalid span: {e}"); return; } }; @@ -2310,20 +2310,16 @@ impl EnvelopeProcessorService { ItemType::OtelSpan if !standalone_span_ingestion_enabled => ItemAction::DropSilently, ItemType::Span if !standalone_span_ingestion_enabled => ItemAction::DropSilently, ItemType::OtelSpan => { - if let Ok(otel_span) = - serde_json::from_slice::(&item.payload()) - { - add_span(Annotated::new(otel_span.into())); - } else { - relay_log::debug!("Failed to parse OTel span"); + match serde_json::from_slice::(&item.payload()) { + Ok(otel_span) => add_span(Annotated::new(otel_span.into())), + Err(err) => relay_log::debug!("failed to parse OTel span: {}", err), } ItemAction::DropSilently } ItemType::Span => { - if let Ok(event_span) = Annotated::::from_json_bytes(&item.payload()) { - add_span(event_span); - } else { - relay_log::debug!("Failed to parse span"); + match Annotated::::from_json_bytes(&item.payload()) { + Ok(event_span) => add_span(event_span), + Err(err) => relay_log::debug!("failed to parse span: {}", err), } ItemAction::DropSilently } diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index b1ce7cd92d..fad5794ede 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1346,6 +1346,7 @@ def test_span_ingestion( "span_id": "bd429c44b67a3eb1", "start_timestamp": 1597976300.0000000, "timestamp": 1597976302.0000000, + "exclusive_time": 2.0, "trace_id": "ff62a8b040f340bda5d830223def1d81", }, ).encode() @@ -1385,6 +1386,7 @@ def test_span_ingestion( "span_id": "bd429c44b67a3eb1", "start_timestamp": 1597976300.0, "timestamp": 1597976302.0, + "exclusive_time": 2.0, "trace_id": "ff62a8b040f340bda5d830223def1d81", }, }, From 99bc5d34ae1093f527d3a04b7a2bdaec89b0ecdb Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 27 Nov 2023 16:41:47 -0500 Subject: [PATCH 43/59] Allow metrics extraction --- tests/integration/test_store.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index fad5794ede..3b38180e55 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1316,6 +1316,8 @@ def test_span_ingestion( project_config = mini_sentry.add_full_project_config(project_id) project_config["config"]["features"] = [ "organizations:standalone-span-ingestion", + "projects:span-metrics-extraction", + "projects:span-metrics-extraction-all-modules", ] # 1 - Send OTel span and sentry span via envelope From 7c30bed96d64db909a2cfe77ec05f1ef6abe30ba Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Mon, 27 Nov 2023 20:16:19 -0500 Subject: [PATCH 44/59] Add what the metric is supposed to look like --- tests/integration/test_store.py | 49 +++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 3b38180e55..3f21b2ab87 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1344,8 +1344,19 @@ def test_span_ingestion( payload=PayloadRef( bytes=json.dumps( { - "op": "before_first_display", + "description": "https://example.com/blah.js", + "is_segment": False, + "op": "resource.script", "span_id": "bd429c44b67a3eb1", + "segment_id": "968cff94913ebb07", + "sentry_tags": { + "category": "resource", + "description": "https://example.com/blah.js", + "group": "37e3d9fab1ae9162", + "op": "resource.script", + "transaction": "hi", + "transaction.op": "hi", + }, "start_timestamp": 1597976300.0000000, "timestamp": 1597976302.0000000, "exclusive_time": 2.0, @@ -1384,7 +1395,18 @@ def test_span_ingestion( "project_id": 42, "retention_days": 90, "span": { - "op": "before_first_display", + "description": "https://example.com/blah.js", + "is_segment": False, + "op": "resource.script", + "segment_id": "968cff94913ebb07", + "sentry_tags": { + "category": "resource", + "description": "https://example.com/blah.js", + "group": "37e3d9fab1ae9162", + "op": "resource.script", + "transaction": "hi", + "transaction.op": "hi", + }, "span_id": "bd429c44b67a3eb1", "start_timestamp": 1597976300.0, "timestamp": 1597976302.0, @@ -1432,4 +1454,25 @@ def test_span_ingestion( metrics = list(metrics_consumer.get_metrics()) - assert metrics == ["TODO"] + assert metrics == [ + { + "name": "d:spans/exclusive_time@millisecond", + "org_id": 1, + "retention_days": 90, + "project_id": 42, + "tags": { + "transaction": "hi", + }, + "type": "d", + "value": [2.0], + }, + { + "name": "d:spans/exclusive_time_light@millisecond", + "org_id": 1, + "retention_days": 90, + "project_id": 42, + "tags": {}, + "type": "d", + "value": [2.0], + }, + ] From 319e50ebcc2ce6072d776391e5502090ba10055c Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Tue, 28 Nov 2023 10:13:02 +0100 Subject: [PATCH 45/59] metrics test --- relay-server/src/actors/processor.rs | 5 +- tests/integration/test_store.py | 90 ++++++++++++++++++++++++---- 2 files changed, 83 insertions(+), 12 deletions(-) diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index c1ecc1b438..b487f6f08b 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -2279,6 +2279,8 @@ impl EnvelopeProcessorService { #[cfg(feature = "processing")] fn process_spans(&self, state: &mut ProcessEnvelopeState) { + use crate::metrics_extraction::generic::extract_metrics; + let span_metrics_extraction_config = match state.project_state.config.metric_extraction { ErrorBoundary::Ok(ref config) if config.is_enabled() => Some(config), _ => None, @@ -2294,7 +2296,8 @@ impl EnvelopeProcessorService { }; if let Some(config) = span_metrics_extraction_config { if let Some(span_value) = validated_span.value() { - crate::metrics_extraction::generic::extract_metrics(span_value, config); + let metrics = extract_metrics(span_value, config); + state.extracted_metrics.project_metrics.extend(metrics); } } if let Ok(payload) = validated_span.to_json() { diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 3f21b2ab87..215c6a4e94 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1311,7 +1311,16 @@ def test_span_ingestion( spans_consumer = spans_consumer() metrics_consumer = metrics_consumer() - relay = relay_with_processing() + relay = relay_with_processing( + options={ + "aggregator": { + "bucket_interval": 1, + "initial_delay": 0, + "debounce_delay": 0, + "max_secs_in_past": 2**64 - 1, + } + } + ) project_id = 42 project_config = mini_sentry.add_full_project_config(project_id) project_config["config"]["features"] = [ @@ -1349,7 +1358,7 @@ def test_span_ingestion( "op": "resource.script", "span_id": "bd429c44b67a3eb1", "segment_id": "968cff94913ebb07", - "sentry_tags": { + "sentry_tags": { # TODO: sentry tags are set by relay "category": "resource", "description": "https://example.com/blah.js", "group": "37e3d9fab1ae9162", @@ -1376,7 +1385,7 @@ def test_span_ingestion( "spanId": "e342abb1214ca182", "name": "my 2nd OTel span", "startTimeUnixNano": 1697620454981000000, - "endTimeUnixNano": 1697620454981078800, + "endTimeUnixNano": 1697620454981079900, }, ) @@ -1439,40 +1448,99 @@ def test_span_ingestion( "span": { "data": {}, "description": "my 2nd OTel span", - "exclusive_time": 0.0788, + "exclusive_time": 0.0799, "is_segment": True, "parent_span_id": "", "segment_id": "e342abb1214ca182", "span_id": "e342abb1214ca182", "start_timestamp": 1697620454.981, "status": "ok", - "timestamp": 1697620454.981079, + "timestamp": 1697620454.98108, "trace_id": "89143b0763095bd9c9955e8175d1fb24", }, }, ] - metrics = list(metrics_consumer.get_metrics()) + metrics = [metric for (metric, _headers) in metrics_consumer.get_metrics()] + metrics.sort(key=lambda m: (m["name"], sorted(m["tags"].items()))) + for metric in metrics: + try: + metric["value"].sort() + except AttributeError: + pass assert metrics == [ { - "name": "d:spans/exclusive_time@millisecond", "org_id": 1, + "project_id": 42, + "name": "c:spans/count_per_op@none", + "type": "c", + "value": 2.0, + "timestamp": 1697620454, + "tags": {}, "retention_days": 90, + }, + { + "org_id": 1, + "project_id": 42, + "name": "c:spans/count_per_op@none", + "type": "c", + "value": 1.0, + "timestamp": 1597976302, + "tags": {"span.category": "resource", "span.op": "resource.script"}, + "retention_days": 90, + }, + { + "org_id": 1, "project_id": 42, + "name": "d:spans/exclusive_time@millisecond", + "type": "d", + "value": [2.0], + "timestamp": 1597976302, "tags": { + "span.category": "resource", + "span.description": "https://example.com/blah.js", + "span.group": "37e3d9fab1ae9162", + "span.op": "resource.script", "transaction": "hi", + "transaction.op": "hi", }, - "type": "d", - "value": [2.0], + "retention_days": 90, }, { - "name": "d:spans/exclusive_time_light@millisecond", "org_id": 1, + "project_id": 42, + "name": "d:spans/exclusive_time@millisecond", + "type": "d", + "value": [0.0788, 0.0799], + "timestamp": 1697620454, + "tags": {"span.status": "ok"}, "retention_days": 90, + }, + { + "org_id": 1, "project_id": 42, - "tags": {}, + "name": "d:spans/exclusive_time_light@millisecond", "type": "d", "value": [2.0], + "timestamp": 1597976302, + "tags": { + "span.category": "resource", + "span.description": "https://example.com/blah.js", + "span.group": "37e3d9fab1ae9162", + "span.op": "resource.script", + "transaction.op": "hi", + }, + "retention_days": 90, + }, + { + "org_id": 1, + "project_id": 42, + "name": "d:spans/exclusive_time_light@millisecond", + "type": "d", + "value": [0.0788, 0.0799], + "timestamp": 1697620454, + "tags": {"span.status": "ok"}, + "retention_days": 90, }, ] From 98770323a7feea96a6c399052837da7f0909828f Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Tue, 28 Nov 2023 11:27:41 -0500 Subject: [PATCH 46/59] Read exclusive_time from an attribute --- relay-spans/src/otel_to_sentry_tags.rs | 37 +++++++++++++------------- relay-spans/src/span.rs | 16 ++++++++--- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/relay-spans/src/otel_to_sentry_tags.rs b/relay-spans/src/otel_to_sentry_tags.rs index 45fbd4b157..cbd816b5de 100644 --- a/relay-spans/src/otel_to_sentry_tags.rs +++ b/relay-spans/src/otel_to_sentry_tags.rs @@ -4,32 +4,33 @@ use once_cell::sync::Lazy; pub static OTEL_TO_SENTRY_TAGS: Lazy> = Lazy::new(|| { BTreeMap::from([ - ("sentry.release", "release"), + ("enduser.id", "user.id"), + ("http.request.cookies", "request.cookies"), + ("http.request.env", "request.env"), + ( + "http.request.headers.content-type", + "request.headers.content-type", + ), + ("http.request.method", "request.method"), ("sentry.environment", "environment"), - ("sentry.origin", "origin"), + ("sentry.exclusive_time_ns", "exclusive_time_ns"), ("sentry.op", "op"), - ("sentry.source", "source"), + ("sentry.origin", "origin"), + ("sentry.release", "release"), ("sentry.sample_rate", "sample_rate"), - ("enduser.id", "user.id"), - ("sentry.user.username", "user.username"), + ("sentry.sdk.integrations", "sdk.integrations"), + ("sentry.sdk.name", "sdk.name"), + ("sentry.sdk.packages", "sdk.packages"), + ("sentry.sdk.version", "sdk.version"), + ("sentry.source", "source"), ("sentry.user.email", "user.email"), - ("sentry.user.ip_address", "user.ip_address"), - ("sentry.user.segment", "user.segment"), ("sentry.user.geo.city", "user.geo.city"), ("sentry.user.geo.country_code", "user.geo.country_code"), ("sentry.user.geo.region", "user.geo.region"), - ("http.request.method", "request.method"), + ("sentry.user.ip_address", "user.ip_address"), + ("sentry.user.segment", "user.segment"), + ("sentry.user.username", "user.username"), ("url.full", "request.url"), ("url.query_string", "request.query_string"), - ("http.request.cookies", "request.cookies"), - ( - "http.request.headers.content-type", - "request.headers.content-type", - ), - ("http.request.env", "request.env"), - ("sentry.sdk.name", "sdk.name"), - ("sentry.sdk.version", "sdk.version"), - ("sentry.sdk.integrations", "sdk.integrations"), - ("sentry.sdk.packages", "sdk.packages"), ]) }); diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 136fdc17c9..e20a7d7c13 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -153,16 +153,26 @@ impl OtelSpan { impl From for EventSpan { fn from(from: OtelSpan) -> Self { - let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); - let exclusive_time = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; + let mut exclusive_time_ms = 0f64; let mut attributes: Object = Object::new(); let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); + let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); for attribute in from.attributes.clone() { let key: String = if let Some(key) = OTEL_TO_SENTRY_TAGS.get(attribute.key.as_str()) { key.to_string() } else { attribute.key }; + if key == "exclusive_time_ns" { + let value = match attribute.value { + AnyValue::Int(v) => v as f64, + AnyValue::Double(v) => v, + AnyValue::String(v) => v.parse::().unwrap_or_default(), + _ => 0f64, + }; + exclusive_time_ms = value / 1e6f64; + continue; + } match attribute.value { AnyValue::Array(_) => {} AnyValue::Bool(v) => { @@ -188,7 +198,7 @@ impl From for EventSpan { let mut span = EventSpan { data: attributes.into(), description: from.name.clone().into(), - exclusive_time: exclusive_time.into(), + exclusive_time: exclusive_time_ms.into(), is_segment: from.parent_span_id.is_empty().into(), parent_span_id: SpanId(from.parent_span_id.clone()).into(), received: Timestamp(Utc::now()).into(), From 07e4a673a4f9867edc50841105c3b585cf0e22a3 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Tue, 28 Nov 2023 12:45:28 -0500 Subject: [PATCH 47/59] Refacor span processing into its own file --- relay-server/src/actors/processor.rs | 265 +------------------- relay-server/src/actors/processor/event.rs | 1 + relay-server/src/actors/processor/span.rs | 266 +++++++++++++++++++++ 3 files changed, 272 insertions(+), 260 deletions(-) create mode 100644 relay-server/src/actors/processor/span.rs diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index fe4c5682a2..2f9778bc52 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -44,8 +44,7 @@ use tokio::sync::Semaphore; use { crate::actors::project_cache::UpdateRateLimits, crate::utils::{EnvelopeLimiter, ItemAction, MetricsLimiter}, - relay_event_normalization::{span, StoreConfig, StoreProcessor}, - relay_event_schema::protocol::Span, + relay_event_normalization::{StoreConfig, StoreProcessor}, relay_metrics::{Aggregator, RedisMetricMetaStore}, relay_quotas::{RateLimitingError, RedisRateLimiter}, relay_redis::RedisPool, @@ -73,6 +72,7 @@ mod profile; mod replay; mod report; mod session; +mod span; /// The minimum clock drift for correction to apply. const MINIMUM_CLOCK_DRIFT: Duration = Duration::from_secs(55 * 60); @@ -1123,260 +1123,6 @@ impl EnvelopeProcessorService { Ok(()) } - #[cfg(feature = "processing")] - fn is_span_allowed(&self, span: &Span) -> bool { - let Some(op) = span.op.value() else { - return false; - }; - let Some(description) = span.description.value() else { - return false; - }; - let system: &str = span - .data - .value() - .and_then(|v| v.get("span.system")) - .and_then(|system| system.as_str()) - .unwrap_or_default(); - op.contains("resource.script") - || op.contains("resource.css") - || op == "http.client" - || op.starts_with("app.") - || op.starts_with("ui.load") - || op.starts_with("file") - || op.starts_with("db") - && !(op.contains("clickhouse") - || op.contains("mongodb") - || op.contains("redis") - || op.contains("compiler")) - && !(op == "db.sql.query" && (description.contains("\"$") || system == "mongodb")) - } - - #[cfg(feature = "processing")] - fn extract_spans(&self, state: &mut ProcessEnvelopeState) { - // Only extract spans from transactions (not errors). - if state.event_type() != Some(EventType::Transaction) { - return; - }; - - // Check feature flag. - if !state - .project_state - .has_feature(Feature::SpanMetricsExtraction) - { - return; - }; - - let mut add_span = |span: Annotated| { - let span = match self.validate_span(span) { - Ok(span) => span, - Err(e) => { - relay_log::error!("Invalid span: {e}"); - return; - } - }; - let span = match span.to_json() { - Ok(span) => span, - Err(e) => { - relay_log::error!(error = &e as &dyn Error, "Failed to serialize span"); - return; - } - }; - let mut item = Item::new(ItemType::Span); - item.set_payload(ContentType::Json, span); - state.managed_envelope.envelope_mut().add_item(item); - }; - - let Some(event) = state.event.value() else { - return; - }; - - // Extract transaction as a span. - let mut transaction_span: Span = event.into(); - - let all_modules_enabled = state - .project_state - .has_feature(Feature::SpanMetricsExtractionAllModules); - - // Add child spans as envelope items. - if let Some(child_spans) = event.spans.value() { - for span in child_spans { - let Some(inner_span) = span.value() else { - continue; - }; - // HACK: filter spans based on module until we figure out grouping. - if !all_modules_enabled && !self.is_span_allowed(inner_span) { - continue; - } - // HACK: clone the span to set the segment_id. This should happen - // as part of normalization once standalone spans reach wider adoption. - let mut new_span = inner_span.clone(); - new_span.is_segment = Annotated::new(false); - new_span.received = transaction_span.received.clone(); - new_span.segment_id = transaction_span.segment_id.clone(); - - // If a profile is associated with the transaction, also associate it with its - // child spans. - new_span.profile_id = transaction_span.profile_id.clone(); - - add_span(Annotated::new(new_span)); - } - } - - // Extract tags to add to this span as well - let shared_tags = span::tag_extraction::extract_shared_tags(event); - transaction_span.sentry_tags = Annotated::new( - shared_tags - .clone() - .into_iter() - .map(|(k, v)| (k.sentry_tag_key().to_owned(), Annotated::new(v))) - .collect(), - ); - add_span(transaction_span.into()); - } - - /// We do not extract spans with missing fields if those fields are required on the Kafka topic. - #[cfg(feature = "processing")] - fn validate_span(&self, mut span: Annotated) -> Result, anyhow::Error> { - let inner = span - .value_mut() - .as_mut() - .ok_or(anyhow::anyhow!("empty span"))?; - let Span { - ref exclusive_time, - ref mut tags, - ref mut sentry_tags, - ref mut start_timestamp, - ref mut timestamp, - ref mut span_id, - ref mut trace_id, - .. - } = inner; - - trace_id - .value() - .ok_or(anyhow::anyhow!("span is missing trace_id"))?; - span_id - .value() - .ok_or(anyhow::anyhow!("span is missing span_id"))?; - - match (start_timestamp.value(), timestamp.value()) { - (Some(start), Some(end)) => { - if end < start { - return Err(anyhow::anyhow!( - "end timestamp is smaller than start timestamp" - )); - } - } - (_, None) => { - return Err(anyhow::anyhow!("timestamp hard-required for spans")); - } - (None, _) => { - return Err(anyhow::anyhow!("start_timestamp hard-required for spans")); - } - } - - // `is_segment` is set by `extract_span`. - exclusive_time - .value() - .ok_or(anyhow::anyhow!("missing exclusive_time"))?; - - if let Some(sentry_tags) = sentry_tags.value_mut() { - sentry_tags.retain(|key, value| match value.value() { - Some(s) => { - match key.as_str() { - "group" => { - // Only allow up to 16-char hex strings in group. - s.len() <= 16 && s.chars().all(|c| c.is_ascii_hexdigit()) - } - "status_code" => s.parse::().is_ok(), - _ => true, - } - } - // Drop empty string values. - None => false, - }); - } - if let Some(tags) = tags.value_mut() { - tags.retain(|_, value| !value.value().is_empty()) - } - - Ok(span) - } - - fn filter_spans(&self, state: &mut ProcessEnvelopeState) { - let standalone_span_ingestion_enabled = state - .project_state - .has_feature(Feature::StandaloneSpanIngestion); - state.managed_envelope.retain_items(|item| match item.ty() { - ItemType::OtelSpan | ItemType::Span => { - if !standalone_span_ingestion_enabled { - relay_log::warn!("dropping span because feature is disabled"); - ItemAction::DropSilently - } else { - ItemAction::Keep - } - } - _ => ItemAction::Keep, - }); - } - - #[cfg(feature = "processing")] - fn process_spans(&self, state: &mut ProcessEnvelopeState) { - use crate::metrics_extraction::generic::extract_metrics; - - let span_metrics_extraction_config = match state.project_state.config.metric_extraction { - ErrorBoundary::Ok(ref config) if config.is_enabled() => Some(config), - _ => None, - }; - let mut items: Vec = Vec::new(); - let mut add_span = |annotated_span: Annotated| { - let validated_span = match self.validate_span(annotated_span) { - Ok(span) => span, - Err(e) => { - relay_log::error!("invalid span: {e}"); - return; - } - }; - if let Some(config) = span_metrics_extraction_config { - if let Some(span_value) = validated_span.value() { - let metrics = extract_metrics(span_value, config); - state.extracted_metrics.project_metrics.extend(metrics); - } - } - if let Ok(payload) = validated_span.to_json() { - let mut item = Item::new(ItemType::Span); - item.set_payload(ContentType::Json, payload); - items.push(item); - } - }; - let standalone_span_ingestion_enabled = state - .project_state - .has_feature(Feature::StandaloneSpanIngestion); - state.managed_envelope.retain_items(|item| match item.ty() { - ItemType::OtelSpan if !standalone_span_ingestion_enabled => ItemAction::DropSilently, - ItemType::Span if !standalone_span_ingestion_enabled => ItemAction::DropSilently, - ItemType::OtelSpan => { - match serde_json::from_slice::(&item.payload()) { - Ok(otel_span) => add_span(Annotated::new(otel_span.into())), - Err(err) => relay_log::debug!("failed to parse OTel span: {}", err), - } - ItemAction::DropSilently - } - ItemType::Span => { - match Annotated::::from_json_bytes(&item.payload()) { - Ok(event_span) => add_span(event_span), - Err(err) => relay_log::debug!("failed to parse span: {}", err), - } - ItemAction::DropSilently - } - _ => ItemAction::Keep, - }); - let envelope = state.managed_envelope.envelope_mut(); - for item in items { - envelope.add_item(item); - } - } - /// Computes the sampling decision on the incoming event fn run_dynamic_sampling(&self, state: &mut ProcessEnvelopeState) { // Running dynamic sampling involves either: @@ -1638,8 +1384,7 @@ impl EnvelopeProcessorService { ); replay::process(state, self.inner.config.clone())?; profile::filter(state); - - self.filter_spans(state); + span::filter(state); if state.creates_event() { // Some envelopes only create events in processing relays; for example, unreal events. @@ -1681,14 +1426,14 @@ impl EnvelopeProcessorService { self.enforce_quotas(state)?; profile::process(state, self.inner.config.clone()); self.process_check_ins(state); - self.process_spans(state); + span::process(state); }); if state.has_event() { self.scrub_event(state)?; self.serialize_event(state)?; if_processing!({ - self.extract_spans(state); + span::extract_from_event(state); }); } diff --git a/relay-server/src/actors/processor/event.rs b/relay-server/src/actors/processor/event.rs index 4fdf2bc137..77ecd9ced6 100644 --- a/relay-server/src/actors/processor/event.rs +++ b/relay-server/src/actors/processor/event.rs @@ -296,6 +296,7 @@ fn is_duplicate(item: &Item, processing_enabled: bool) -> bool { ItemType::ReplayRecording => false, ItemType::CheckIn => false, ItemType::Span => false, + ItemType::OtelSpan => false, // Without knowing more, `Unknown` items are allowed to be repeated ItemType::Unknown(_) => false, diff --git a/relay-server/src/actors/processor/span.rs b/relay-server/src/actors/processor/span.rs new file mode 100644 index 0000000000..39625c66f3 --- /dev/null +++ b/relay-server/src/actors/processor/span.rs @@ -0,0 +1,266 @@ +#[cfg(feature = "processing")] +use { + crate::envelope::ContentType, crate::envelope::Item, relay_dynamic_config::ErrorBoundary, + relay_event_normalization::span::tag_extraction, relay_event_schema::protocol::EventType, + relay_event_schema::protocol::Span, relay_protocol::Annotated, relay_protocol::Empty, + std::error::Error, +}; + +use crate::actors::processor::ProcessEnvelopeState; +use crate::envelope::ItemType; +use crate::utils::ItemAction; +use relay_dynamic_config::Feature; + +pub fn filter(state: &mut ProcessEnvelopeState) { + let standalone_span_ingestion_enabled = state + .project_state + .has_feature(Feature::StandaloneSpanIngestion); + state.managed_envelope.retain_items(|item| match item.ty() { + ItemType::OtelSpan | ItemType::Span => { + if !standalone_span_ingestion_enabled { + relay_log::warn!("dropping span because feature is disabled"); + ItemAction::DropSilently + } else { + ItemAction::Keep + } + } + _ => ItemAction::Keep, + }); +} + +#[cfg(feature = "processing")] +pub fn process(state: &mut ProcessEnvelopeState) { + use crate::metrics_extraction::generic::extract_metrics; + + let span_metrics_extraction_config = match state.project_state.config.metric_extraction { + ErrorBoundary::Ok(ref config) if config.is_enabled() => Some(config), + _ => None, + }; + let mut items: Vec = Vec::new(); + let mut add_span = |annotated_span: Annotated| { + let validated_span = match validate(annotated_span) { + Ok(span) => span, + Err(e) => { + relay_log::error!("invalid span: {e}"); + return; + } + }; + if let Some(config) = span_metrics_extraction_config { + if let Some(span_value) = validated_span.value() { + let metrics = extract_metrics(span_value, config); + state.extracted_metrics.project_metrics.extend(metrics); + } + } + if let Ok(payload) = validated_span.to_json() { + let mut item = Item::new(ItemType::Span); + item.set_payload(ContentType::Json, payload); + items.push(item); + } + }; + let standalone_span_ingestion_enabled = state + .project_state + .has_feature(Feature::StandaloneSpanIngestion); + state.managed_envelope.retain_items(|item| match item.ty() { + ItemType::OtelSpan if !standalone_span_ingestion_enabled => ItemAction::DropSilently, + ItemType::Span if !standalone_span_ingestion_enabled => ItemAction::DropSilently, + ItemType::OtelSpan => { + match serde_json::from_slice::(&item.payload()) { + Ok(otel_span) => add_span(Annotated::new(otel_span.into())), + Err(err) => relay_log::debug!("failed to parse OTel span: {}", err), + } + ItemAction::DropSilently + } + ItemType::Span => { + match Annotated::::from_json_bytes(&item.payload()) { + Ok(event_span) => add_span(event_span), + Err(err) => relay_log::debug!("failed to parse span: {}", err), + } + ItemAction::DropSilently + } + _ => ItemAction::Keep, + }); + let envelope = state.managed_envelope.envelope_mut(); + for item in items { + envelope.add_item(item); + } +} + +/// We do not extract spans with missing fields if those fields are required on the Kafka topic. +#[cfg(feature = "processing")] +pub fn validate(mut span: Annotated) -> Result, anyhow::Error> { + let inner = span + .value_mut() + .as_mut() + .ok_or(anyhow::anyhow!("empty span"))?; + let Span { + ref exclusive_time, + ref mut tags, + ref mut sentry_tags, + ref mut start_timestamp, + ref mut timestamp, + ref mut span_id, + ref mut trace_id, + .. + } = inner; + + trace_id + .value() + .ok_or(anyhow::anyhow!("span is missing trace_id"))?; + span_id + .value() + .ok_or(anyhow::anyhow!("span is missing span_id"))?; + + match (start_timestamp.value(), timestamp.value()) { + (Some(start), Some(end)) => { + if end < start { + return Err(anyhow::anyhow!( + "end timestamp is smaller than start timestamp" + )); + } + } + (_, None) => { + return Err(anyhow::anyhow!("timestamp hard-required for spans")); + } + (None, _) => { + return Err(anyhow::anyhow!("start_timestamp hard-required for spans")); + } + } + + // `is_segment` is set by `extract_span`. + exclusive_time + .value() + .ok_or(anyhow::anyhow!("missing exclusive_time"))?; + + if let Some(sentry_tags) = sentry_tags.value_mut() { + sentry_tags.retain(|key, value| match value.value() { + Some(s) => { + match key.as_str() { + "group" => { + // Only allow up to 16-char hex strings in group. + s.len() <= 16 && s.chars().all(|c| c.is_ascii_hexdigit()) + } + "status_code" => s.parse::().is_ok(), + _ => true, + } + } + // Drop empty string values. + None => false, + }); + } + if let Some(tags) = tags.value_mut() { + tags.retain(|_, value| !value.value().is_empty()) + } + + Ok(span) +} + +#[cfg(feature = "processing")] +fn is_allowed(span: &Span) -> bool { + let Some(op) = span.op.value() else { + return false; + }; + let Some(description) = span.description.value() else { + return false; + }; + let system: &str = span + .data + .value() + .and_then(|v| v.get("span.system")) + .and_then(|system| system.as_str()) + .unwrap_or_default(); + op.contains("resource.script") + || op.contains("resource.css") + || op == "http.client" + || op.starts_with("app.") + || op.starts_with("ui.load") + || op.starts_with("file") + || op.starts_with("db") + && !(op.contains("clickhouse") + || op.contains("mongodb") + || op.contains("redis") + || op.contains("compiler")) + && !(op == "db.sql.query" && (description.contains("\"$") || system == "mongodb")) +} + +#[cfg(feature = "processing")] +pub fn extract_from_event(state: &mut ProcessEnvelopeState) { + // Only extract spans from transactions (not errors). + if state.event_type() != Some(EventType::Transaction) { + return; + }; + + // Check feature flag. + if !state + .project_state + .has_feature(Feature::SpanMetricsExtraction) + { + return; + }; + + let mut add_span = |span: Annotated| { + let span = match validate(span) { + Ok(span) => span, + Err(e) => { + relay_log::error!("Invalid span: {e}"); + return; + } + }; + let span = match span.to_json() { + Ok(span) => span, + Err(e) => { + relay_log::error!(error = &e as &dyn Error, "Failed to serialize span"); + return; + } + }; + let mut item = Item::new(ItemType::Span); + item.set_payload(ContentType::Json, span); + state.managed_envelope.envelope_mut().add_item(item); + }; + + let Some(event) = state.event.value() else { + return; + }; + + // Extract transaction as a span. + let mut transaction_span: Span = event.into(); + + let all_modules_enabled = state + .project_state + .has_feature(Feature::SpanMetricsExtractionAllModules); + + // Add child spans as envelope items. + if let Some(child_spans) = event.spans.value() { + for span in child_spans { + let Some(inner_span) = span.value() else { + continue; + }; + // HACK: filter spans based on module until we figure out grouping. + if !all_modules_enabled && !is_allowed(inner_span) { + continue; + } + // HACK: clone the span to set the segment_id. This should happen + // as part of normalization once standalone spans reach wider adoption. + let mut new_span = inner_span.clone(); + new_span.is_segment = Annotated::new(false); + new_span.received = transaction_span.received.clone(); + new_span.segment_id = transaction_span.segment_id.clone(); + + // If a profile is associated with the transaction, also associate it with its + // child spans. + new_span.profile_id = transaction_span.profile_id.clone(); + + add_span(Annotated::new(new_span)); + } + } + + // Extract tags to add to this span as well + let shared_tags = tag_extraction::extract_shared_tags(event); + transaction_span.sentry_tags = Annotated::new( + shared_tags + .clone() + .into_iter() + .map(|(k, v)| (k.sentry_tag_key().to_owned(), Annotated::new(v))) + .collect(), + ); + add_span(transaction_span.into()); +} From 7fb5b42618c1ca8b3c4ba4410bee59235f36a8e7 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Tue, 28 Nov 2023 16:54:28 -0500 Subject: [PATCH 48/59] Add exclusive_time attributes to span fixtures --- tests/integration/test_store.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 215c6a4e94..ad362c2418 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1342,6 +1342,14 @@ def test_span_ingestion( "name": "my 1st OTel span", "startTimeUnixNano": 1697620454980000000, "endTimeUnixNano": 1697620454980078800, + "attributes": [ + { + "key": "sentry.exclusive_time_ns", + "value": { + "doubleValue": 78800, + }, + } + ], } ).encode() ), @@ -1386,6 +1394,14 @@ def test_span_ingestion( "name": "my 2nd OTel span", "startTimeUnixNano": 1697620454981000000, "endTimeUnixNano": 1697620454981079900, + "attributes": [ + { + "key": "sentry.exclusive_time_ns", + "value": { + "doubleValue": 79900, + }, + } + ], }, ) From 963c72bb0da7c9fb373ee2ebf3a371606fcd294f Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Tue, 28 Nov 2023 17:04:11 -0500 Subject: [PATCH 49/59] Set received and is_segment for both OTel and Event spans --- relay-server/src/actors/processor/span.rs | 18 ++++++++++++++---- relay-spans/src/span.rs | 2 -- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/relay-server/src/actors/processor/span.rs b/relay-server/src/actors/processor/span.rs index 39625c66f3..0121a107c5 100644 --- a/relay-server/src/actors/processor/span.rs +++ b/relay-server/src/actors/processor/span.rs @@ -1,8 +1,11 @@ #[cfg(feature = "processing")] use { - crate::envelope::ContentType, crate::envelope::Item, relay_dynamic_config::ErrorBoundary, - relay_event_normalization::span::tag_extraction, relay_event_schema::protocol::EventType, - relay_event_schema::protocol::Span, relay_protocol::Annotated, relay_protocol::Empty, + crate::envelope::{ContentType, Item}, + chrono::{TimeZone, Utc}, + relay_dynamic_config::ErrorBoundary, + relay_event_normalization::span::tag_extraction, + relay_event_schema::protocol::{EventType, Span, Timestamp}, + relay_protocol::{Annotated, Empty}, std::error::Error, }; @@ -38,19 +41,26 @@ pub fn process(state: &mut ProcessEnvelopeState) { }; let mut items: Vec = Vec::new(); let mut add_span = |annotated_span: Annotated| { - let validated_span = match validate(annotated_span) { + let mut validated_span = match validate(annotated_span) { Ok(span) => span, Err(e) => { relay_log::error!("invalid span: {e}"); return; } }; + if let Some(config) = span_metrics_extraction_config { if let Some(span_value) = validated_span.value() { let metrics = extract_metrics(span_value, config); state.extracted_metrics.project_metrics.extend(metrics); } } + + if let Some(span) = validated_span.value_mut() { + span.received = Timestamp(Utc::now()).into(); + span.is_segment = span.parent_span_id.is_empty().into(); + } + if let Ok(payload) = validated_span.to_json() { let mut item = Item::new(ItemType::Span); item.set_payload(ContentType::Json, payload); diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index e20a7d7c13..cc8c7c6432 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -199,9 +199,7 @@ impl From for EventSpan { data: attributes.into(), description: from.name.clone().into(), exclusive_time: exclusive_time_ms.into(), - is_segment: from.parent_span_id.is_empty().into(), parent_span_id: SpanId(from.parent_span_id.clone()).into(), - received: Timestamp(Utc::now()).into(), span_id: SpanId(from.span_id.clone()).into(), start_timestamp: Timestamp(start_timestamp).into(), timestamp: Timestamp(end_timestamp).into(), From 0b17b26ee16f5c8d05466e240c35056fb0ccad5c Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Tue, 28 Nov 2023 18:28:46 -0500 Subject: [PATCH 50/59] Remove unused import --- relay-server/src/actors/processor/span.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/actors/processor/span.rs b/relay-server/src/actors/processor/span.rs index 0121a107c5..2e9cd7421c 100644 --- a/relay-server/src/actors/processor/span.rs +++ b/relay-server/src/actors/processor/span.rs @@ -1,7 +1,7 @@ #[cfg(feature = "processing")] use { crate::envelope::{ContentType, Item}, - chrono::{TimeZone, Utc}, + chrono::Utc, relay_dynamic_config::ErrorBoundary, relay_event_normalization::span::tag_extraction, relay_event_schema::protocol::{EventType, Span, Timestamp}, From 2a3c80f79fa890b2cdd797b4ba4554414b63b881 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Tue, 28 Nov 2023 21:34:28 -0500 Subject: [PATCH 51/59] Fix test --- tests/integration/test_store.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index ad362c2418..f332d8db9f 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1362,7 +1362,6 @@ def test_span_ingestion( bytes=json.dumps( { "description": "https://example.com/blah.js", - "is_segment": False, "op": "resource.script", "span_id": "bd429c44b67a3eb1", "segment_id": "968cff94913ebb07", @@ -1421,7 +1420,7 @@ def test_span_ingestion( "retention_days": 90, "span": { "description": "https://example.com/blah.js", - "is_segment": False, + "is_segment": True, "op": "resource.script", "segment_id": "968cff94913ebb07", "sentry_tags": { From b4fbe2a338800afaa8a8ae2856fe7764bf38f2c2 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Wed, 29 Nov 2023 15:03:57 +0100 Subject: [PATCH 52/59] feat(spans): Normalization for span ingestion (#2777) Run full normalization before extracting metrics from a standalone span. --------- Co-authored-by: Pierre Massat --- relay-event-normalization/src/lib.rs | 4 + .../src/normalize/span/tag_extraction.rs | 4 +- relay-event-normalization/src/remove_other.rs | 1 + relay-event-normalization/src/schema.rs | 1 + relay-event-normalization/src/trimming.rs | 2 + relay-server/src/actors/processor.rs | 2 +- relay-server/src/actors/processor/span.rs | 248 ++++++++++++++---- tests/integration/test_store.py | 116 ++++---- 8 files changed, 267 insertions(+), 111 deletions(-) diff --git a/relay-event-normalization/src/lib.rs b/relay-event-normalization/src/lib.rs index 7dd90801b5..3195fa2ea3 100644 --- a/relay-event-normalization/src/lib.rs +++ b/relay-event-normalization/src/lib.rs @@ -33,7 +33,11 @@ mod trimming; pub mod replay; pub use normalize::breakdowns::*; pub use normalize::*; +pub use remove_other::RemoveOtherProcessor; +pub use schema::SchemaProcessor; +pub use timestamp::TimestampProcessor; pub use transactions::*; +pub use trimming::TrimmingProcessor; pub use user_agent::*; pub use self::clock_drift::*; diff --git a/relay-event-normalization/src/normalize/span/tag_extraction.rs b/relay-event-normalization/src/normalize/span/tag_extraction.rs index 6c62ddd3a8..c468cdc1ed 100644 --- a/relay-event-normalization/src/normalize/span/tag_extraction.rs +++ b/relay-event-normalization/src/normalize/span/tag_extraction.rs @@ -122,7 +122,7 @@ impl std::fmt::Display for RenderBlockingStatus { } /// Configuration for span tag extraction. -pub(crate) struct Config { +pub struct Config { /// The maximum allowed size of tag values in bytes. Longer values will be cropped. pub max_tag_value_size: usize, } @@ -216,7 +216,7 @@ pub fn extract_shared_tags(event: &Event) -> BTreeMap { /// [span operations](https://develop.sentry.dev/sdk/performance/span-operations/) and /// existing [span data](https://develop.sentry.dev/sdk/performance/span-data-conventions/) fields, /// and rely on Sentry conventions and heuristics. -pub(crate) fn extract_tags( +pub fn extract_tags( span: &Span, config: &Config, initial_display: Option, diff --git a/relay-event-normalization/src/remove_other.rs b/relay-event-normalization/src/remove_other.rs index d77f6fab9e..27d5bc2783 100644 --- a/relay-event-normalization/src/remove_other.rs +++ b/relay-event-normalization/src/remove_other.rs @@ -9,6 +9,7 @@ fn create_errors(other: &mut Object) { } } +/// Removes unknown, internal and deprecated fields from a payload. pub struct RemoveOtherProcessor; impl Processor for RemoveOtherProcessor { diff --git a/relay-event-normalization/src/schema.rs b/relay-event-normalization/src/schema.rs index b3b75e8dea..74725adc0e 100644 --- a/relay-event-normalization/src/schema.rs +++ b/relay-event-normalization/src/schema.rs @@ -3,6 +3,7 @@ use relay_event_schema::processor::{ }; use relay_protocol::{Array, Empty, Error, ErrorKind, Meta, Object}; +/// Validates constraints such as empty strings or arrays and invalid characters. pub struct SchemaProcessor; impl Processor for SchemaProcessor { diff --git a/relay-event-normalization/src/trimming.rs b/relay-event-normalization/src/trimming.rs index 80e47ea5d3..79507f956e 100644 --- a/relay-event-normalization/src/trimming.rs +++ b/relay-event-normalization/src/trimming.rs @@ -14,12 +14,14 @@ struct BagSizeState { size_remaining: usize, } +/// Limits data bags to a maximum size and depth. #[derive(Default)] pub struct TrimmingProcessor { bag_size_state: Vec, } impl TrimmingProcessor { + /// Creates a new trimming processor. pub fn new() -> Self { Self::default() } diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 2f9778bc52..7d37e3ffa9 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -1426,7 +1426,7 @@ impl EnvelopeProcessorService { self.enforce_quotas(state)?; profile::process(state, self.inner.config.clone()); self.process_check_ins(state); - span::process(state); + span::process(state, self.inner.config.clone()); }); if state.has_event() { diff --git a/relay-server/src/actors/processor/span.rs b/relay-server/src/actors/processor/span.rs index 2e9cd7421c..2a24bc2607 100644 --- a/relay-server/src/actors/processor/span.rs +++ b/relay-server/src/actors/processor/span.rs @@ -1,18 +1,30 @@ +//! Processor code related to standalone spans. +#[cfg(feature = "processing")] +use {std::error::Error, std::sync::Arc}; + +use relay_dynamic_config::Feature; #[cfg(feature = "processing")] use { - crate::envelope::{ContentType, Item}, - chrono::Utc, + chrono::{DateTime, Utc}, + relay_base_schema::events::EventType, + relay_config::Config, relay_dynamic_config::ErrorBoundary, + relay_dynamic_config::ProjectConfig, relay_event_normalization::span::tag_extraction, - relay_event_schema::protocol::{EventType, Span, Timestamp}, + relay_event_schema::processor::{process_value, ProcessingState}, + relay_event_schema::protocol::Span, + relay_metrics::{aggregator::AggregatorConfig, MetricNamespace, UnixTimestamp}, + relay_pii::PiiProcessor, relay_protocol::{Annotated, Empty}, - std::error::Error, }; -use crate::actors::processor::ProcessEnvelopeState; -use crate::envelope::ItemType; -use crate::utils::ItemAction; -use relay_dynamic_config::Feature; +use crate::{actors::processor::ProcessEnvelopeState, envelope::ItemType, utils::ItemAction}; +#[cfg(feature = "processing")] +use { + crate::actors::processor::ProcessingError, + crate::envelope::{ContentType, Item}, + crate::metrics_extraction::generic::extract_metrics, +}; pub fn filter(state: &mut ProcessEnvelopeState) { let standalone_span_ingestion_enabled = state @@ -31,68 +43,198 @@ pub fn filter(state: &mut ProcessEnvelopeState) { }); } +/// Config needed to normalize a standalone span. +#[cfg(feature = "processing")] +#[derive(Clone, Debug)] +pub struct NormalizeSpanConfig { + /// The time at which the event was received in this Relay. + pub received_at: DateTime, + /// Allowed time range for transactions. + pub transaction_range: std::ops::Range, + /// The maximum allowed size of tag values in bytes. Longer values will be cropped. + pub max_tag_value_size: usize, +} + +/// Normalizes a standalone span. +#[cfg(feature = "processing")] +fn normalize( + annotated_span: &mut Annotated, + config: NormalizeSpanConfig, +) -> Result<(), ProcessingError> { + use relay_event_normalization::{ + SchemaProcessor, TimestampProcessor, TransactionsProcessor, TrimmingProcessor, + }; + + let NormalizeSpanConfig { + received_at, + transaction_range, + max_tag_value_size, + } = config; + + // This follows the steps of `NormalizeProcessor::process_event`. + // Ideally, `NormalizeProcessor` would execute these steps generically, i.e. also when calling + // `process` on it. + + process_value( + annotated_span, + &mut SchemaProcessor, + ProcessingState::root(), + )?; + + process_value( + annotated_span, + &mut TimestampProcessor, + ProcessingState::root(), + )?; + + process_value( + annotated_span, + &mut TransactionsProcessor::new(Default::default(), Some(transaction_range)), + ProcessingState::root(), + )?; + + let Some(span) = annotated_span.value_mut() else { + return Err(ProcessingError::NoEventPayload); + }; + span.is_segment = Annotated::new(span.parent_span_id.is_empty()); + span.received = Annotated::new(received_at.into()); + + // Tag extraction: + let config = tag_extraction::Config { max_tag_value_size }; + let is_mobile = false; // TODO: find a way to determine is_mobile from a standalone span. + let tags = tag_extraction::extract_tags(span, &config, None, None, is_mobile); + span.sentry_tags = Annotated::new( + tags.into_iter() + .map(|(k, v)| (k.sentry_tag_key().to_owned(), Annotated::new(v))) + .collect(), + ); + + process_value( + annotated_span, + &mut TrimmingProcessor::new(), + ProcessingState::root(), + )?; + + Ok(()) +} + +#[cfg(feature = "processing")] +fn scrub( + annotated_span: &mut Annotated, + project_config: &ProjectConfig, +) -> Result<(), ProcessingError> { + if let Some(ref config) = project_config.pii_config { + let mut processor = PiiProcessor::new(config.compiled()); + process_value(annotated_span, &mut processor, ProcessingState::root())?; + } + let pii_config = project_config + .datascrubbing_settings + .pii_config() + .map_err(|e| ProcessingError::PiiConfigError(e.clone()))?; + if let Some(config) = pii_config { + let mut processor = PiiProcessor::new(config.compiled()); + process_value(annotated_span, &mut processor, ProcessingState::root())?; + } + + Ok(()) +} + #[cfg(feature = "processing")] -pub fn process(state: &mut ProcessEnvelopeState) { - use crate::metrics_extraction::generic::extract_metrics; +pub fn process(state: &mut ProcessEnvelopeState, config: Arc) { + use relay_event_normalization::RemoveOtherProcessor; let span_metrics_extraction_config = match state.project_state.config.metric_extraction { ErrorBoundary::Ok(ref config) if config.is_enabled() => Some(config), _ => None, }; - let mut items: Vec = Vec::new(); - let mut add_span = |annotated_span: Annotated| { - let mut validated_span = match validate(annotated_span) { - Ok(span) => span, - Err(e) => { - relay_log::error!("invalid span: {e}"); - return; + + let config = NormalizeSpanConfig { + received_at: state.managed_envelope.received_at(), + transaction_range: AggregatorConfig::from( + config.aggregator_config_for(MetricNamespace::Transactions), + ) + .timestamp_range(), + max_tag_value_size: config + .aggregator_config_for(MetricNamespace::Spans) + .max_tag_value_length, + }; + + state.managed_envelope.retain_items(|item| { + let mut annotated_span = match item.ty() { + ItemType::OtelSpan => { + match serde_json::from_slice::(&item.payload()) { + Ok(otel_span) => Annotated::new(otel_span.into()), + Err(err) => { + relay_log::debug!("failed to parse OTel span: {}", err); + return ItemAction::DropSilently; + } + } } + ItemType::Span => match Annotated::::from_json_bytes(&item.payload()) { + Ok(span) => span, + Err(err) => { + relay_log::debug!("failed to parse span: {}", err); + return ItemAction::DropSilently; + } + }, + + _ => return ItemAction::Keep, + }; + + if let Err(e) = normalize(&mut annotated_span, config.clone()) { + relay_log::debug!("failed to normalize span: {}", e); + return ItemAction::DropSilently; + }; + + let Some(span) = annotated_span.value_mut() else { + return ItemAction::DropSilently; }; if let Some(config) = span_metrics_extraction_config { - if let Some(span_value) = validated_span.value() { - let metrics = extract_metrics(span_value, config); - state.extracted_metrics.project_metrics.extend(metrics); - } + let metrics = extract_metrics(span, config); + state.extracted_metrics.project_metrics.extend(metrics); } - if let Some(span) = validated_span.value_mut() { - span.received = Timestamp(Utc::now()).into(); - span.is_segment = span.parent_span_id.is_empty().into(); - } + // TODO: dynamic sampling - if let Ok(payload) = validated_span.to_json() { - let mut item = Item::new(ItemType::Span); - item.set_payload(ContentType::Json, payload); - items.push(item); + if let Err(e) = scrub(&mut annotated_span, &state.project_state.config) { + relay_log::error!("failed to scrub span: {e}"); } - }; - let standalone_span_ingestion_enabled = state - .project_state - .has_feature(Feature::StandaloneSpanIngestion); - state.managed_envelope.retain_items(|item| match item.ty() { - ItemType::OtelSpan if !standalone_span_ingestion_enabled => ItemAction::DropSilently, - ItemType::Span if !standalone_span_ingestion_enabled => ItemAction::DropSilently, - ItemType::OtelSpan => { - match serde_json::from_slice::(&item.payload()) { - Ok(otel_span) => add_span(Annotated::new(otel_span.into())), - Err(err) => relay_log::debug!("failed to parse OTel span: {}", err), + + // TODO: rate limiting + + // Remove additional fields. + process_value( + &mut annotated_span, + &mut RemoveOtherProcessor, + ProcessingState::root(), + ) + .ok(); + + // Validate for kafka (TODO: this should be moved to kafka producer) + let annotated_span = match validate(annotated_span) { + Ok(res) => res, + Err(err) => { + relay_log::error!("invalid span: {err}"); + return ItemAction::DropSilently; } - ItemAction::DropSilently - } - ItemType::Span => { - match Annotated::::from_json_bytes(&item.payload()) { - Ok(event_span) => add_span(event_span), - Err(err) => relay_log::debug!("failed to parse span: {}", err), + }; + + // Write back: + let mut new_item = Item::new(ItemType::Span); + let payload = match annotated_span.to_json() { + Ok(payload) => payload, + Err(err) => { + relay_log::debug!("failed to serialize span: {}", err); + return ItemAction::DropSilently; } - ItemAction::DropSilently - } - _ => ItemAction::Keep, + }; + new_item.set_payload(ContentType::Json, payload); + + *item = new_item; + + ItemAction::Keep }); - let envelope = state.managed_envelope.envelope_mut(); - for item in items { - envelope.add_item(item); - } } /// We do not extract spans with missing fields if those fields are required on the Kafka topic. diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index f332d8db9f..d46f6e1775 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1329,6 +1329,10 @@ def test_span_ingestion( "projects:span-metrics-extraction-all-modules", ] + duration = timedelta(milliseconds=500) + end = datetime.utcnow().replace(tzinfo=timezone.utc) - timedelta(seconds=1) + start = end - duration + # 1 - Send OTel span and sentry span via envelope envelope = Envelope() envelope.add_item( @@ -1340,13 +1344,13 @@ def test_span_ingestion( "traceId": "89143b0763095bd9c9955e8175d1fb23", "spanId": "e342abb1214ca181", "name": "my 1st OTel span", - "startTimeUnixNano": 1697620454980000000, - "endTimeUnixNano": 1697620454980078800, + "startTimeUnixNano": int(start.timestamp() * 1_000_000_000), + "endTimeUnixNano": int(end.timestamp() * 1_000_000_000), "attributes": [ { "key": "sentry.exclusive_time_ns", "value": { - "doubleValue": 78800, + "intValue": int(duration.total_seconds() * 1e9), }, } ], @@ -1361,21 +1365,13 @@ def test_span_ingestion( payload=PayloadRef( bytes=json.dumps( { - "description": "https://example.com/blah.js", + "description": "https://example.com/p/blah.js", "op": "resource.script", "span_id": "bd429c44b67a3eb1", "segment_id": "968cff94913ebb07", - "sentry_tags": { # TODO: sentry tags are set by relay - "category": "resource", - "description": "https://example.com/blah.js", - "group": "37e3d9fab1ae9162", - "op": "resource.script", - "transaction": "hi", - "transaction.op": "hi", - }, - "start_timestamp": 1597976300.0000000, - "timestamp": 1597976302.0000000, - "exclusive_time": 2.0, + "start_timestamp": start.timestamp(), + "timestamp": end.timestamp() + 1, + "exclusive_time": 345.0, # The SDK knows that this span has a lower exclusive time "trace_id": "ff62a8b040f340bda5d830223def1d81", }, ).encode() @@ -1391,13 +1387,13 @@ def test_span_ingestion( "traceId": "89143b0763095bd9c9955e8175d1fb24", "spanId": "e342abb1214ca182", "name": "my 2nd OTel span", - "startTimeUnixNano": 1697620454981000000, - "endTimeUnixNano": 1697620454981079900, + "startTimeUnixNano": int(start.timestamp() * 1_000_000_000) + 2, + "endTimeUnixNano": int(end.timestamp() * 1_000_000_000) + 3, "attributes": [ { "key": "sentry.exclusive_time_ns", "value": { - "doubleValue": 79900, + "intValue": int((duration.total_seconds() + 1) * 1e9), }, } ], @@ -1419,22 +1415,22 @@ def test_span_ingestion( "project_id": 42, "retention_days": 90, "span": { - "description": "https://example.com/blah.js", + "description": "https://example.com/p/blah.js", "is_segment": True, "op": "resource.script", "segment_id": "968cff94913ebb07", "sentry_tags": { "category": "resource", - "description": "https://example.com/blah.js", - "group": "37e3d9fab1ae9162", + "description": "https://example.com/*/blah.js", + "domain": "example.com", + "file_extension": "js", + "group": "8a97a9e43588e2bd", "op": "resource.script", - "transaction": "hi", - "transaction.op": "hi", }, "span_id": "bd429c44b67a3eb1", - "start_timestamp": 1597976300.0, - "timestamp": 1597976302.0, - "exclusive_time": 2.0, + "start_timestamp": start.timestamp(), + "timestamp": end.timestamp() + 1, + "exclusive_time": 345.0, "trace_id": "ff62a8b040f340bda5d830223def1d81", }, }, @@ -1445,14 +1441,16 @@ def test_span_ingestion( "span": { "data": {}, "description": "my 1st OTel span", - "exclusive_time": 0.0788, + "exclusive_time": 500.0, "is_segment": True, + "op": "default", "parent_span_id": "", "segment_id": "e342abb1214ca181", + "sentry_tags": {"op": "default"}, "span_id": "e342abb1214ca181", - "start_timestamp": 1697620454.98, + "start_timestamp": start.timestamp(), "status": "ok", - "timestamp": 1697620454.980079, + "timestamp": end.timestamp(), "trace_id": "89143b0763095bd9c9955e8175d1fb23", }, }, @@ -1463,14 +1461,16 @@ def test_span_ingestion( "span": { "data": {}, "description": "my 2nd OTel span", - "exclusive_time": 0.0799, + "exclusive_time": 1500.0, "is_segment": True, + "op": "default", "parent_span_id": "", "segment_id": "e342abb1214ca182", + "sentry_tags": {"op": "default"}, "span_id": "e342abb1214ca182", - "start_timestamp": 1697620454.981, + "start_timestamp": start.timestamp(), "status": "ok", - "timestamp": 1697620454.98108, + "timestamp": end.timestamp(), "trace_id": "89143b0763095bd9c9955e8175d1fb24", }, }, @@ -1484,15 +1484,17 @@ def test_span_ingestion( except AttributeError: pass + expected_timestamp = int(end.timestamp()) + assert metrics == [ { "org_id": 1, "project_id": 42, "name": "c:spans/count_per_op@none", "type": "c", - "value": 2.0, - "timestamp": 1697620454, - "tags": {}, + "value": 1.0, + "timestamp": expected_timestamp + 1, + "tags": {"span.category": "resource", "span.op": "resource.script"}, "retention_days": 90, }, { @@ -1500,9 +1502,9 @@ def test_span_ingestion( "project_id": 42, "name": "c:spans/count_per_op@none", "type": "c", - "value": 1.0, - "timestamp": 1597976302, - "tags": {"span.category": "resource", "span.op": "resource.script"}, + "value": 2.0, + "timestamp": expected_timestamp, + "tags": {"span.op": "default"}, "retention_days": 90, }, { @@ -1510,15 +1512,15 @@ def test_span_ingestion( "project_id": 42, "name": "d:spans/exclusive_time@millisecond", "type": "d", - "value": [2.0], - "timestamp": 1597976302, + "value": [345.0], + "timestamp": expected_timestamp + 1, "tags": { + "file_extension": "js", "span.category": "resource", - "span.description": "https://example.com/blah.js", - "span.group": "37e3d9fab1ae9162", + "span.description": "https://example.com/*/blah.js", + "span.domain": "example.com", + "span.group": "8a97a9e43588e2bd", "span.op": "resource.script", - "transaction": "hi", - "transaction.op": "hi", }, "retention_days": 90, }, @@ -1527,9 +1529,9 @@ def test_span_ingestion( "project_id": 42, "name": "d:spans/exclusive_time@millisecond", "type": "d", - "value": [0.0788, 0.0799], - "timestamp": 1697620454, - "tags": {"span.status": "ok"}, + "value": [500.0, 1500], + "timestamp": expected_timestamp, + "tags": {"span.status": "ok", "span.op": "default"}, "retention_days": 90, }, { @@ -1537,14 +1539,15 @@ def test_span_ingestion( "project_id": 42, "name": "d:spans/exclusive_time_light@millisecond", "type": "d", - "value": [2.0], - "timestamp": 1597976302, + "value": [345.0], + "timestamp": expected_timestamp + 1, "tags": { + "file_extension": "js", "span.category": "resource", - "span.description": "https://example.com/blah.js", - "span.group": "37e3d9fab1ae9162", + "span.description": "https://example.com/*/blah.js", + "span.domain": "example.com", + "span.group": "8a97a9e43588e2bd", "span.op": "resource.script", - "transaction.op": "hi", }, "retention_days": 90, }, @@ -1553,9 +1556,12 @@ def test_span_ingestion( "project_id": 42, "name": "d:spans/exclusive_time_light@millisecond", "type": "d", - "value": [0.0788, 0.0799], - "timestamp": 1697620454, - "tags": {"span.status": "ok"}, + "value": [500.0, 1500], + "timestamp": expected_timestamp, + "tags": { + "span.op": "default", + "span.status": "ok", + }, "retention_days": 90, }, ] From 3169524d618e97e7dcbd17f3fbe194863c1a46ee Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Wed, 29 Nov 2023 13:34:00 -0500 Subject: [PATCH 53/59] Update CHANGELOG.md Co-authored-by: Joris Bayer --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b09dfc8fe4..6b10d0f850 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ **Features**: -- Ingest OpenTelemetry spans via HTTP or an envelope. ([#2620](https://github.com/getsentry/relay/pull/2620)) +- Ingest OpenTelemetry and standalone Sentry spans via HTTP or an envelope. ([#2620](https://github.com/getsentry/relay/pull/2620)) - Partition and split metric buckets just before sending. Log outcomes for metrics. ([#2682](https://github.com/getsentry/relay/pull/2682)) **Internal**: From 1d5f2ebd210d4a10137e93a989fbd8e47c39da5f Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Wed, 29 Nov 2023 13:34:36 -0500 Subject: [PATCH 54/59] Make NormalizeSpanConfig private Co-authored-by: Joris Bayer --- relay-server/src/actors/processor/span.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/actors/processor/span.rs b/relay-server/src/actors/processor/span.rs index 2a24bc2607..e6f548d6f3 100644 --- a/relay-server/src/actors/processor/span.rs +++ b/relay-server/src/actors/processor/span.rs @@ -46,7 +46,7 @@ pub fn filter(state: &mut ProcessEnvelopeState) { /// Config needed to normalize a standalone span. #[cfg(feature = "processing")] #[derive(Clone, Debug)] -pub struct NormalizeSpanConfig { +struct NormalizeSpanConfig { /// The time at which the event was received in this Relay. pub received_at: DateTime, /// Allowed time range for transactions. From c3566cf9f80f210c3ec769e8c6a3b3ce15b9f565 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Wed, 29 Nov 2023 13:37:59 -0500 Subject: [PATCH 55/59] Move all public functions to the top of the file --- relay-server/src/actors/processor/span.rs | 386 +++++++++++----------- 1 file changed, 193 insertions(+), 193 deletions(-) diff --git a/relay-server/src/actors/processor/span.rs b/relay-server/src/actors/processor/span.rs index e6f548d6f3..d7d54c9958 100644 --- a/relay-server/src/actors/processor/span.rs +++ b/relay-server/src/actors/processor/span.rs @@ -43,102 +43,6 @@ pub fn filter(state: &mut ProcessEnvelopeState) { }); } -/// Config needed to normalize a standalone span. -#[cfg(feature = "processing")] -#[derive(Clone, Debug)] -struct NormalizeSpanConfig { - /// The time at which the event was received in this Relay. - pub received_at: DateTime, - /// Allowed time range for transactions. - pub transaction_range: std::ops::Range, - /// The maximum allowed size of tag values in bytes. Longer values will be cropped. - pub max_tag_value_size: usize, -} - -/// Normalizes a standalone span. -#[cfg(feature = "processing")] -fn normalize( - annotated_span: &mut Annotated, - config: NormalizeSpanConfig, -) -> Result<(), ProcessingError> { - use relay_event_normalization::{ - SchemaProcessor, TimestampProcessor, TransactionsProcessor, TrimmingProcessor, - }; - - let NormalizeSpanConfig { - received_at, - transaction_range, - max_tag_value_size, - } = config; - - // This follows the steps of `NormalizeProcessor::process_event`. - // Ideally, `NormalizeProcessor` would execute these steps generically, i.e. also when calling - // `process` on it. - - process_value( - annotated_span, - &mut SchemaProcessor, - ProcessingState::root(), - )?; - - process_value( - annotated_span, - &mut TimestampProcessor, - ProcessingState::root(), - )?; - - process_value( - annotated_span, - &mut TransactionsProcessor::new(Default::default(), Some(transaction_range)), - ProcessingState::root(), - )?; - - let Some(span) = annotated_span.value_mut() else { - return Err(ProcessingError::NoEventPayload); - }; - span.is_segment = Annotated::new(span.parent_span_id.is_empty()); - span.received = Annotated::new(received_at.into()); - - // Tag extraction: - let config = tag_extraction::Config { max_tag_value_size }; - let is_mobile = false; // TODO: find a way to determine is_mobile from a standalone span. - let tags = tag_extraction::extract_tags(span, &config, None, None, is_mobile); - span.sentry_tags = Annotated::new( - tags.into_iter() - .map(|(k, v)| (k.sentry_tag_key().to_owned(), Annotated::new(v))) - .collect(), - ); - - process_value( - annotated_span, - &mut TrimmingProcessor::new(), - ProcessingState::root(), - )?; - - Ok(()) -} - -#[cfg(feature = "processing")] -fn scrub( - annotated_span: &mut Annotated, - project_config: &ProjectConfig, -) -> Result<(), ProcessingError> { - if let Some(ref config) = project_config.pii_config { - let mut processor = PiiProcessor::new(config.compiled()); - process_value(annotated_span, &mut processor, ProcessingState::root())?; - } - let pii_config = project_config - .datascrubbing_settings - .pii_config() - .map_err(|e| ProcessingError::PiiConfigError(e.clone()))?; - if let Some(config) = pii_config { - let mut processor = PiiProcessor::new(config.compiled()); - process_value(annotated_span, &mut processor, ProcessingState::root())?; - } - - Ok(()) -} - #[cfg(feature = "processing")] pub fn process(state: &mut ProcessEnvelopeState, config: Arc) { use relay_event_normalization::RemoveOtherProcessor; @@ -237,103 +141,6 @@ pub fn process(state: &mut ProcessEnvelopeState, config: Arc) { }); } -/// We do not extract spans with missing fields if those fields are required on the Kafka topic. -#[cfg(feature = "processing")] -pub fn validate(mut span: Annotated) -> Result, anyhow::Error> { - let inner = span - .value_mut() - .as_mut() - .ok_or(anyhow::anyhow!("empty span"))?; - let Span { - ref exclusive_time, - ref mut tags, - ref mut sentry_tags, - ref mut start_timestamp, - ref mut timestamp, - ref mut span_id, - ref mut trace_id, - .. - } = inner; - - trace_id - .value() - .ok_or(anyhow::anyhow!("span is missing trace_id"))?; - span_id - .value() - .ok_or(anyhow::anyhow!("span is missing span_id"))?; - - match (start_timestamp.value(), timestamp.value()) { - (Some(start), Some(end)) => { - if end < start { - return Err(anyhow::anyhow!( - "end timestamp is smaller than start timestamp" - )); - } - } - (_, None) => { - return Err(anyhow::anyhow!("timestamp hard-required for spans")); - } - (None, _) => { - return Err(anyhow::anyhow!("start_timestamp hard-required for spans")); - } - } - - // `is_segment` is set by `extract_span`. - exclusive_time - .value() - .ok_or(anyhow::anyhow!("missing exclusive_time"))?; - - if let Some(sentry_tags) = sentry_tags.value_mut() { - sentry_tags.retain(|key, value| match value.value() { - Some(s) => { - match key.as_str() { - "group" => { - // Only allow up to 16-char hex strings in group. - s.len() <= 16 && s.chars().all(|c| c.is_ascii_hexdigit()) - } - "status_code" => s.parse::().is_ok(), - _ => true, - } - } - // Drop empty string values. - None => false, - }); - } - if let Some(tags) = tags.value_mut() { - tags.retain(|_, value| !value.value().is_empty()) - } - - Ok(span) -} - -#[cfg(feature = "processing")] -fn is_allowed(span: &Span) -> bool { - let Some(op) = span.op.value() else { - return false; - }; - let Some(description) = span.description.value() else { - return false; - }; - let system: &str = span - .data - .value() - .and_then(|v| v.get("span.system")) - .and_then(|system| system.as_str()) - .unwrap_or_default(); - op.contains("resource.script") - || op.contains("resource.css") - || op == "http.client" - || op.starts_with("app.") - || op.starts_with("ui.load") - || op.starts_with("file") - || op.starts_with("db") - && !(op.contains("clickhouse") - || op.contains("mongodb") - || op.contains("redis") - || op.contains("compiler")) - && !(op == "db.sql.query" && (description.contains("\"$") || system == "mongodb")) -} - #[cfg(feature = "processing")] pub fn extract_from_event(state: &mut ProcessEnvelopeState) { // Only extract spans from transactions (not errors). @@ -416,3 +223,196 @@ pub fn extract_from_event(state: &mut ProcessEnvelopeState) { ); add_span(transaction_span.into()); } + +/// Config needed to normalize a standalone span. +#[cfg(feature = "processing")] +#[derive(Clone, Debug)] +struct NormalizeSpanConfig { + /// The time at which the event was received in this Relay. + pub received_at: DateTime, + /// Allowed time range for transactions. + pub transaction_range: std::ops::Range, + /// The maximum allowed size of tag values in bytes. Longer values will be cropped. + pub max_tag_value_size: usize, +} + +/// Normalizes a standalone span. +#[cfg(feature = "processing")] +fn normalize( + annotated_span: &mut Annotated, + config: NormalizeSpanConfig, +) -> Result<(), ProcessingError> { + use relay_event_normalization::{ + SchemaProcessor, TimestampProcessor, TransactionsProcessor, TrimmingProcessor, + }; + + let NormalizeSpanConfig { + received_at, + transaction_range, + max_tag_value_size, + } = config; + + // This follows the steps of `NormalizeProcessor::process_event`. + // Ideally, `NormalizeProcessor` would execute these steps generically, i.e. also when calling + // `process` on it. + + process_value( + annotated_span, + &mut SchemaProcessor, + ProcessingState::root(), + )?; + + process_value( + annotated_span, + &mut TimestampProcessor, + ProcessingState::root(), + )?; + + process_value( + annotated_span, + &mut TransactionsProcessor::new(Default::default(), Some(transaction_range)), + ProcessingState::root(), + )?; + + let Some(span) = annotated_span.value_mut() else { + return Err(ProcessingError::NoEventPayload); + }; + span.is_segment = Annotated::new(span.parent_span_id.is_empty()); + span.received = Annotated::new(received_at.into()); + + // Tag extraction: + let config = tag_extraction::Config { max_tag_value_size }; + let is_mobile = false; // TODO: find a way to determine is_mobile from a standalone span. + let tags = tag_extraction::extract_tags(span, &config, None, None, is_mobile); + span.sentry_tags = Annotated::new( + tags.into_iter() + .map(|(k, v)| (k.sentry_tag_key().to_owned(), Annotated::new(v))) + .collect(), + ); + + process_value( + annotated_span, + &mut TrimmingProcessor::new(), + ProcessingState::root(), + )?; + + Ok(()) +} + +#[cfg(feature = "processing")] +fn scrub( + annotated_span: &mut Annotated, + project_config: &ProjectConfig, +) -> Result<(), ProcessingError> { + if let Some(ref config) = project_config.pii_config { + let mut processor = PiiProcessor::new(config.compiled()); + process_value(annotated_span, &mut processor, ProcessingState::root())?; + } + let pii_config = project_config + .datascrubbing_settings + .pii_config() + .map_err(|e| ProcessingError::PiiConfigError(e.clone()))?; + if let Some(config) = pii_config { + let mut processor = PiiProcessor::new(config.compiled()); + process_value(annotated_span, &mut processor, ProcessingState::root())?; + } + + Ok(()) +} + +#[cfg(feature = "processing")] +fn is_allowed(span: &Span) -> bool { + let Some(op) = span.op.value() else { + return false; + }; + let Some(description) = span.description.value() else { + return false; + }; + let system: &str = span + .data + .value() + .and_then(|v| v.get("span.system")) + .and_then(|system| system.as_str()) + .unwrap_or_default(); + op.contains("resource.script") + || op.contains("resource.css") + || op == "http.client" + || op.starts_with("app.") + || op.starts_with("ui.load") + || op.starts_with("file") + || op.starts_with("db") + && !(op.contains("clickhouse") + || op.contains("mongodb") + || op.contains("redis") + || op.contains("compiler")) + && !(op == "db.sql.query" && (description.contains("\"$") || system == "mongodb")) +} + +/// We do not extract or ingest spans with missing fields if those fields are required on the Kafka topic. +#[cfg(feature = "processing")] +fn validate(mut span: Annotated) -> Result, anyhow::Error> { + let inner = span + .value_mut() + .as_mut() + .ok_or(anyhow::anyhow!("empty span"))?; + let Span { + ref exclusive_time, + ref mut tags, + ref mut sentry_tags, + ref mut start_timestamp, + ref mut timestamp, + ref mut span_id, + ref mut trace_id, + .. + } = inner; + + trace_id + .value() + .ok_or(anyhow::anyhow!("span is missing trace_id"))?; + span_id + .value() + .ok_or(anyhow::anyhow!("span is missing span_id"))?; + + match (start_timestamp.value(), timestamp.value()) { + (Some(start), Some(end)) => { + if end < start { + return Err(anyhow::anyhow!( + "end timestamp is smaller than start timestamp" + )); + } + } + (_, None) => { + return Err(anyhow::anyhow!("timestamp hard-required for spans")); + } + (None, _) => { + return Err(anyhow::anyhow!("start_timestamp hard-required for spans")); + } + } + + // `is_segment` is set by `extract_span`. + exclusive_time + .value() + .ok_or(anyhow::anyhow!("missing exclusive_time"))?; + + if let Some(sentry_tags) = sentry_tags.value_mut() { + sentry_tags.retain(|key, value| match value.value() { + Some(s) => { + match key.as_str() { + "group" => { + // Only allow up to 16-char hex strings in group. + s.len() <= 16 && s.chars().all(|c| c.is_ascii_hexdigit()) + } + "status_code" => s.parse::().is_ok(), + _ => true, + } + } + // Drop empty string values. + None => false, + }); + } + if let Some(tags) = tags.value_mut() { + tags.retain(|_, value| !value.value().is_empty()) + } + + Ok(span) +} From 6dcaecdb546b6f54e157b84c83872663fd70eb3d Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Wed, 29 Nov 2023 14:29:42 -0500 Subject: [PATCH 56/59] Less clone, more perform --- relay-spans/src/span.rs | 115 ++++++++++++++++++++++------------------ 1 file changed, 62 insertions(+), 53 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index cc8c7c6432..084b13305f 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -112,59 +112,67 @@ pub struct OtelSpan { pub status: Status, } -impl OtelSpan { - /// sentry_status() returns a status as defined by Sentry based on the span status. - pub fn sentry_status(&self) -> &'static str { - let status_code = self.status.code.clone(); - - if status_code == StatusCode::Unset || status_code == StatusCode::Ok { - return "ok"; - } +/// convert_from_otel_to_sentry_status returns a status as defined by Sentry based on the OTel status. +fn convert_from_otel_to_sentry_status( + status_code: StatusCode, + http_status_code: Option<&KeyValue>, + grpc_status_code: Option<&KeyValue>, +) -> SpanStatus { + if status_code == StatusCode::Unset || status_code == StatusCode::Ok { + return SpanStatus::Ok; + } - if let Some(code) = self - .attributes - .clone() - .into_iter() - .find(|a| a.key == "http.status_code") - { - if let Some(code_value) = code.value.to_i64() { - if let Some(sentry_status) = status_codes::HTTP.get(&code_value) { - return sentry_status; + if let Some(status_code_value) = http_status_code { + if let Some(code_value) = status_code_value.value.to_i64() { + if let Some(sentry_status) = status_codes::HTTP.get(&code_value) { + if let Ok(span_status) = SpanStatus::from_str(sentry_status) { + return span_status; } } } + } - if let Some(code) = self - .attributes - .clone() - .into_iter() - .find(|a| a.key == "rpc.grpc.status_code") - { - if let Some(code_value) = code.value.to_i64() { - if let Some(sentry_status) = status_codes::GRPC.get(&code_value) { - return sentry_status; + if let Some(status_code_value) = grpc_status_code { + if let Some(code_value) = status_code_value.value.to_i64() { + if let Some(sentry_status) = status_codes::GRPC.get(&code_value) { + if let Ok(span_status) = SpanStatus::from_str(sentry_status) { + return span_status; } } } - - "unknown_error" } + + SpanStatus::Unknown } impl From for EventSpan { fn from(from: OtelSpan) -> Self { let mut exclusive_time_ms = 0f64; - let mut attributes: Object = Object::new(); + let mut data: Object = Object::new(); let start_timestamp = Utc.timestamp_nanos(from.start_time_unix_nano); let end_timestamp = Utc.timestamp_nanos(from.end_time_unix_nano); - for attribute in from.attributes.clone() { + let OtelSpan { + trace_id, + span_id, + parent_span_id, + name, + attributes, + status, + .. + } = from; + let segment_id = if parent_span_id.is_empty() { + Annotated::new(SpanId(span_id.clone())) + } else { + Annotated::empty() + }; + for attribute in attributes.iter() { let key: String = if let Some(key) = OTEL_TO_SENTRY_TAGS.get(attribute.key.as_str()) { key.to_string() } else { - attribute.key + attribute.key.clone() }; if key == "exclusive_time_ns" { - let value = match attribute.value { + let value = match attribute.value.clone() { AnyValue::Int(v) => v as f64, AnyValue::Double(v) => v, AnyValue::String(v) => v.parse::().unwrap_or_default(), @@ -173,46 +181,47 @@ impl From for EventSpan { exclusive_time_ms = value / 1e6f64; continue; } - match attribute.value { + match attribute.value.clone() { AnyValue::Array(_) => {} AnyValue::Bool(v) => { - attributes.insert(key, Annotated::new(v.into())); + data.insert(key, Annotated::new(v.into())); } AnyValue::Bytes(v) => { if let Ok(v) = String::from_utf8(v.clone()) { - attributes.insert(key, Annotated::new(v.into())); + data.insert(key, Annotated::new(v.into())); } } AnyValue::Double(v) => { - attributes.insert(key, Annotated::new(v.into())); + data.insert(key, Annotated::new(v.into())); } AnyValue::Int(v) => { - attributes.insert(key, Annotated::new(v.into())); + data.insert(key, Annotated::new(v.into())); } AnyValue::Kvlist(_) => {} AnyValue::String(v) => { - attributes.insert(key, Annotated::new(v.into())); + data.insert(key, Annotated::new(v.into())); } }; } - let mut span = EventSpan { - data: attributes.into(), - description: from.name.clone().into(), + let http_status_code = attributes.iter().find(|a| a.key == "http.status_code"); + let grpc_status_code = attributes.iter().find(|a| a.key == "rpc.grpc.status_code"); + EventSpan { + data: data.into(), + description: name.into(), exclusive_time: exclusive_time_ms.into(), - parent_span_id: SpanId(from.parent_span_id.clone()).into(), - span_id: SpanId(from.span_id.clone()).into(), + parent_span_id: SpanId(parent_span_id.clone()).into(), + segment_id, + span_id: Annotated::new(SpanId(span_id)), start_timestamp: Timestamp(start_timestamp).into(), + status: Annotated::new(convert_from_otel_to_sentry_status( + status.code, + http_status_code, + grpc_status_code, + )), timestamp: Timestamp(end_timestamp).into(), - trace_id: TraceId(from.trace_id.clone()).into(), + trace_id: TraceId(trace_id).into(), ..Default::default() - }; - if let Ok(status) = SpanStatus::from_str(from.sentry_status()) { - span.status = status.into(); } - if from.parent_span_id.is_empty() { - span.segment_id = span.span_id.clone(); - } - span } } @@ -296,8 +305,8 @@ pub struct Status { pub code: StatusCode, } -#[derive(Clone, Debug, Deserialize_repr, Default, PartialEq)] -#[repr(u32)] +#[derive(Clone, Copy, Debug, Deserialize_repr, Default, PartialEq)] +#[repr(u8)] pub enum StatusCode { /// The default status. #[default] From 2c5643d16adcac61a718c119eaedb7acddbfd94d Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Wed, 29 Nov 2023 16:14:08 -0500 Subject: [PATCH 57/59] Use attributes to get exclusive time or default to duration --- relay-spans/src/otel_to_sentry_tags.rs | 1 - relay-spans/src/span.rs | 58 ++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/relay-spans/src/otel_to_sentry_tags.rs b/relay-spans/src/otel_to_sentry_tags.rs index cbd816b5de..69a365f510 100644 --- a/relay-spans/src/otel_to_sentry_tags.rs +++ b/relay-spans/src/otel_to_sentry_tags.rs @@ -13,7 +13,6 @@ pub static OTEL_TO_SENTRY_TAGS: Lazy> = Lazy::new(|| { ), ("http.request.method", "request.method"), ("sentry.environment", "environment"), - ("sentry.exclusive_time_ns", "exclusive_time_ns"), ("sentry.op", "op"), ("sentry.origin", "origin"), ("sentry.release", "release"), diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 084b13305f..d3fe0fbbcd 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -171,7 +171,7 @@ impl From for EventSpan { } else { attribute.key.clone() }; - if key == "exclusive_time_ns" { + if key.contains("exclusive_time_ns") { let value = match attribute.value.clone() { AnyValue::Int(v) => v as f64, AnyValue::Double(v) => v, @@ -203,6 +203,10 @@ impl From for EventSpan { } }; } + if exclusive_time_ms == 0f64 { + exclusive_time_ms = + (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; + } let http_status_code = attributes.iter().find(|a| a.key == "http.status_code"); let grpc_status_code = attributes.iter().find(|a| a.key == "rpc.grpc.status_code"); EventSpan { @@ -447,6 +451,12 @@ mod tests { "value": { "boolValue": true } + }, + { + "key": "sentry.exclusive_time_ns", + "value": { + "intValue": 1000000000 + } } ], "droppedAttributesCount": 0, @@ -459,10 +469,52 @@ mod tests { "droppedLinksCount": 0 }"#; let otel_span: OtelSpan = serde_json::from_str(json).unwrap(); - let event_span: Annotated = Annotated::new(otel_span.into()); + let event_span: EventSpan = otel_span.into(); + assert_eq!(event_span.exclusive_time, Annotated::new(1000.0)); + let annotated_span: Annotated = Annotated::new(event_span); assert_eq!( - get_path!(event_span.data["environment"]), + get_path!(annotated_span.data["environment"]), Some(&Annotated::new("test".into())) ); } + + #[test] + fn parse_span_with_exclusive_time_ns_attribute() { + let json = r#"{ + "traceId": "89143b0763095bd9c9955e8175d1fb23", + "spanId": "e342abb1214ca181", + "parentSpanId": "0c7a7dea069bf5a6", + "name": "middleware - fastify -> @fastify/multipart", + "kind": 1, + "startTimeUnixNano": 1697620454980000000, + "endTimeUnixNano": 1697620454980078800, + "attributes": [ + { + "key": "sentry.exclusive_time_ns", + "value": { + "intValue": 3200000000 + } + } + ] + }"#; + let otel_span: OtelSpan = serde_json::from_str(json).unwrap(); + let event_span: EventSpan = otel_span.into(); + assert_eq!(event_span.exclusive_time, Annotated::new(3200.0)); + } + + #[test] + fn parse_span_no_exclusive_time_ns_attribute() { + let json = r#"{ + "traceId": "89143b0763095bd9c9955e8175d1fb23", + "spanId": "e342abb1214ca181", + "parentSpanId": "0c7a7dea069bf5a6", + "name": "middleware - fastify -> @fastify/multipart", + "kind": 1, + "startTimeUnixNano": 1697620454980000000, + "endTimeUnixNano": 1697620454980078800 + }"#; + let otel_span: OtelSpan = serde_json::from_str(json).unwrap(); + let event_span: EventSpan = otel_span.into(); + assert_eq!(event_span.exclusive_time, Annotated::new(0.0788)); + } } From 8e9569839f229aa4ac5649edd515ab440a80d1ba Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Wed, 29 Nov 2023 18:20:15 -0500 Subject: [PATCH 58/59] Parse a proper OTel trace on HTTP ingestion --- relay-server/src/endpoints/spans.rs | 34 +++++++++---------- relay-spans/src/lib.rs | 2 ++ relay-spans/src/span.rs | 29 +++++++++-------- relay-spans/src/trace.rs | 37 +++++++++++++++++++++ tests/integration/fixtures/__init__.py | 3 +- tests/integration/test_store.py | 45 +++++++++++++++++--------- 6 files changed, 103 insertions(+), 47 deletions(-) create mode 100644 relay-spans/src/trace.rs diff --git a/relay-server/src/endpoints/spans.rs b/relay-server/src/endpoints/spans.rs index 5d69435651..ed478bc0ba 100644 --- a/relay-server/src/endpoints/spans.rs +++ b/relay-server/src/endpoints/spans.rs @@ -1,36 +1,36 @@ -use axum::extract; +use axum::extract::{DefaultBodyLimit, Json}; use axum::http::StatusCode; use axum::response::IntoResponse; use axum::routing::{post, MethodRouter}; use bytes::Bytes; use relay_config::Config; +use relay_spans::TracesData; use crate::endpoints::common::{self, BadStoreRequest}; use crate::envelope::{ContentType, Envelope, Item, ItemType}; -use crate::extractors::{RawContentType, RequestMeta}; +use crate::extractors::RequestMeta; use crate::service::ServiceState; async fn handle( state: ServiceState, - content_type: RawContentType, meta: RequestMeta, - body: Bytes, + Json(trace): Json, ) -> Result { - if !content_type.as_ref().starts_with("application/json") { - return Ok(StatusCode::UNSUPPORTED_MEDIA_TYPE); - } - - if body.is_empty() { - return Err(BadStoreRequest::EmptyBody); - } - - let mut item = Item::new(ItemType::OtelSpan); - item.set_payload(ContentType::Json, body); let mut envelope = Envelope::from_request(None, meta); - envelope.add_item(item); + for resource_span in trace.resource_spans { + for scope_span in resource_span.scope_spans { + for span in scope_span.spans { + let Ok(payload) = serde_json::to_vec(&span) else { + continue; + }; + let mut item = Item::new(ItemType::OtelSpan); + item.set_payload(ContentType::Json, payload); + envelope.add_item(item); + } + } + } common::handle_envelope(&state, envelope).await?; - Ok(StatusCode::ACCEPTED) } @@ -40,5 +40,5 @@ where B::Data: Send + Into, B::Error: Into, { - post(handle).route_layer(extract::DefaultBodyLimit::max(config.max_span_size())) + post(handle).route_layer(DefaultBodyLimit::max(config.max_span_size())) } diff --git a/relay-spans/src/lib.rs b/relay-spans/src/lib.rs index d15c4c79f8..7e640f434f 100644 --- a/relay-spans/src/lib.rs +++ b/relay-spans/src/lib.rs @@ -7,7 +7,9 @@ )] pub use crate::span::OtelSpan; +pub use crate::trace::TracesData; mod otel_to_sentry_tags; mod span; mod status_codes; +mod trace; diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index d3fe0fbbcd..fd664b7f90 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -1,8 +1,8 @@ use std::str::FromStr; use chrono::{TimeZone, Utc}; -use serde::Deserialize; -use serde_repr::Deserialize_repr; +use serde::{Deserialize, Serialize}; +use serde_repr::{Deserialize_repr, Serialize_repr}; use relay_event_schema::protocol::{Span as EventSpan, SpanId, SpanStatus, Timestamp, TraceId}; use relay_protocol::{Annotated, Object, Value}; @@ -12,7 +12,7 @@ use crate::status_codes; /// This is a serde implementation of . /// A Span represents a single operation performed by a single component of the system. -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct OtelSpan { /// A unique identifier for a trace. All spans from the same trace share @@ -231,7 +231,8 @@ impl From for EventSpan { /// Event is a time-stamped annotation of the span, consisting of user-supplied /// text description and key-value pairs. -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] pub struct Event { /// attributes is a collection of attribute key/value pairs on the event. /// Attribute keys MUST be unique (it is not allowed to have more than one @@ -250,7 +251,8 @@ pub struct Event { pub time_unix_nano: u64, } -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] pub struct Link { /// attributes is a collection of attribute key/value pairs on the link. /// Attribute keys MUST be unique (it is not allowed to have more than one @@ -273,8 +275,8 @@ pub struct Link { pub trace_state: String, } -#[derive(Clone, Default, Debug, Deserialize_repr)] -#[repr(u32)] +#[derive(Clone, Default, Debug, Deserialize_repr, Serialize_repr)] +#[repr(u8)] pub enum SpanKind { /// Unspecified. Do NOT use as default. /// Implementations MAY assume SpanKind to be INTERNAL when receiving UNSPECIFIED. @@ -299,7 +301,8 @@ pub enum SpanKind { Consumer = 5, } -#[derive(Clone, Debug, Deserialize, Default)] +#[derive(Clone, Debug, Deserialize, Default, Serialize)] +#[serde(rename_all = "camelCase")] pub struct Status { /// A developer-facing human readable error message. #[serde(default)] @@ -309,7 +312,7 @@ pub struct Status { pub code: StatusCode, } -#[derive(Clone, Copy, Debug, Deserialize_repr, Default, PartialEq)] +#[derive(Clone, Copy, Debug, Deserialize_repr, Default, PartialEq, Serialize_repr)] #[repr(u8)] pub enum StatusCode { /// The default status. @@ -327,7 +330,7 @@ pub enum StatusCode { /// object containing arrays, key-value lists and primitives. /// The value is one of the listed fields. It is valid for all values to be unspecified /// in which case this AnyValue is considered to be "empty". -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub enum AnyValue { #[serde(rename = "arrayValue")] Array(ArrayValue), @@ -370,7 +373,7 @@ impl AnyValue { /// ArrayValue is a list of AnyValue messages. We need ArrayValue as a message /// since oneof in AnyValue does not allow repeated fields. -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct ArrayValue { /// Array of values. The array may be empty (contain 0 elements). #[serde(default)] @@ -383,7 +386,7 @@ pub struct ArrayValue { /// a list of KeyValue messages (e.g. in Span) we use `repeated KeyValue` directly to /// avoid unnecessary extra wrapping (which slows down the protocol). The 2 approaches /// are semantically equivalent. -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct KeyValueList { /// A collection of key/value pairs of key-value pairs. The list may be empty (may /// contain 0 elements). @@ -394,7 +397,7 @@ pub struct KeyValueList { /// KeyValue is a key-value pair that is used to store Span attributes, Link /// attributes, etc. -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct KeyValue { pub key: String, pub value: AnyValue, diff --git a/relay-spans/src/trace.rs b/relay-spans/src/trace.rs new file mode 100644 index 0000000000..b75d2ccb99 --- /dev/null +++ b/relay-spans/src/trace.rs @@ -0,0 +1,37 @@ +use crate::span::OtelSpan; +use serde::Deserialize; + +/// TracesData represents the traces data that can be stored in a persistent storage, +/// OR can be embedded by other protocols that transfer OTLP traces data but do +/// not implement the OTLP protocol. +/// +/// The main difference between this message and collector protocol is that +/// in this message there will not be any "control" or "metadata" specific to +/// OTLP protocol. +/// +/// When new fields are added into this message, the OTLP request MUST be updated +/// as well. +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct TracesData { + /// An array of ResourceSpans. + /// For data coming from a single resource this array will typically contain + /// one element. Intermediary nodes that receive data from multiple origins + /// typically batch the data before forwarding further and in that case this + /// array will contain multiple elements. + pub resource_spans: Vec, +} +/// A collection of ScopeSpans from a Resource. +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ResourceSpans { + /// A list of ScopeSpans that originate from a resource. + pub scope_spans: Vec, +} +/// A collection of Spans produced by an InstrumentationScope. +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ScopeSpans { + /// A list of Spans that originate from an instrumentation scope. + pub spans: Vec, +} diff --git a/tests/integration/fixtures/__init__.py b/tests/integration/fixtures/__init__.py index 2f6d4ff51b..4f58617d79 100644 --- a/tests/integration/fixtures/__init__.py +++ b/tests/integration/fixtures/__init__.py @@ -2,9 +2,8 @@ from requests import Session from requests.adapters import HTTPAdapter -from urllib3.util import Retry from sentry_sdk.envelope import Envelope, Item, PayloadRef - +from urllib3.util import Retry session = Session() retries = Retry(total=5, backoff_factor=0.1) diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index d46f6e1775..85fe066189 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1344,17 +1344,17 @@ def test_span_ingestion( "traceId": "89143b0763095bd9c9955e8175d1fb23", "spanId": "e342abb1214ca181", "name": "my 1st OTel span", - "startTimeUnixNano": int(start.timestamp() * 1_000_000_000), - "endTimeUnixNano": int(end.timestamp() * 1_000_000_000), + "startTimeUnixNano": int(start.timestamp() * 1e9), + "endTimeUnixNano": int(end.timestamp() * 1e9), "attributes": [ { "key": "sentry.exclusive_time_ns", "value": { "intValue": int(duration.total_seconds() * 1e9), }, - } + }, ], - } + }, ).encode() ), ) @@ -1384,18 +1384,33 @@ def test_span_ingestion( relay.send_otel_span( project_id, { - "traceId": "89143b0763095bd9c9955e8175d1fb24", - "spanId": "e342abb1214ca182", - "name": "my 2nd OTel span", - "startTimeUnixNano": int(start.timestamp() * 1_000_000_000) + 2, - "endTimeUnixNano": int(end.timestamp() * 1_000_000_000) + 3, - "attributes": [ + "resourceSpans": [ { - "key": "sentry.exclusive_time_ns", - "value": { - "intValue": int((duration.total_seconds() + 1) * 1e9), - }, - } + "scopeSpans": [ + { + "spans": [ + { + "traceId": "89143b0763095bd9c9955e8175d1fb24", + "spanId": "e342abb1214ca182", + "name": "my 2nd OTel span", + "startTimeUnixNano": int(start.timestamp() * 1e9) + + 2, + "endTimeUnixNano": int(end.timestamp() * 1e9) + 3, + "attributes": [ + { + "key": "sentry.exclusive_time_ns", + "value": { + "intValue": int( + (duration.total_seconds() + 1) * 1e9 + ), + }, + }, + ], + }, + ], + }, + ], + }, ], }, ) From 59fcb539716ebdfc27f9e42d4bfc4a1d42688ae3 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 30 Nov 2023 11:28:21 +0100 Subject: [PATCH 59/59] ref: fewer clones --- relay-spans/src/span.rs | 47 ++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index fd664b7f90..c64b2b6a20 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -115,29 +115,25 @@ pub struct OtelSpan { /// convert_from_otel_to_sentry_status returns a status as defined by Sentry based on the OTel status. fn convert_from_otel_to_sentry_status( status_code: StatusCode, - http_status_code: Option<&KeyValue>, - grpc_status_code: Option<&KeyValue>, + http_status_code: Option, + grpc_status_code: Option, ) -> SpanStatus { if status_code == StatusCode::Unset || status_code == StatusCode::Ok { return SpanStatus::Ok; } - if let Some(status_code_value) = http_status_code { - if let Some(code_value) = status_code_value.value.to_i64() { - if let Some(sentry_status) = status_codes::HTTP.get(&code_value) { - if let Ok(span_status) = SpanStatus::from_str(sentry_status) { - return span_status; - } + if let Some(code) = http_status_code { + if let Some(sentry_status) = status_codes::HTTP.get(&code) { + if let Ok(span_status) = SpanStatus::from_str(sentry_status) { + return span_status; } } } - if let Some(status_code_value) = grpc_status_code { - if let Some(code_value) = status_code_value.value.to_i64() { - if let Some(sentry_status) = status_codes::GRPC.get(&code_value) { - if let Ok(span_status) = SpanStatus::from_str(sentry_status) { - return span_status; - } + if let Some(code) = grpc_status_code { + if let Some(sentry_status) = status_codes::GRPC.get(&code) { + if let Ok(span_status) = SpanStatus::from_str(sentry_status) { + return span_status; } } } @@ -165,14 +161,17 @@ impl From for EventSpan { } else { Annotated::empty() }; - for attribute in attributes.iter() { + + let mut http_status_code = None; + let mut grpc_status_code = None; + for attribute in attributes.into_iter() { let key: String = if let Some(key) = OTEL_TO_SENTRY_TAGS.get(attribute.key.as_str()) { key.to_string() } else { - attribute.key.clone() + attribute.key }; if key.contains("exclusive_time_ns") { - let value = match attribute.value.clone() { + let value = match attribute.value { AnyValue::Int(v) => v as f64, AnyValue::Double(v) => v, AnyValue::String(v) => v.parse::().unwrap_or_default(), @@ -181,13 +180,18 @@ impl From for EventSpan { exclusive_time_ms = value / 1e6f64; continue; } - match attribute.value.clone() { + if key == "http.status_code" { + http_status_code = attribute.value.to_i64(); + } else if key == "rpc.grpc.status_code" { + grpc_status_code = attribute.value.to_i64(); + } + match attribute.value { AnyValue::Array(_) => {} AnyValue::Bool(v) => { data.insert(key, Annotated::new(v.into())); } AnyValue::Bytes(v) => { - if let Ok(v) = String::from_utf8(v.clone()) { + if let Ok(v) = String::from_utf8(v) { data.insert(key, Annotated::new(v.into())); } } @@ -207,13 +211,12 @@ impl From for EventSpan { exclusive_time_ms = (from.end_time_unix_nano - from.start_time_unix_nano) as f64 / 1e6f64; } - let http_status_code = attributes.iter().find(|a| a.key == "http.status_code"); - let grpc_status_code = attributes.iter().find(|a| a.key == "rpc.grpc.status_code"); + EventSpan { data: data.into(), description: name.into(), exclusive_time: exclusive_time_ms.into(), - parent_span_id: SpanId(parent_span_id.clone()).into(), + parent_span_id: SpanId(parent_span_id).into(), segment_id, span_id: Annotated::new(SpanId(span_id)), start_timestamp: Timestamp(start_timestamp).into(),