From 69b829016b1d167440b89364e0b5fa4d77dbe9a5 Mon Sep 17 00:00:00 2001 From: Kev Date: Thu, 27 Mar 2025 11:20:44 -0400 Subject: [PATCH 01/21] feat(ourlogs): Adjust 'log' protocol per sdk feedback Now that we've mostly finalized the logs protocol with sdks (see develop doc for more info), we want to update Relay to allow the 'log' item type to be sent in this format. Also in this PR is some shimming between the updated protocol / event schema and the kafka consumers. This shimming is temporary until the generic EAP items consumers are ready, at which point we'll have to transform the event-schema Relay accepts into the generic eap item kafka. --- CHANGELOG.md | 1 + relay-event-schema/src/processor/traits.rs | 1 - relay-event-schema/src/protocol/attribute.rs | 17 + relay-event-schema/src/protocol/ourlog.rs | 418 ++++++++++++++---- relay-ourlogs/src/lib.rs | 2 +- relay-ourlogs/src/ourlog.rs | 406 +++++++++++++++-- relay-server/src/services/processor/ourlog.rs | 2 +- tests/integration/test_ourlogs.py | 154 ++++++- 8 files changed, 860 insertions(+), 141 deletions(-) create mode 100644 relay-event-schema/src/protocol/attribute.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 02d37a16161..e23bdafaa5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ - Bump the revision of `sysinfo` to the revision at `15b3be3273ba286740122fed7bb7dccd2a79dc8f`. ([#4613](https://github.com/getsentry/relay/pull/4613)) - Switch the processor and store to `async`. ([#4552](https://github.com/getsentry/relay/pull/4552)) - Validate the spooling memory configuration on startup. ([#4617](https://github.com/getsentry/relay/pull/4617)) +- Adjust 'log' protocol / envelope item type per sdk feedback ([#4592](https://github.com/getsentry/relay/pull/4592)) ## 25.3.0 diff --git a/relay-event-schema/src/processor/traits.rs b/relay-event-schema/src/processor/traits.rs index 9c5491872d4..99f1fa55a7b 100644 --- a/relay-event-schema/src/processor/traits.rs +++ b/relay-event-schema/src/processor/traits.rs @@ -113,7 +113,6 @@ pub trait Processor: Sized { process_method!(process_trace_context, crate::protocol::TraceContext); process_method!(process_native_image_path, crate::protocol::NativeImagePath); process_method!(process_contexts, crate::protocol::Contexts); - process_method!(process_attribute_value, crate::protocol::AttributeValue); fn process_other( &mut self, diff --git a/relay-event-schema/src/protocol/attribute.rs b/relay-event-schema/src/protocol/attribute.rs new file mode 100644 index 00000000000..3f68787146a --- /dev/null +++ b/relay-event-schema/src/protocol/attribute.rs @@ -0,0 +1,17 @@ +use crate::processor::ProcessValue; +use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Value}; + +#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] +#[metastructure(trim = false)] +pub struct Attribute { + #[metastructure(field = "value", pii = "true", required = true)] + value: Annotated, +} + +#[derive(Clone, Debug, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] +pub enum AttributeValue { + String { value: Annotated }, + Int { value: Annotated }, + Double { value: Annotated }, + Bool { value: Annotated }, +} diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 753493e2cda..c7b108f7eb6 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -1,42 +1,33 @@ use relay_protocol::{ Annotated, Empty, Error, FromValue, IntoValue, Object, SkipSerialization, Value, }; +use std::fmt::{self, Display}; +use std::str::FromStr; use serde::ser::SerializeMap; +use serde::{Serialize, Serializer}; use crate::processor::ProcessValue; -use crate::protocol::{SpanId, TraceId}; +use crate::protocol::{SpanId, Timestamp, TraceId}; #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] #[metastructure(process_func = "process_ourlog", value_type = "OurLog")] pub struct OurLog { - /// Time when the event occurred. - #[metastructure(required = true, trim = false)] - pub timestamp_nanos: Annotated, - - /// Time when the event was observed. - #[metastructure(required = true, trim = false)] - pub observed_timestamp_nanos: Annotated, + /// Timestamp when the log was created. + #[metastructure(required = true)] + pub timestamp: Annotated, /// The ID of the trace the log belongs to. - #[metastructure(required = false, trim = false)] + #[metastructure(required = true, trim = false)] pub trace_id: Annotated, + /// The Span id. - /// #[metastructure(required = false, trim = false)] pub span_id: Annotated, - /// Trace flag bitfield. - #[metastructure(required = false)] - pub trace_flags: Annotated, - - /// This is the original string representation of the severity as it is known at the source - #[metastructure(required = false, max_chars = 32, pii = "true", trim = false)] - pub severity_text: Annotated, - - /// Numerical representation of the severity level - #[metastructure(required = false)] - pub severity_number: Annotated, + /// The log level. + #[metastructure(required = true)] + pub level: Annotated, /// Log body. #[metastructure(required = true, pii = "true", trim = false)] @@ -51,15 +42,29 @@ pub struct OurLog { pub other: Object, } +impl OurLog { + pub fn attribute(&self, key: &str) -> Option { + Some(match self.attributes.value()?.get(key) { + Some(value) => match value.value() { + Some(v) => match v { + AttributeValue::StringValue(s) => Value::String(s.clone()), + AttributeValue::IntValue(i) => Value::I64(*i), + AttributeValue::DoubleValue(f) => Value::F64(*f), + AttributeValue::BoolValue(b) => Value::Bool(*b), + _ => return None, + }, + None => return None, + }, + None => return None, + }) + } +} + #[derive(Debug, Clone, PartialEq, ProcessValue)] pub enum AttributeValue { - #[metastructure(field = "string_value", pii = "true")] StringValue(String), - #[metastructure(field = "int_value", pii = "true")] IntValue(i64), - #[metastructure(field = "double_value", pii = "true")] DoubleValue(f64), - #[metastructure(field = "bool_value", pii = "true")] BoolValue(bool), /// Any other unknown attribute value. /// @@ -154,90 +159,319 @@ impl Empty for AttributeValue { impl FromValue for AttributeValue { fn from_value(value: Annotated) -> Annotated { match value { - Annotated(Some(Value::String(value)), meta) => { - Annotated(Some(AttributeValue::StringValue(value)), meta) - } - Annotated(Some(Value::I64(value)), meta) => { - Annotated(Some(AttributeValue::IntValue(value)), meta) + Annotated(Some(Value::Object(mut object)), meta) => { + let attribute_type = object + .remove("type") + .and_then(|v| v.value().cloned()) + .and_then(|v| match v { + Value::String(s) => Some(s), + _ => None, + }); + + let value = object.remove("value"); + + match (attribute_type.as_deref(), value) { + (Some("string"), Some(Annotated(Some(Value::String(string_value)), _))) => { + Annotated(Some(AttributeValue::StringValue(string_value)), meta) + } + (Some("int"), Some(Annotated(Some(Value::String(string_value)), _))) => { + let mut meta = meta; + if let Ok(int_value) = string_value.parse::() { + Annotated(Some(AttributeValue::IntValue(int_value)), meta) + } else { + meta.add_error(Error::invalid("integer could not be parsed.")); + meta.set_original_value(Some(Value::Object(object))); + Annotated(None, meta) + } + } + (Some("int"), Some(Annotated(Some(Value::I64(_)), _))) => { + let mut meta = meta; + meta.add_error(Error::expected( + "64 bit integers have to be represented by a string in JSON", + )); + meta.set_original_value(Some(Value::Object(object))); + Annotated(None, meta) + } + (Some("double"), Some(Annotated(Some(Value::F64(double_value)), _))) => { + Annotated(Some(AttributeValue::DoubleValue(double_value)), meta) + } + (Some("bool"), Some(Annotated(Some(Value::Bool(bool_value)), _))) => { + Annotated(Some(AttributeValue::BoolValue(bool_value)), meta) + } + (Some(_), Some(Annotated(Some(Value::String(unknown_value)), _))) => { + Annotated(Some(AttributeValue::Unknown(unknown_value)), meta) + } + _ => { + let mut meta = meta; + meta.add_error(Error::expected( + "a valid attribute value (string, int, double, bool)", + )); + meta.set_original_value(Some(Value::Object(object))); + Annotated(None, meta) + } + } } - Annotated(Some(Value::F64(value)), meta) => { - Annotated(Some(AttributeValue::DoubleValue(value)), meta) + Annotated(None, meta) => Annotated(None, meta), + Annotated(Some(value), mut meta) => { + meta.add_error(Error::expected("an object")); + meta.set_original_value(Some(value)); + Annotated(None, meta) } - Annotated(Some(Value::Bool(value)), meta) => { - Annotated(Some(AttributeValue::BoolValue(value)), meta) + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum OurLogLevel { + Trace, + Debug, + Info, + Warn, + Error, + Fatal, + /// Unknown status, for forward compatibility. + Unknown(String), +} + +impl OurLogLevel { + fn as_str(&self) -> &str { + match self { + OurLogLevel::Trace => "trace", + OurLogLevel::Debug => "debug", + OurLogLevel::Info => "info", + OurLogLevel::Warn => "warn", + OurLogLevel::Error => "error", + OurLogLevel::Fatal => "fatal", + OurLogLevel::Unknown(s) => s.as_str(), + } + } +} + +impl Display for OurLogLevel { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.as_str()) + } +} +impl FromStr for OurLogLevel { + type Err = ParseOurLogLevelError; + + fn from_str(s: &str) -> Result { + Ok(match s { + "trace" => OurLogLevel::Trace, + "debug" => OurLogLevel::Debug, + "info" => OurLogLevel::Info, + "warn" => OurLogLevel::Warn, + "error" => OurLogLevel::Error, + "fatal" => OurLogLevel::Fatal, + other => OurLogLevel::Unknown(other.to_owned()), + }) + } +} + +impl FromValue for OurLogLevel { + fn from_value(value: Annotated) -> Annotated { + match value { + Annotated(Some(Value::String(value)), mut meta) => { + match OurLogLevel::from_str(&value) { + Ok(value) => Annotated(Some(value), meta), + Err(err) => { + meta.add_error(Error::invalid(err)); + meta.set_original_value(Some(value)); + Annotated(None, meta) + } + } } + Annotated(None, meta) => Annotated(None, meta), Annotated(Some(value), mut meta) => { - meta.add_error(Error::expected( - "a valid attribute value (string, int, double, bool)", - )); + meta.add_error(Error::expected("a level")); meta.set_original_value(Some(value)); Annotated(None, meta) } - Annotated(None, meta) => Annotated(None, meta), } } } +impl IntoValue for OurLogLevel { + fn into_value(self) -> Value { + Value::String(self.to_string()) + } + + fn serialize_payload(&self, s: S, _behavior: SkipSerialization) -> Result + where + Self: Sized, + S: Serializer, + { + Serialize::serialize(self.as_str(), s) + } +} + +impl ProcessValue for OurLogLevel {} + +impl Empty for OurLogLevel { + #[inline] + fn is_empty(&self) -> bool { + false + } +} + +/// An error used when parsing `OurLogLevel`. +#[derive(Debug)] +pub struct ParseOurLogLevelError; + +impl fmt::Display for ParseOurLogLevelError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "invalid our log level") + } +} + +impl std::error::Error for ParseOurLogLevelError {} + #[cfg(test)] mod tests { use super::*; + use relay_protocol::SerializableAnnotated; #[test] fn test_ourlog_serialization() { let json = r#"{ - "timestamp_nanos": 1544712660300000000, - "observed_timestamp_nanos": 1544712660300000000, - "trace_id": "5b8efff798038103d269b633813fc60c", - "span_id": "eee19b7ec3c1b174", - "severity_text": "Information", - "severity_number": 10, - "body": "Example log record", - "attributes": { - "boolean.attribute": { - "bool_value": true - }, - "double.attribute": { - "double_value": 637.704 - }, - "int.attribute": { - "int_value": 10 - }, - "string.attribute": { - "string_value": "some string" - } - } -}"#; - - let mut attributes = Object::new(); - attributes.insert( - "string.attribute".into(), - Annotated::new(AttributeValue::StringValue("some string".into())), - ); - attributes.insert( - "boolean.attribute".into(), - Annotated::new(AttributeValue::BoolValue(true)), - ); - attributes.insert( - "int.attribute".into(), - Annotated::new(AttributeValue::IntValue(10)), - ); - attributes.insert( - "double.attribute".into(), - Annotated::new(AttributeValue::DoubleValue(637.704)), - ); - - let log = Annotated::new(OurLog { - timestamp_nanos: Annotated::new(1544712660300000000), - observed_timestamp_nanos: Annotated::new(1544712660300000000), - severity_number: Annotated::new(10), - severity_text: Annotated::new("Information".to_string()), - trace_id: Annotated::new(TraceId("5b8efff798038103d269b633813fc60c".into())), - span_id: Annotated::new(SpanId("eee19b7ec3c1b174".into())), - body: Annotated::new("Example log record".to_string()), - attributes: Annotated::new(attributes), - ..Default::default() - }); - - assert_eq!(json, log.to_json_pretty().unwrap()); + "timestamp": 1544719860.0, + "trace_id": "5b8efff798038103d269b633813fc60c", + "span_id": "eee19b7ec3c1b174", + "level": "info", + "body": "Example log record", + "attributes": { + "boolean.attribute": { + "value": true, + "type": "bool" + }, + "sentry.severity_text": { + "value": "info", + "type": "string" + }, + "sentry.severity_number": { + "value": "10", + "type": "int" + }, + "sentry.observed_timestamp_nanos": { + "value": "1544712660300000000", + "type": "int" + }, + "sentry.trace_flags": { + "value": "10", + "type": "int" + } + } + }"#; + + let data = Annotated::::from_json(json).unwrap(); + insta::assert_debug_snapshot!(data, @r###" + OurLog { + timestamp: Timestamp( + 2018-12-13T16:51:00Z, + ), + trace_id: TraceId( + "5b8efff798038103d269b633813fc60c", + ), + span_id: SpanId( + "eee19b7ec3c1b174", + ), + level: Info, + body: "Example log record", + attributes: { + "boolean.attribute": BoolValue( + true, + ), + "sentry.observed_timestamp_nanos": IntValue( + 1544712660300000000, + ), + "sentry.severity_number": IntValue( + 10, + ), + "sentry.severity_text": StringValue( + "info", + ), + "sentry.trace_flags": IntValue( + 10, + ), + }, + other: {}, + } + "###); + + insta::assert_json_snapshot!(SerializableAnnotated(&data), @r###" + { + "timestamp": 1544719860.0, + "trace_id": "5b8efff798038103d269b633813fc60c", + "span_id": "eee19b7ec3c1b174", + "level": "info", + "body": "Example log record", + "attributes": { + "boolean.attribute": { + "bool_value": true + }, + "sentry.observed_timestamp_nanos": { + "int_value": 1544712660300000000 + }, + "sentry.severity_number": { + "int_value": 10 + }, + "sentry.severity_text": { + "string_value": "info" + }, + "sentry.trace_flags": { + "int_value": 10 + } + } + } + "###); + } + + #[test] + fn test_invalid_int_attribute() { + let json = r#"{ + "timestamp": 1544719860.0, + "trace_id": "5b8efff798038103d269b633813fc60c", + "span_id": "eee19b7ec3c1b174", + "level": "info", + "body": "Example log record", + "attributes": { + "sentry.severity_number": { + "value": 10, + "type": "int" + } + } + }"#; + + let data = Annotated::::from_json(json).unwrap(); + + insta::assert_json_snapshot!(SerializableAnnotated(&data), @r###" + { + "timestamp": 1544719860.0, + "trace_id": "5b8efff798038103d269b633813fc60c", + "span_id": "eee19b7ec3c1b174", + "level": "info", + "body": "Example log record", + "attributes": { + "sentry.severity_number": null + }, + "_meta": { + "attributes": { + "sentry.severity_number": { + "": { + "err": [ + [ + "invalid_data", + { + "reason": "expected 64 bit integers have to be represented by a string in JSON" + } + ] + ], + "val": {} + } + } + } + } + } + "###); } } diff --git a/relay-ourlogs/src/lib.rs b/relay-ourlogs/src/lib.rs index 38130a4a4c6..8498ba09e0f 100644 --- a/relay-ourlogs/src/lib.rs +++ b/relay-ourlogs/src/lib.rs @@ -7,7 +7,7 @@ )] pub use crate::ourlog::otel_to_sentry_log; - +pub use crate::ourlog::ourlog_merge_otel; pub use opentelemetry_proto::tonic::logs::v1::LogRecord as OtelLog; mod ourlog; diff --git a/relay-ourlogs/src/ourlog.rs b/relay-ourlogs/src/ourlog.rs index c6f2fafd30b..2d107fc7214 100644 --- a/relay-ourlogs/src/ourlog.rs +++ b/relay-ourlogs/src/ourlog.rs @@ -1,9 +1,14 @@ +use std::str::FromStr; + +use chrono::{TimeZone, Utc}; use opentelemetry_proto::tonic::common::v1::any_value::Value as OtelValue; use crate::OtelLog; use relay_common::time::UnixTimestamp; -use relay_event_schema::protocol::{AttributeValue, OurLog, SpanId, TraceId}; -use relay_protocol::{Annotated, Object}; +use relay_event_schema::protocol::{ + AttributeValue, OurLog, OurLogLevel, SpanId, Timestamp, TraceId, +}; +use relay_protocol::{Annotated, Object, Value}; /// Transform an OtelLog to a Sentry log. pub fn otel_to_sentry_log(otel_log: OtelLog) -> OurLog { @@ -14,11 +19,14 @@ pub fn otel_to_sentry_log(otel_log: OtelLog) -> OurLog { attributes, trace_id, span_id, + time_unix_nano, .. } = otel_log; - let span_id = hex::encode(span_id); - let trace_id = hex::encode(trace_id); + let span_id = SpanId(hex::encode(span_id)); + let trace_id = TraceId(hex::encode(trace_id)); + let nanos = time_unix_nano; + let timestamp = Utc.timestamp_nanos(nanos as i64); let body = body .and_then(|v| v.value) @@ -34,6 +42,29 @@ pub fn otel_to_sentry_log(otel_log: OtelLog) -> OurLog { // We may change this in the future with forwarding Relays. let observed_time_unix_nano = UnixTimestamp::now().as_nanos(); + attribute_data.insert( + "sentry.severity_text".to_string(), + Annotated::new(AttributeValue::StringValue(severity_text.clone())), + ); + attribute_data.insert( + "sentry.severity_number".to_string(), + Annotated::new(AttributeValue::IntValue(severity_number as i64)), + ); + attribute_data.insert( + "sentry.timestamp_nanos".to_string(), + Annotated::new(AttributeValue::StringValue(time_unix_nano.to_string())), + ); + attribute_data.insert( + "sentry.observed_timestamp_nanos".to_string(), + Annotated::new(AttributeValue::StringValue( + observed_time_unix_nano.to_string(), + )), + ); + attribute_data.insert( + "sentry.trace_flags".to_string(), + Annotated::new(AttributeValue::IntValue(0)), + ); + for attribute in attributes.into_iter() { if let Some(value) = attribute.value.and_then(|v| v.value) { let key = attribute.key; @@ -61,27 +92,168 @@ pub fn otel_to_sentry_log(otel_log: OtelLog) -> OurLog { } } + // Map severity_number to OurLogLevel, falling back to severity_text if it's not a number. + // Finally default to Info if severity_number is not in range and severity_text is not a valid + // log level. + let level = match severity_number { + 1..=4 => OurLogLevel::Trace, + 5..=8 => OurLogLevel::Debug, + 9..=12 => OurLogLevel::Info, + 13..=16 => OurLogLevel::Warn, + 17..=20 => OurLogLevel::Error, + 21..=24 => OurLogLevel::Fatal, + _ => OurLogLevel::from_str(&severity_text).unwrap_or(OurLogLevel::Info), + }; + + let mut other = Object::default(); + other.insert( + "severity_text".to_string(), + Annotated::new(Value::String(severity_text)), + ); + other.insert( + "severity_number".to_string(), + Annotated::new(Value::I64(severity_number as i64)), + ); + other.insert("trace_flags".to_string(), Annotated::new(Value::I64(0))); + other.insert( + "timestamp_nanos".to_string(), + Annotated::new(Value::U64(otel_log.time_unix_nano)), + ); + other.insert( + "observed_timestamp_nanos".to_string(), + Annotated::new(Value::U64(observed_time_unix_nano)), + ); + OurLog { - timestamp_nanos: Annotated::new(otel_log.time_unix_nano), - observed_timestamp_nanos: Annotated::new(observed_time_unix_nano), - trace_id: TraceId(trace_id).into(), - span_id: Annotated::new(SpanId(span_id)), - trace_flags: Annotated::new(0), - severity_text: severity_text.into(), - severity_number: Annotated::new(severity_number as i64), - attributes: attribute_data.into(), + timestamp: Annotated::new(Timestamp(timestamp)), + trace_id: Annotated::new(trace_id), + span_id: Annotated::new(span_id), + level: Annotated::new(level), + attributes: Annotated::new(attribute_data), body: Annotated::new(body), - ..Default::default() + other, + } +} + +/// This fills attributes with OTel specific fields to be compatible with the otel schema. +/// +/// This also currently backfills data into deprecated fields on the OurLog protocol in order to continue working with the snuba consumers. +/// +/// This will need to transform all fields into attributes to be ported to using the generic trace items consumers once they're done. +pub fn ourlog_merge_otel(ourlog: Annotated) -> Annotated { + let mut ourlog = ourlog; + if let Some(ourlog_value) = ourlog.value_mut() { + let attributes = ourlog_value.attributes.value_mut().get_or_insert_default(); + attributes.insert( + "sentry.severity_number".to_string(), + Annotated::new(AttributeValue::IntValue(level_to_otel_severity_number( + ourlog_value.level.value().cloned(), + ))), + ); + attributes.insert( + "sentry.severity_text".to_owned(), + Annotated::new(AttributeValue::StringValue( + ourlog_value + .level + .value() + .map(|level| level.to_string()) + .unwrap_or_else(|| "info".to_string()), + )), + ); + + if let Some(AttributeValue::StringValue(s)) = attributes + .get("sentry.severity_text") + .and_then(|a| a.value()) + { + ourlog_value.other.insert( + "severity_text".to_string(), + Annotated::new(Value::String(s.clone())), + ); + } + + if let Some(AttributeValue::IntValue(i)) = attributes + .get("sentry.severity_number") + .and_then(|a| a.value()) + { + ourlog_value.other.insert( + "severity_number".to_string(), + Annotated::new(Value::I64(*i)), + ); + } + + if let Some(AttributeValue::IntValue(i)) = + attributes.get("sentry.trace_flags").and_then(|a| a.value()) + { + ourlog_value + .other + .insert("trace_flags".to_string(), Annotated::new(Value::I64(*i))); + } + + if let Some(AttributeValue::StringValue(s)) = attributes + .get("sentry.observed_timestamp_nanos") + .and_then(|a| a.value()) + { + if let Ok(nanos) = s.parse::() { + ourlog_value.other.insert( + "observed_timestamp_nanos".to_string(), + Annotated::new(Value::U64(nanos)), + ); + } + } + + if let Some(AttributeValue::StringValue(s)) = attributes + .get("sentry.timestamp_nanos") + .and_then(|a| a.value()) + { + if let Ok(nanos) = s.parse::() { + ourlog_value.other.insert( + "timestamp_nanos".to_string(), + Annotated::new(Value::U64(nanos)), + ); + } + } + + // We ignore the passed observed time since Relay always acts as the collector in Sentry. + // We may change this in the future with forwarding Relays. + let observed_time_unix_nano = UnixTimestamp::now().as_nanos(); + ourlog_value.other.insert( + "observed_timestamp_nanos".to_string(), + Annotated::new(Value::U64(observed_time_unix_nano)), + ); + + if let Some(timestamp_nanos) = ourlog_value + .timestamp + .value() + .and_then(|timestamp| timestamp.0.timestamp_nanos_opt()) + { + ourlog_value.other.insert( + "timestamp_nanos".to_string(), + Annotated::new(Value::U64(timestamp_nanos as u64)), + ); + } + } + ourlog +} + +fn level_to_otel_severity_number(level: Option) -> i64 { + match level { + Some(OurLogLevel::Trace) => 1, + Some(OurLogLevel::Debug) => 5, + Some(OurLogLevel::Info) => 9, + Some(OurLogLevel::Warn) => 13, + Some(OurLogLevel::Error) => 17, + Some(OurLogLevel::Fatal) => 21, + _ => 25, } } #[cfg(test)] mod tests { use super::*; - use relay_protocol::{get_path, get_value}; + use relay_protocol::{get_path, get_value, SerializableAnnotated}; #[test] - fn parse_log() { + fn parse_otel_log() { // https://github.com/open-telemetry/opentelemetry-proto/blob/c4214b8168d0ce2a5236185efb8a1c8950cccdd6/examples/logs.json let json = r#"{ "timeUnixNano": "1544712660300000000", @@ -161,7 +333,28 @@ mod tests { } #[test] - fn parse_log_with_db_attributes() { + fn parse_otellog_with_invalid_trace_id() { + let json = r#"{ + "timeUnixNano": "1544712660300000000", + "observedTimeUnixNano": "1544712660300000000", + "severityNumber": 10, + "severityText": "Information", + "traceId": "", + "spanId": "EEE19B7EC3C1B174" + }"#; + + let otel_log: OtelLog = serde_json::from_str(json).unwrap(); + let our_log = otel_to_sentry_log(otel_log); + let annotated_log: Annotated = Annotated::new(our_log); + + assert_eq!( + get_path!(annotated_log.trace_id), + Some(&Annotated::new(TraceId("".into()))) + ); + } + + #[test] + fn parse_otel_log_with_db_attributes() { let json = r#"{ "timeUnixNano": "1544712660300000000", "observedTimeUnixNano": "1544712660300000000", @@ -208,7 +401,7 @@ mod tests { } #[test] - fn parse_log_without_observed_time() { + fn parse_otel_log_without_observed_time() { let json_without_observed_time = r#"{ "timeUnixNano": "1544712660300000000", "observedTimeUnixNano": "0", @@ -227,14 +420,19 @@ mod tests { let our_log: OurLog = otel_to_sentry_log(otel_log); let after_test = UnixTimestamp::now().as_nanos(); - let observed_time = our_log.observed_timestamp_nanos.value().unwrap(); - assert!(*observed_time > 0); - assert!(*observed_time >= before_test); - assert!(*observed_time <= after_test); + // Get the observed timestamp from attributes + let observed_timestamp = our_log + .attribute("sentry.observed_timestamp_nanos") + .and_then(|value| value.as_str().and_then(|s| s.parse::().ok())) + .unwrap_or(0); + + assert!(observed_timestamp > 0); + assert!(observed_timestamp >= before_test); + assert!(observed_timestamp <= after_test); } #[test] - fn parse_log_ignores_observed_time() { + fn parse_otel_log_ignores_observed_time() { let json_with_observed_time = r#"{ "timeUnixNano": "1544712660300000000", "observedTimeUnixNano": "1544712660300000000", @@ -253,14 +451,164 @@ mod tests { let our_log: OurLog = otel_to_sentry_log(otel_log); let after_test = UnixTimestamp::now().as_nanos(); - let observed_time = our_log.observed_timestamp_nanos.value().unwrap(); - assert!(*observed_time > 0); - assert!(*observed_time >= before_test); - assert!(*observed_time <= after_test); + // Get the observed timestamp from attributes + let observed_timestamp = our_log + .attribute("sentry.observed_timestamp_nanos") + .and_then(|value| value.as_str().and_then(|s| s.parse::().ok())) + .unwrap_or(0); + + assert!(observed_timestamp > 0); + assert!(observed_timestamp >= before_test); + assert!(observed_timestamp <= after_test); + assert_ne!(observed_timestamp, 1544712660300000000); + } + + #[test] + fn ourlog_merge_otel_log() { + let json = r#"{ + "timestamp": 946684800.0, + "level": "info", + "trace_id": "5B8EFFF798038103D269B633813FC60C", + "span_id": "EEE19B7EC3C1B174", + "body": "Example log record", + "attributes": { + "foo": { + "value": "9", + "type": "string" + } + } + }"#; + + let data = Annotated::::from_json(json).unwrap(); + let mut merged_log = ourlog_merge_otel(data); + if let Some(log) = merged_log.value_mut() { + log.other.insert( + "observed_timestamp_nanos".to_string(), + Annotated::new(Value::U64(1742481864000000000)), + ); + } - assert_ne!( - our_log.observed_timestamp_nanos, - Annotated::new(1544712660300000000) + insta::assert_debug_snapshot!(merged_log, @r###" + OurLog { + timestamp: Timestamp( + 2000-01-01T00:00:00Z, + ), + trace_id: TraceId( + "5b8efff798038103d269b633813fc60c", + ), + span_id: SpanId( + "eee19b7ec3c1b174", + ), + level: Info, + body: "Example log record", + attributes: { + "foo": StringValue( + "9", + ), + "sentry.severity_number": IntValue( + 9, + ), + "sentry.severity_text": StringValue( + "info", + ), + }, + other: { + "observed_timestamp_nanos": U64( + 1742481864000000000, + ), + "severity_number": I64( + 9, + ), + "severity_text": String( + "info", + ), + "timestamp_nanos": U64( + 946684800000000000, + ), + }, + } + "###); + } + + #[test] + #[allow(deprecated)] + fn ourlog_merge_otel_log_with_timestamp() { + let mut attributes = Object::new(); + attributes.insert( + "foo".to_string(), + Annotated::new(AttributeValue::StringValue("9".to_string())), ); + let datetime = Utc.with_ymd_and_hms(2021, 11, 29, 0, 0, 0).unwrap(); + let ourlog = OurLog { + timestamp: Annotated::new(Timestamp(datetime)), + attributes: Annotated::new(attributes), + ..Default::default() + }; + + let mut merged_log = ourlog_merge_otel(Annotated::new(ourlog)); + if let Some(log) = merged_log.value_mut() { + log.other.insert( + "observed_timestamp_nanos".to_string(), + Annotated::new(Value::U64(1742481864000000000)), + ); + } + + insta::assert_debug_snapshot!(merged_log, @r###" + OurLog { + timestamp: Timestamp( + 2021-11-29T00:00:00Z, + ), + trace_id: ~, + span_id: ~, + level: ~, + body: ~, + attributes: { + "foo": StringValue( + "9", + ), + "sentry.severity_number": IntValue( + 25, + ), + "sentry.severity_text": StringValue( + "info", + ), + }, + other: { + "observed_timestamp_nanos": U64( + 1742481864000000000, + ), + "severity_number": I64( + 25, + ), + "severity_text": String( + "info", + ), + "timestamp_nanos": U64( + 1638144000000000000, + ), + }, + } + "###); + + insta::assert_json_snapshot!(SerializableAnnotated(&merged_log), @r###" + { + "timestamp": 1638144000.0, + "attributes": { + "foo": { + "string_value": "9" + }, + "sentry.severity_number": { + "int_value": 25 + }, + "sentry.severity_text": { + "string_value": "info" + } + }, + "observed_timestamp_nanos": 1742481864000000000, + "severity_number": 25, + "severity_text": "info", + "timestamp_nanos": 1638144000000000000 + } + "###); } } diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index 6a8209a6ff7..43d033f9bb3 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -60,7 +60,7 @@ pub fn process(managed_envelope: &mut TypedEnvelope, project_info: Arc } }, ItemType::Log => match Annotated::::from_json_bytes(&item.payload()) { - Ok(our_log) => our_log, + Ok(our_log) => relay_ourlogs::ourlog_merge_otel(our_log), Err(err) => { relay_log::debug!("failed to parse Sentry Log: {}", err); return ItemAction::Drop(Outcome::Invalid(DiscardReason::InvalidLog)); diff --git a/tests/integration/test_ourlogs.py b/tests/integration/test_ourlogs.py index 31b738a1a62..5dfa0456261 100644 --- a/tests/integration/test_ourlogs.py +++ b/tests/integration/test_ourlogs.py @@ -14,7 +14,18 @@ } -def envelope_with_ourlogs(start: datetime, end: datetime) -> Envelope: +def envelope_with_sentry_logs(payload: dict) -> Envelope: + envelope = Envelope() + envelope.add_item( + Item( + type="log", + payload=PayloadRef(json=payload), + ) + ) + return envelope + + +def envelope_with_otel_logs(start: datetime) -> Envelope: envelope = Envelope() envelope.add_item( Item( @@ -23,7 +34,7 @@ def envelope_with_ourlogs(start: datetime, end: datetime) -> Envelope: bytes=json.dumps( { "timeUnixNano": str(int(start.timestamp() * 1e9)), - "observedTimeUnixNano": str(int(end.timestamp() * 1e9)), + "observedTimeUnixNano": str(int(start.timestamp() * 1e9)), "severityNumber": 10, "severityText": "Information", "traceId": "5B8EFFF798038103D269B633813FC60C", @@ -49,7 +60,7 @@ def envelope_with_ourlogs(start: datetime, end: datetime) -> Envelope: return envelope -def test_ourlog_extraction( +def test_ourlog_extraction_with_otel_logs( mini_sentry, relay_with_processing, ourlogs_consumer, @@ -63,42 +74,154 @@ def test_ourlog_extraction( relay = relay_with_processing(options=TEST_CONFIG) + start = datetime.now(timezone.utc) + duration = timedelta(milliseconds=500) now = datetime.now(timezone.utc) end = now - timedelta(seconds=1) start = end - duration - # Send OTel log and sentry log via envelope - envelope = envelope_with_ourlogs(start, end) + envelope = envelope_with_otel_logs(start) relay.send_envelope(project_id, envelope) ourlogs = ourlogs_consumer.get_ourlogs() - assert len(ourlogs) == 1 expected = { "organization_id": 1, "project_id": 42, "retention_days": 90, "timestamp_nanos": int(start.timestamp() * 1e9), - "observed_timestamp_nanos": time_within_delta( - start.timestamp(), expect_resolution="ns" - ), + "observed_timestamp_nanos": time_within_delta(start, expect_resolution="ns"), "trace_id": "5b8efff798038103d269b633813fc60c", "body": "Example log record", "trace_flags": 0, "span_id": "eee19b7ec3c1b174", "severity_text": "Information", "severity_number": 10, + "received": time_within_delta(start, expect_resolution="s"), "attributes": { "string.attribute": {"string_value": "some string"}, "boolean.attribute": {"bool_value": True}, "int.attribute": {"int_value": 10}, "double.attribute": {"double_value": 637.704}, + "sentry.severity_number": {"int_value": 10}, + "sentry.severity_text": {"string_value": "Information"}, + "sentry.timestamp_nanos": { + "string_value": str(int(start.timestamp() * 1e9)) + }, + "sentry.trace_flags": {"int_value": 0}, + }, + } + + del ourlogs[0]["attributes"]["sentry.observed_timestamp_nanos"] + + assert ourlogs == [expected] + + ourlogs_consumer.assert_empty() + + +def test_ourlog_extraction_with_sentry_logs( + mini_sentry, + relay_with_processing, + ourlogs_consumer, +): + ourlogs_consumer = ourlogs_consumer() + project_id = 42 + project_config = mini_sentry.add_full_project_config(project_id) + project_config["config"]["features"] = [ + "organizations:ourlogs-ingestion", + ] + + relay = relay_with_processing(options=TEST_CONFIG) + + start = datetime.now(timezone.utc) + + envelope = envelope_with_sentry_logs( + { + "timestamp": start.timestamp(), + "trace_id": "5b8efff798038103d269b633813fc60c", + "span_id": "eee19b7ec3c1b174", + "level": "info", + "body": "Example log record", + "severity_number": 10, + "attributes": { + "boolean.attribute": {"value": True, "type": "bool"}, + "pii": {"value": "4242 4242 4242 4242", "type": "string"}, + "sentry.severity_text": {"value": "info", "type": "string"}, + }, + } + ) + relay.send_envelope(project_id, envelope) + + ourlogs = ourlogs_consumer.get_ourlogs() + expected = { + "organization_id": 1, + "project_id": 42, + "retention_days": 90, + "timestamp_nanos": time_within_delta(start, expect_resolution="ns"), + "observed_timestamp_nanos": time_within_delta(start, expect_resolution="ns"), + "received": time_within_delta(start, expect_resolution="s"), + "trace_id": "5b8efff798038103d269b633813fc60c", + "body": "Example log record", + "span_id": "eee19b7ec3c1b174", + "severity_text": "info", + "severity_number": 9, + "attributes": { + "boolean.attribute": {"bool_value": True}, + "pii": {"string_value": "[creditcard]"}, + "sentry.severity_number": {"int_value": 9}, + "sentry.severity_text": {"string_value": "info"}, }, } - del ourlogs[0]["received"] - assert ourlogs[0] == expected + assert ourlogs == [expected] + ourlogs_consumer.assert_empty() + + +def test_ourlog_extraction_with_sentry_logs_with_missing_fields( + mini_sentry, + relay_with_processing, + ourlogs_consumer, +): + + ourlogs_consumer = ourlogs_consumer() + project_id = 42 + project_config = mini_sentry.add_full_project_config(project_id) + project_config["config"]["features"] = [ + "organizations:ourlogs-ingestion", + ] + + relay = relay_with_processing(options=TEST_CONFIG) + start = datetime.now(timezone.utc) + + envelope = envelope_with_sentry_logs( + { + "timestamp": start.timestamp(), + "trace_id": "5b8efff798038103d269b633813fc60c", + "level": "warn", + "body": "Example log record 2", + } + ) + relay.send_envelope(project_id, envelope) + + ourlogs = ourlogs_consumer.get_ourlogs() + expected = { + "organization_id": 1, + "project_id": 42, + "retention_days": 90, + "timestamp_nanos": time_within_delta(start, expect_resolution="ns"), + "observed_timestamp_nanos": time_within_delta(start, expect_resolution="ns"), + "received": time_within_delta(start, expect_resolution="s"), + "trace_id": "5b8efff798038103d269b633813fc60c", + "body": "Example log record 2", + "severity_text": "warn", + "severity_number": 13, + "attributes": { + "sentry.severity_number": {"int_value": 13}, + "sentry.severity_text": {"string_value": "warn"}, + }, + } + assert ourlogs == [expected] ourlogs_consumer.assert_empty() @@ -113,15 +236,12 @@ def test_ourlog_extraction_is_disabled_without_feature( project_config = mini_sentry.add_full_project_config(project_id) project_config["config"]["features"] = [] - duration = timedelta(milliseconds=500) - now = datetime.now(timezone.utc) - end = now - timedelta(seconds=1) - start = end - duration + start = datetime.now(timezone.utc) - envelope = envelope_with_ourlogs(start, end) + envelope = envelope_with_otel_logs(start) relay.send_envelope(project_id, envelope) ourlogs = ourlogs_consumer.get_ourlogs() - assert len(ourlogs) == 0 + assert len(ourlogs) == 0 ourlogs_consumer.assert_empty() From c978f7983d3a802b0319b2a62565a9e4f65780e8 Mon Sep 17 00:00:00 2001 From: Kev Date: Fri, 4 Apr 2025 11:51:22 -0400 Subject: [PATCH 02/21] Switch to using flattened structure in the protocol --- relay-event-schema/src/protocol/ourlog.rs | 318 ++++++++++------------ relay-ourlogs/src/ourlog.rs | 276 ++++++++++++------- 2 files changed, 323 insertions(+), 271 deletions(-) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index c7b108f7eb6..8c4e2c2736b 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -4,7 +4,6 @@ use relay_protocol::{ use std::fmt::{self, Display}; use std::str::FromStr; -use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; use crate::processor::ProcessValue; @@ -35,7 +34,7 @@ pub struct OurLog { /// Arbitrary attributes on a log. #[metastructure(pii = "true", trim = false)] - pub attributes: Annotated>, + pub attributes: Annotated>, /// Additional arbitrary fields for forwards compatibility. #[metastructure(additional_properties, retain = true, pii = "maybe", trim = false)] @@ -43,184 +42,130 @@ pub struct OurLog { } impl OurLog { - pub fn attribute(&self, key: &str) -> Option { - Some(match self.attributes.value()?.get(key) { - Some(value) => match value.value() { - Some(v) => match v { - AttributeValue::StringValue(s) => Value::String(s.clone()), - AttributeValue::IntValue(i) => Value::I64(*i), - AttributeValue::DoubleValue(f) => Value::F64(*f), - AttributeValue::BoolValue(b) => Value::Bool(*b), - _ => return None, - }, - None => return None, - }, - None => return None, - }) + pub fn attribute(&self, key: &str) -> Option> { + Some( + self.attributes + .value()? + .get(key)? + .value()? + .value + .clone() + .value, + ) } } -#[derive(Debug, Clone, PartialEq, ProcessValue)] -pub enum AttributeValue { - StringValue(String), - IntValue(i64), - DoubleValue(f64), - BoolValue(bool), - /// Any other unknown attribute value. - /// - /// This exists to ensure other attribute values such as array and object can be added in the future. - Unknown(String), +#[derive(Debug, Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] +pub struct OurLogAttribute { + #[metastructure(flatten)] + pub value: OurLogAttributeValue, + + /// Additional arbitrary fields for forwards compatibility. + #[metastructure(additional_properties)] + pub other: Object, } -impl IntoValue for AttributeValue { - fn into_value(self) -> Value { - let mut map = Object::new(); - match self { - AttributeValue::StringValue(v) => { - map.insert("string_value".to_string(), Annotated::new(Value::String(v))); - } - AttributeValue::IntValue(v) => { - map.insert("int_value".to_string(), Annotated::new(Value::I64(v))); - } - AttributeValue::DoubleValue(v) => { - map.insert("double_value".to_string(), Annotated::new(Value::F64(v))); - } - AttributeValue::BoolValue(v) => { - map.insert("bool_value".to_string(), Annotated::new(Value::Bool(v))); - } - AttributeValue::Unknown(v) => { - map.insert("unknown".to_string(), Annotated::new(Value::String(v))); - } +impl OurLogAttribute { + pub fn new(attribute_type: OurLogAttributeType, value: Value) -> Self { + Self { + value: OurLogAttributeValue::new(attribute_type, value), + other: Object::new(), } - Value::Object(map) } +} - fn serialize_payload(&self, s: S, _behavior: SkipSerialization) -> Result - where - Self: Sized, - S: serde::Serializer, - { - let mut map = s.serialize_map(None)?; - match self { - AttributeValue::StringValue(v) => { - map.serialize_entry("string_value", v)?; - } - AttributeValue::IntValue(v) => { - map.serialize_entry("int_value", v)?; - } - AttributeValue::DoubleValue(v) => { - map.serialize_entry("double_value", v)?; - } - AttributeValue::BoolValue(v) => { - map.serialize_entry("bool_value", v)?; - } - AttributeValue::Unknown(v) => { - map.serialize_entry("unknown", v)?; - } - } - map.end() - } +#[derive(Debug, Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] +pub struct OurLogAttributeValue { + #[metastructure(field = "type", required = true, trim = false)] + ty: Annotated, + #[metastructure(required = true, pii = "true")] + value: Annotated, } -impl AttributeValue { - pub fn string_value(&self) -> Option<&String> { - match self { - AttributeValue::StringValue(s) => Some(s), - _ => None, +impl OurLogAttributeValue { + fn new(attribute_type: OurLogAttributeType, value: Value) -> Self { + Self { + ty: Annotated::new(attribute_type.as_str().to_string()), + value: Annotated::new(value), } } - pub fn int_value(&self) -> Option { +} + +#[derive(Debug)] +pub enum OurLogAttributeType { + Bool, + Int, + Float, + String, + Unknown(String), +} + +impl OurLogAttributeType { + fn as_str(&self) -> &str { match self { - AttributeValue::IntValue(i) => Some(*i), - _ => None, + Self::Bool => "bool", + Self::Int => "int", + Self::Float => "float", + Self::String => "string", + Self::Unknown(value) => value, } } - pub fn double_value(&self) -> Option { - match self { - AttributeValue::DoubleValue(d) => Some(*d), - _ => None, - } +} + +impl fmt::Display for OurLogAttributeType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.as_str()) } - pub fn bool_value(&self) -> Option { - match self { - AttributeValue::BoolValue(b) => Some(*b), - _ => None, +} + +impl From for OurLogAttributeType { + fn from(value: String) -> Self { + match value.as_str() { + "bool" => Self::Bool, + "int" => Self::Int, + "float" => Self::Float, + "string" => Self::String, + _ => Self::Unknown(value), } } } -impl Empty for AttributeValue { +impl Empty for OurLogAttributeType { #[inline] fn is_empty(&self) -> bool { - matches!(self, Self::Unknown(_)) + false } } -impl FromValue for AttributeValue { +impl FromValue for OurLogAttributeType { fn from_value(value: Annotated) -> Annotated { - match value { - Annotated(Some(Value::Object(mut object)), meta) => { - let attribute_type = object - .remove("type") - .and_then(|v| v.value().cloned()) - .and_then(|v| match v { - Value::String(s) => Some(s), - _ => None, - }); - - let value = object.remove("value"); - - match (attribute_type.as_deref(), value) { - (Some("string"), Some(Annotated(Some(Value::String(string_value)), _))) => { - Annotated(Some(AttributeValue::StringValue(string_value)), meta) - } - (Some("int"), Some(Annotated(Some(Value::String(string_value)), _))) => { - let mut meta = meta; - if let Ok(int_value) = string_value.parse::() { - Annotated(Some(AttributeValue::IntValue(int_value)), meta) - } else { - meta.add_error(Error::invalid("integer could not be parsed.")); - meta.set_original_value(Some(Value::Object(object))); - Annotated(None, meta) - } - } - (Some("int"), Some(Annotated(Some(Value::I64(_)), _))) => { - let mut meta = meta; - meta.add_error(Error::expected( - "64 bit integers have to be represented by a string in JSON", - )); - meta.set_original_value(Some(Value::Object(object))); - Annotated(None, meta) - } - (Some("double"), Some(Annotated(Some(Value::F64(double_value)), _))) => { - Annotated(Some(AttributeValue::DoubleValue(double_value)), meta) - } - (Some("bool"), Some(Annotated(Some(Value::Bool(bool_value)), _))) => { - Annotated(Some(AttributeValue::BoolValue(bool_value)), meta) - } - (Some(_), Some(Annotated(Some(Value::String(unknown_value)), _))) => { - Annotated(Some(AttributeValue::Unknown(unknown_value)), meta) - } - _ => { - let mut meta = meta; - meta.add_error(Error::expected( - "a valid attribute value (string, int, double, bool)", - )); - meta.set_original_value(Some(Value::Object(object))); - Annotated(None, meta) - } - } - } + match String::from_value(value) { + Annotated(Some(value), meta) => Annotated(Some(value.into()), meta), Annotated(None, meta) => Annotated(None, meta), - Annotated(Some(value), mut meta) => { - meta.add_error(Error::expected("an object")); - meta.set_original_value(Some(value)); - Annotated(None, meta) - } } } } +impl IntoValue for OurLogAttributeType { + fn into_value(self) -> Value + where + Self: Sized, + { + Value::String(match self { + Self::Unknown(s) => s, + s => s.to_string(), + }) + } + + fn serialize_payload(&self, s: S, _behavior: SkipSerialization) -> Result + where + Self: Sized, + S: serde::Serializer, + { + serde::ser::Serialize::serialize(self.as_str(), s) + } +} + #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum OurLogLevel { Trace, @@ -378,21 +323,51 @@ mod tests { level: Info, body: "Example log record", attributes: { - "boolean.attribute": BoolValue( - true, - ), - "sentry.observed_timestamp_nanos": IntValue( - 1544712660300000000, - ), - "sentry.severity_number": IntValue( - 10, - ), - "sentry.severity_text": StringValue( - "info", - ), - "sentry.trace_flags": IntValue( - 10, - ), + "boolean.attribute": Attribute { + value: AttributeValue { + ty: "bool", + value: Bool( + true, + ), + }, + other: {}, + }, + "sentry.observed_timestamp_nanos": Attribute { + value: AttributeValue { + ty: "int", + value: String( + "1544712660300000000", + ), + }, + other: {}, + }, + "sentry.severity_number": Attribute { + value: AttributeValue { + ty: "int", + value: String( + "10", + ), + }, + other: {}, + }, + "sentry.severity_text": Attribute { + value: AttributeValue { + ty: "string", + value: String( + "info", + ), + }, + other: {}, + }, + "sentry.trace_flags": Attribute { + value: AttributeValue { + ty: "int", + value: String( + "10", + ), + }, + other: {}, + }, }, other: {}, } @@ -407,19 +382,24 @@ mod tests { "body": "Example log record", "attributes": { "boolean.attribute": { - "bool_value": true + "type": "bool", + "value": true }, "sentry.observed_timestamp_nanos": { - "int_value": 1544712660300000000 + "type": "int", + "value": "1544712660300000000" }, "sentry.severity_number": { - "int_value": 10 + "type": "int", + "value": "10" }, "sentry.severity_text": { - "string_value": "info" + "type": "string", + "value": "info" }, "sentry.trace_flags": { - "int_value": 10 + "type": "int", + "value": "10" } } } diff --git a/relay-ourlogs/src/ourlog.rs b/relay-ourlogs/src/ourlog.rs index 2d107fc7214..2f03f6aa073 100644 --- a/relay-ourlogs/src/ourlog.rs +++ b/relay-ourlogs/src/ourlog.rs @@ -6,10 +6,39 @@ use opentelemetry_proto::tonic::common::v1::any_value::Value as OtelValue; use crate::OtelLog; use relay_common::time::UnixTimestamp; use relay_event_schema::protocol::{ - AttributeValue, OurLog, OurLogLevel, SpanId, Timestamp, TraceId, + OurLog, OurLogAttribute, OurLogAttributeType, OurLogLevel, SpanId, Timestamp, TraceId, }; use relay_protocol::{Annotated, Object, Value}; +fn otel_value_to_log_attribute(value: OtelValue) -> Option { + match value { + OtelValue::BoolValue(v) => Some(OurLogAttribute::new( + OurLogAttributeType::Bool, + Value::Bool(v), + )), + OtelValue::DoubleValue(v) => Some(OurLogAttribute::new( + OurLogAttributeType::Float, + Value::F64(v), + )), + OtelValue::IntValue(v) => Some(OurLogAttribute::new( + OurLogAttributeType::Int, + Value::I64(v), + )), + OtelValue::StringValue(v) => Some(OurLogAttribute::new( + OurLogAttributeType::String, + Value::String(v), + )), + OtelValue::BytesValue(v) => String::from_utf8(v).map_or(None, |str| { + Some(OurLogAttribute::new( + OurLogAttributeType::String, + Value::String(str), + )) + }), + OtelValue::ArrayValue(_) => None, + OtelValue::KvlistValue(_) => None, + } +} + /// Transform an OtelLog to a Sentry log. pub fn otel_to_sentry_log(otel_log: OtelLog) -> OurLog { let OtelLog { @@ -44,50 +73,45 @@ pub fn otel_to_sentry_log(otel_log: OtelLog) -> OurLog { attribute_data.insert( "sentry.severity_text".to_string(), - Annotated::new(AttributeValue::StringValue(severity_text.clone())), + Annotated::new(OurLogAttribute::new( + OurLogAttributeType::String, + Value::String(severity_text.clone()), + )), ); attribute_data.insert( "sentry.severity_number".to_string(), - Annotated::new(AttributeValue::IntValue(severity_number as i64)), + Annotated::new(OurLogAttribute::new( + OurLogAttributeType::Int, + Value::I64(severity_number as i64), + )), ); attribute_data.insert( "sentry.timestamp_nanos".to_string(), - Annotated::new(AttributeValue::StringValue(time_unix_nano.to_string())), + Annotated::new(OurLogAttribute::new( + OurLogAttributeType::String, + Value::String(time_unix_nano.to_string()), + )), ); attribute_data.insert( "sentry.observed_timestamp_nanos".to_string(), - Annotated::new(AttributeValue::StringValue( - observed_time_unix_nano.to_string(), + Annotated::new(OurLogAttribute::new( + OurLogAttributeType::String, + Value::String(observed_time_unix_nano.to_string()), )), ); attribute_data.insert( "sentry.trace_flags".to_string(), - Annotated::new(AttributeValue::IntValue(0)), + Annotated::new(OurLogAttribute::new( + OurLogAttributeType::Int, + Value::I64(0), + )), ); for attribute in attributes.into_iter() { if let Some(value) = attribute.value.and_then(|v| v.value) { let key = attribute.key; - match value { - OtelValue::ArrayValue(_) => {} - OtelValue::BoolValue(v) => { - attribute_data.insert(key, Annotated::new(AttributeValue::BoolValue(v))); - } - OtelValue::BytesValue(v) => { - if let Ok(v) = String::from_utf8(v) { - attribute_data.insert(key, Annotated::new(AttributeValue::StringValue(v))); - } - } - OtelValue::DoubleValue(v) => { - attribute_data.insert(key, Annotated::new(AttributeValue::DoubleValue(v))); - } - OtelValue::IntValue(v) => { - attribute_data.insert(key, Annotated::new(AttributeValue::IntValue(v))); - } - OtelValue::KvlistValue(_) => {} - OtelValue::StringValue(v) => { - attribute_data.insert(key, Annotated::new(AttributeValue::StringValue(v))); - } + if let Some(v) = otel_value_to_log_attribute(value) { + attribute_data.insert(key, Annotated::new(v)); } } } @@ -137,7 +161,7 @@ pub fn otel_to_sentry_log(otel_log: OtelLog) -> OurLog { /// This fills attributes with OTel specific fields to be compatible with the otel schema. /// -/// This also currently backfills data into deprecated fields on the OurLog protocol in order to continue working with the snuba consumers. +/// This also currently backfills data into deprecated fields (other) on the OurLog protocol in order to continue working with the snuba consumers. /// /// This will need to transform all fields into attributes to be ported to using the generic trace items consumers once they're done. pub fn ourlog_merge_otel(ourlog: Annotated) -> Annotated { @@ -146,70 +170,71 @@ pub fn ourlog_merge_otel(ourlog: Annotated) -> Annotated { let attributes = ourlog_value.attributes.value_mut().get_or_insert_default(); attributes.insert( "sentry.severity_number".to_string(), - Annotated::new(AttributeValue::IntValue(level_to_otel_severity_number( - ourlog_value.level.value().cloned(), - ))), + Annotated::new(OurLogAttribute::new( + OurLogAttributeType::Int, + Value::I64(level_to_otel_severity_number( + ourlog_value.level.value().cloned(), + )), + )), ); attributes.insert( "sentry.severity_text".to_owned(), - Annotated::new(AttributeValue::StringValue( - ourlog_value - .level - .value() - .map(|level| level.to_string()) - .unwrap_or_else(|| "info".to_string()), + Annotated::new(OurLogAttribute::new( + OurLogAttributeType::String, + Value::String( + ourlog_value + .level + .value() + .map(|level| level.to_string()) + .unwrap_or_else(|| "info".to_string()), + ), )), ); - if let Some(AttributeValue::StringValue(s)) = attributes - .get("sentry.severity_text") - .and_then(|a| a.value()) - { - ourlog_value.other.insert( - "severity_text".to_string(), - Annotated::new(Value::String(s.clone())), - ); + if let Some(value) = ourlog_value.attribute("sentry.severity_text") { + if let Some(s) = value.as_str() { + ourlog_value.other.insert( + "severity_text".to_string(), + Annotated::new(Value::String(s.to_string())), + ); + } } - if let Some(AttributeValue::IntValue(i)) = attributes - .get("sentry.severity_number") - .and_then(|a| a.value()) - { - ourlog_value.other.insert( - "severity_number".to_string(), - Annotated::new(Value::I64(*i)), - ); + if let Some(value) = ourlog_value.attribute("sentry.severity_number") { + if let Some(v) = value.value() { + ourlog_value + .other + .insert("severity_number".to_string(), Annotated::new(v.clone())); + } } - if let Some(AttributeValue::IntValue(i)) = - attributes.get("sentry.trace_flags").and_then(|a| a.value()) - { - ourlog_value - .other - .insert("trace_flags".to_string(), Annotated::new(Value::I64(*i))); + if let Some(value) = ourlog_value.attribute("sentry.trace_flags") { + if let Some(v) = value.value() { + ourlog_value + .other + .insert("trace_flags".to_string(), Annotated::new(v.clone())); + } } - if let Some(AttributeValue::StringValue(s)) = attributes - .get("sentry.observed_timestamp_nanos") - .and_then(|a| a.value()) - { - if let Ok(nanos) = s.parse::() { - ourlog_value.other.insert( - "observed_timestamp_nanos".to_string(), - Annotated::new(Value::U64(nanos)), - ); + if let Some(value) = ourlog_value.attribute("sentry.observed_timestamp_nanos") { + if let Some(s) = value.as_str() { + if let Ok(nanos) = s.parse::() { + ourlog_value.other.insert( + "observed_timestamp_nanos".to_string(), + Annotated::new(Value::U64(nanos)), + ); + } } } - if let Some(AttributeValue::StringValue(s)) = attributes - .get("sentry.timestamp_nanos") - .and_then(|a| a.value()) - { - if let Ok(nanos) = s.parse::() { - ourlog_value.other.insert( - "timestamp_nanos".to_string(), - Annotated::new(Value::U64(nanos)), - ); + if let Some(value) = ourlog_value.attribute("sentry.timestamp_nanos") { + if let Some(s) = value.as_str() { + if let Ok(nanos) = s.parse::() { + ourlog_value.other.insert( + "timestamp_nanos".to_string(), + Annotated::new(Value::U64(nanos)), + ); + } } } @@ -250,7 +275,7 @@ fn level_to_otel_severity_number(level: Option) -> i64 { #[cfg(test)] mod tests { use super::*; - use relay_protocol::{get_path, get_value, SerializableAnnotated}; + use relay_protocol::{get_path, SerializableAnnotated}; #[test] fn parse_otel_log() { @@ -395,8 +420,13 @@ mod tests { Some(&Annotated::new("Database query executed".into())) ); assert_eq!( - get_value!(annotated_log.attributes["db.statement"]!).string_value(), - Some(&"SELECT \"table\".\"col\" FROM \"table\" WHERE \"table\".\"col\" = %s".into()) + annotated_log + .value() + .and_then(|v| v.attribute("db.statement")) + .unwrap() + .value() + .and_then(|v| v.as_str()), + Some("SELECT \"table\".\"col\" FROM \"table\" WHERE \"table\".\"col\" = %s") ); } @@ -502,15 +532,33 @@ mod tests { level: Info, body: "Example log record", attributes: { - "foo": StringValue( - "9", - ), - "sentry.severity_number": IntValue( - 9, - ), - "sentry.severity_text": StringValue( - "info", - ), + "foo": OurLogAttribute { + value: OurLogAttributeValue { + ty: "string", + value: String( + "9", + ), + }, + other: {}, + }, + "sentry.severity_number": OurLogAttribute { + value: OurLogAttributeValue { + ty: "int", + value: I64( + 9, + ), + }, + other: {}, + }, + "sentry.severity_text": OurLogAttribute { + value: OurLogAttributeValue { + ty: "string", + value: String( + "info", + ), + }, + other: {}, + }, }, other: { "observed_timestamp_nanos": U64( @@ -536,7 +584,10 @@ mod tests { let mut attributes = Object::new(); attributes.insert( "foo".to_string(), - Annotated::new(AttributeValue::StringValue("9".to_string())), + Annotated::new(OurLogAttribute::new( + OurLogAttributeType::String, + Value::String("9".to_string()), + )), ); let datetime = Utc.with_ymd_and_hms(2021, 11, 29, 0, 0, 0).unwrap(); let ourlog = OurLog { @@ -563,15 +614,33 @@ mod tests { level: ~, body: ~, attributes: { - "foo": StringValue( - "9", - ), - "sentry.severity_number": IntValue( - 25, - ), - "sentry.severity_text": StringValue( - "info", - ), + "foo": OurLogAttribute { + value: OurLogAttributeValue { + ty: "string", + value: String( + "9", + ), + }, + other: {}, + }, + "sentry.severity_number": OurLogAttribute { + value: OurLogAttributeValue { + ty: "int", + value: I64( + 25, + ), + }, + other: {}, + }, + "sentry.severity_text": OurLogAttribute { + value: OurLogAttributeValue { + ty: "string", + value: String( + "info", + ), + }, + other: {}, + }, }, other: { "observed_timestamp_nanos": U64( @@ -595,13 +664,16 @@ mod tests { "timestamp": 1638144000.0, "attributes": { "foo": { - "string_value": "9" + "type": "string", + "value": "9" }, "sentry.severity_number": { - "int_value": 25 + "type": "int", + "value": 25 }, "sentry.severity_text": { - "string_value": "info" + "type": "string", + "value": "info" } }, "observed_timestamp_nanos": 1742481864000000000, From 469a4dde2d54d75d4aa757f1a62b0bfa9ad703ab Mon Sep 17 00:00:00 2001 From: Kev Date: Fri, 4 Apr 2025 12:10:02 -0400 Subject: [PATCH 03/21] Rm dead code --- relay-event-schema/src/protocol/attribute.rs | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 relay-event-schema/src/protocol/attribute.rs diff --git a/relay-event-schema/src/protocol/attribute.rs b/relay-event-schema/src/protocol/attribute.rs deleted file mode 100644 index 3f68787146a..00000000000 --- a/relay-event-schema/src/protocol/attribute.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::processor::ProcessValue; -use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Value}; - -#[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] -#[metastructure(trim = false)] -pub struct Attribute { - #[metastructure(field = "value", pii = "true", required = true)] - value: Annotated, -} - -#[derive(Clone, Debug, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] -pub enum AttributeValue { - String { value: Annotated }, - Int { value: Annotated }, - Double { value: Annotated }, - Bool { value: Annotated }, -} From e93ce63c8c807787c270b8559e32d5cf28d5a66e Mon Sep 17 00:00:00 2001 From: Kev Date: Fri, 4 Apr 2025 16:13:53 -0400 Subject: [PATCH 04/21] Add structs for LogKafkaMessage to fix serialization of attributes --- relay-server/src/services/store.rs | 67 ++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 4 deletions(-) diff --git a/relay-server/src/services/store.rs b/relay-server/src/services/store.rs index 7499a7c6dd5..2ee5f0d022e 100644 --- a/relay-server/src/services/store.rs +++ b/relay-server/src/services/store.rs @@ -996,6 +996,7 @@ impl StoreService { log.project_id = scoping.project_id.value(); log.retention_days = retention_days; log.received = safe_timestamp(received_at); + let message = KafkaMessage::Log { headers: BTreeMap::from([("project_id".to_string(), scoping.project_id.to_string())]), message: log, @@ -1124,8 +1125,8 @@ where .serialize(serializer) } -pub fn serialize_btreemap_skip_nulls( - map: &Option>>, +pub fn serialize_btreemap_skip_nulls( + map: &Option>>, serializer: S, ) -> Result where @@ -1143,6 +1144,39 @@ where m.end() } +/** + * This shouldn't be necessary with enum serialization, but since serde's tag doesn't work with `type` as it has to be renamed due to being a keyword, + * this allows us to not emit the 'type' field, which would otherwise break the ourlogs consumer. + */ +fn serialize_log_attribute_value( + attr: &Option, + serializer: S, +) -> Result +where + S: serde::Serializer, +{ + let Some(attr) = attr else { + return serializer.serialize_none(); + }; + + let mut map = serializer.serialize_map(Some(1))?; + match attr { + LogAttributeValue::String(value) => { + map.serialize_entry("string_value", value)?; + } + LogAttributeValue::Int(value) => { + map.serialize_entry("int_value", value)?; + } + LogAttributeValue::Bool(value) => { + map.serialize_entry("bool_value", value)?; + } + LogAttributeValue::Float(value) => { + map.serialize_entry("float_value", value)?; + } + } + map.end() +} + /// Container payload for event messages. #[derive(Debug, Serialize)] struct EventKafkaMessage { @@ -1394,6 +1428,27 @@ struct SpanKafkaMessage<'a> { platform: Cow<'a, str>, // We only use this for logging for now } +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(tag = "type", content = "value")] +enum LogAttributeValue { + #[serde(rename = "string")] + String(String), + #[serde(rename = "bool")] + Bool(bool), + #[serde(rename = "int")] + Int(i64), + #[serde(rename = "float")] + Float(f64), +} + +/// This is a temporary struct to convert the old attribute format to the new one. +#[derive(Debug, Serialize, Deserialize)] +#[allow(dead_code)] +struct LogAttribute { + #[serde(flatten, serialize_with = "serialize_log_attribute_value")] + value: Option, +} + #[derive(Debug, Deserialize, Serialize)] struct LogKafkaMessage<'a> { #[serde(default)] @@ -1417,8 +1472,12 @@ struct LogKafkaMessage<'a> { severity_text: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] severity_number: Option, - #[serde(default, skip_serializing_if = "none_or_empty_object")] - attributes: Option<&'a RawValue>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + serialize_with = "serialize_btreemap_skip_nulls" + )] + attributes: Option>>, #[serde(default, skip_serializing_if = "Option::is_none")] trace_flags: Option, } From a70e22c4436c3fad6929e2bb92e7d2a9f57d34cf Mon Sep 17 00:00:00 2001 From: Kev Date: Fri, 4 Apr 2025 17:32:11 -0400 Subject: [PATCH 05/21] Get processing working --- relay-event-schema/src/protocol/ourlog.rs | 114 +++---- relay-ourlogs/src/ourlog.rs | 2 +- relay-server/src/services/processor/ourlog.rs | 312 +++++++++++++++++- relay-server/src/services/store.rs | 43 ++- tests/integration/test_ourlogs.py | 2 + 5 files changed, 396 insertions(+), 77 deletions(-) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 8c4e2c2736b..50444bdd392 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -55,7 +55,7 @@ impl OurLog { } } -#[derive(Debug, Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] +#[derive(Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] pub struct OurLogAttribute { #[metastructure(flatten)] pub value: OurLogAttributeValue, @@ -68,25 +68,34 @@ pub struct OurLogAttribute { impl OurLogAttribute { pub fn new(attribute_type: OurLogAttributeType, value: Value) -> Self { Self { - value: OurLogAttributeValue::new(attribute_type, value), + value: OurLogAttributeValue::new(Annotated::new(attribute_type), Annotated::new(value)), other: Object::new(), } } } +impl fmt::Debug for OurLogAttribute { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("OurLogAttribute") + .field("value", &self.value.value) + .field("type", &self.value.ty) + .finish() + } +} + #[derive(Debug, Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] pub struct OurLogAttributeValue { #[metastructure(field = "type", required = true, trim = false)] - ty: Annotated, + pub ty: Annotated, #[metastructure(required = true, pii = "true")] - value: Annotated, + pub value: Annotated, } impl OurLogAttributeValue { - fn new(attribute_type: OurLogAttributeType, value: Value) -> Self { + pub fn new(attribute_type: Annotated, value: Annotated) -> Self { Self { - ty: Annotated::new(attribute_type.as_str().to_string()), - value: Annotated::new(value), + ty: attribute_type.map_value(|at| at.as_str().to_string()), + value, } } } @@ -95,7 +104,7 @@ impl OurLogAttributeValue { pub enum OurLogAttributeType { Bool, Int, - Float, + Double, String, Unknown(String), } @@ -105,7 +114,7 @@ impl OurLogAttributeType { match self { Self::Bool => "bool", Self::Int => "int", - Self::Float => "float", + Self::Double => "double", Self::String => "string", Self::Unknown(value) => value, } @@ -123,7 +132,7 @@ impl From for OurLogAttributeType { match value.as_str() { "bool" => Self::Bool, "int" => Self::Int, - "float" => Self::Float, + "double" => Self::Double, "string" => Self::String, _ => Self::Unknown(value), } @@ -323,50 +332,35 @@ mod tests { level: Info, body: "Example log record", attributes: { - "boolean.attribute": Attribute { - value: AttributeValue { - ty: "bool", - value: Bool( - true, - ), - }, - other: {}, + "boolean.attribute": OurLogAttribute { + value: Bool( + true, + ), + type: "bool", }, - "sentry.observed_timestamp_nanos": Attribute { - value: AttributeValue { - ty: "int", - value: String( - "1544712660300000000", - ), - }, - other: {}, + "sentry.observed_timestamp_nanos": OurLogAttribute { + value: String( + "1544712660300000000", + ), + type: "int", }, - "sentry.severity_number": Attribute { - value: AttributeValue { - ty: "int", - value: String( - "10", - ), - }, - other: {}, + "sentry.severity_number": OurLogAttribute { + value: String( + "10", + ), + type: "int", }, - "sentry.severity_text": Attribute { - value: AttributeValue { - ty: "string", - value: String( - "info", - ), - }, - other: {}, + "sentry.severity_text": OurLogAttribute { + value: String( + "info", + ), + type: "string", }, - "sentry.trace_flags": Attribute { - value: AttributeValue { - ty: "int", - value: String( - "10", - ), - }, - other: {}, + "sentry.trace_flags": OurLogAttribute { + value: String( + "10", + ), + type: "int", }, }, other: {}, @@ -432,23 +426,9 @@ mod tests { "level": "info", "body": "Example log record", "attributes": { - "sentry.severity_number": null - }, - "_meta": { - "attributes": { - "sentry.severity_number": { - "": { - "err": [ - [ - "invalid_data", - { - "reason": "expected 64 bit integers have to be represented by a string in JSON" - } - ] - ], - "val": {} - } - } + "sentry.severity_number": { + "type": "int", + "value": 10 } } } diff --git a/relay-ourlogs/src/ourlog.rs b/relay-ourlogs/src/ourlog.rs index 2f03f6aa073..46ff8efe550 100644 --- a/relay-ourlogs/src/ourlog.rs +++ b/relay-ourlogs/src/ourlog.rs @@ -17,7 +17,7 @@ fn otel_value_to_log_attribute(value: OtelValue) -> Option { Value::Bool(v), )), OtelValue::DoubleValue(v) => Some(OurLogAttribute::new( - OurLogAttributeType::Float, + OurLogAttributeType::Double, Value::F64(v), )), OtelValue::IntValue(v) => Some(OurLogAttribute::new( diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index 43d033f9bb3..f27638a7813 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -18,10 +18,10 @@ use { crate::services::processor::ProcessingError, relay_dynamic_config::ProjectConfig, relay_event_schema::processor::{process_value, ProcessingState}, - relay_event_schema::protocol::OurLog, + relay_event_schema::protocol::{OurLog, OurLogAttributeType, OurLogAttributeValue}, relay_ourlogs::OtelLog, relay_pii::PiiProcessor, - relay_protocol::Annotated, + relay_protocol::{Annotated, ErrorKind, Value}, }; /// Removes logs from the envelope if the feature is not enabled. @@ -75,6 +75,11 @@ pub fn process(managed_envelope: &mut TypedEnvelope, project_info: Arc return ItemAction::Drop(Outcome::Invalid(DiscardReason::Internal)); } + if let Err(e) = normalize(&mut annotated_log) { + relay_log::debug!("failed to normalize log: {}", e); + return ItemAction::Drop(Outcome::Invalid(DiscardReason::Internal)); + }; + let mut new_item = Item::new(ItemType::Log); let payload = match annotated_log.to_json() { Ok(payload) => payload, @@ -91,6 +96,17 @@ pub fn process(managed_envelope: &mut TypedEnvelope, project_info: Arc }); } +#[cfg(feature = "processing")] +fn normalize(annotated_log: &mut Annotated) -> Result<(), ProcessingError> { + let Some(log) = annotated_log.value_mut() else { + return Err(ProcessingError::NoEventPayload); + }; + + process_attribute_types(log); + + Ok(()) +} + #[cfg(feature = "processing")] fn scrub( annotated_log: &mut Annotated, @@ -112,6 +128,75 @@ fn scrub( Ok(()) } +#[cfg(feature = "processing")] +fn process_attribute_types(ourlog: &mut OurLog) { + if let Some(data) = ourlog.attributes.value_mut() { + for (_, attr) in data.iter_mut() { + if let Some(attr) = attr.value_mut() { + attr.value = match (attr.value.ty.clone(), attr.value.value.clone()) { + (Annotated(Some(ty), ty_meta), Annotated(Some(value), mut value_meta)) => { + let attr_type = OurLogAttributeType::from(ty); + match (attr_type, value) { + (OurLogAttributeType::Bool, Value::Bool(_)) => attr.value.clone(), + (OurLogAttributeType::Int, Value::I64(_)) => attr.value.clone(), + (OurLogAttributeType::Int, Value::U64(_)) => attr.value.clone(), + (OurLogAttributeType::Int, Value::String(v)) => { + if v.parse::().is_ok() { + attr.value.clone() + } else { + value_meta.add_error(ErrorKind::InvalidData); + value_meta.set_original_value(Some(v.clone())); + OurLogAttributeValue::new( + Annotated( + Some(OurLogAttributeType::Unknown( + "unknown".to_string(), + )), + ty_meta, + ), + Annotated(Some(Value::String("".to_string())), value_meta), + ) + } + } + (OurLogAttributeType::Double, Value::F64(_)) => attr.value.clone(), + (OurLogAttributeType::Double, Value::I64(_)) => attr.value.clone(), + (OurLogAttributeType::Double, Value::U64(_)) => attr.value.clone(), + (OurLogAttributeType::Double, Value::String(_)) => attr.value.clone(), + (OurLogAttributeType::String, Value::String(_)) => attr.value.clone(), + (_ty @ OurLogAttributeType::Unknown(_), value) => { + value_meta.add_error(ErrorKind::InvalidData); + value_meta.set_original_value(Some(value.clone())); + OurLogAttributeValue::new( + Annotated( + Some(OurLogAttributeType::Unknown("unknown".to_string())), + ty_meta, + ), + Annotated(Some(Value::String("".to_string())), value_meta), + ) + } + (_, value) => { + value_meta.add_error(ErrorKind::InvalidData); + value_meta.set_original_value(Some(value.clone())); + OurLogAttributeValue::new( + Annotated( + Some(OurLogAttributeType::Unknown("unknown".to_string())), + ty_meta, + ), + Annotated(Some(Value::String("".to_string())), value_meta), + ) + } + } + } + (mut ty, mut value) => { + ty.meta_mut().add_error(ErrorKind::MissingAttribute); + value.meta_mut().add_error(ErrorKind::MissingAttribute); + OurLogAttributeValue { ty, value } + } + }; + } + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -119,7 +204,10 @@ mod tests { use crate::services::processor::ProcessingGroup; use crate::utils::ManagedEnvelope; use bytes::Bytes; + use relay_dynamic_config::GlobalConfig; + use relay_event_schema::protocol::OurLog; + use relay_protocol::Annotated; use relay_system::Addr; @@ -205,4 +293,224 @@ mod tests { managed_envelope.envelope() ); } + + #[test] + fn test_process_attribute_types() { + let json = r#"{ + "timestamp": 1544719860.0, + "trace_id": "5b8efff798038103d269b633813fc60c", + "span_id": "eee19b7ec3c1b174", + "level": "info", + "body": "Test log message", + "attributes": { + "valid_bool": { + "type": "bool", + "value": true + }, + "valid_int_i64": { + "type": "int", + "value": -42 + }, + "valid_int_u64": { + "type": "int", + "value": 42 + }, + "valid_int_from_string": { + "type": "int", + "value": "42" + }, + "valid_double": { + "type": "double", + "value": 42.5 + }, + "valid_double_with_i64": { + "type": "double", + "value": -42 + }, + "valid_double_with_u64": { + "type": "double", + "value": 42 + }, + "valid_string": { + "type": "string", + "value": "test" + }, + "unknown_type": { + "type": "custom", + "value": "test" + }, + "invalid_int_from_invalid_string": { + "type": "int", + "value": "abc" + }, + "missing_type": { + "value": "value with missing type" + }, + "missing_value": { + "type": "string" + } + } + }"#; + + let mut data = Annotated::::from_json(json).unwrap(); + + if let Some(log) = data.value_mut() { + process_attribute_types(log); + } + + insta::assert_debug_snapshot!(data.value().unwrap().attributes, @r###" + { + "invalid_int_from_invalid_string": OurLogAttribute { + value: Annotated( + String( + "", + ), + Meta { + remarks: [], + errors: [ + Error { + kind: InvalidData, + data: {}, + }, + ], + original_length: None, + original_value: Some( + String( + "abc", + ), + ), + }, + ), + type: "unknown", + }, + "missing_type": OurLogAttribute { + value: Annotated( + String( + "value with missing type", + ), + Meta { + remarks: [], + errors: [ + Error { + kind: MissingAttribute, + data: {}, + }, + ], + original_length: None, + original_value: None, + }, + ), + type: Meta { + remarks: [], + errors: [ + Error { + kind: MissingAttribute, + data: {}, + }, + ], + original_length: None, + original_value: None, + }, + }, + "missing_value": OurLogAttribute { + value: Meta { + remarks: [], + errors: [ + Error { + kind: MissingAttribute, + data: {}, + }, + ], + original_length: None, + original_value: None, + }, + type: Annotated( + "string", + Meta { + remarks: [], + errors: [ + Error { + kind: MissingAttribute, + data: {}, + }, + ], + original_length: None, + original_value: None, + }, + ), + }, + "unknown_type": OurLogAttribute { + value: Annotated( + String( + "", + ), + Meta { + remarks: [], + errors: [ + Error { + kind: InvalidData, + data: {}, + }, + ], + original_length: None, + original_value: Some( + String( + "test", + ), + ), + }, + ), + type: "unknown", + }, + "valid_bool": OurLogAttribute { + value: Bool( + true, + ), + type: "bool", + }, + "valid_double": OurLogAttribute { + value: F64( + 42.5, + ), + type: "double", + }, + "valid_double_with_i64": OurLogAttribute { + value: I64( + -42, + ), + type: "double", + }, + "valid_double_with_u64": OurLogAttribute { + value: I64( + 42, + ), + type: "double", + }, + "valid_int_from_string": OurLogAttribute { + value: String( + "42", + ), + type: "int", + }, + "valid_int_i64": OurLogAttribute { + value: I64( + -42, + ), + type: "int", + }, + "valid_int_u64": OurLogAttribute { + value: I64( + 42, + ), + type: "int", + }, + "valid_string": OurLogAttribute { + value: String( + "test", + ), + type: "string", + }, + } + "###); + } } diff --git a/relay-server/src/services/store.rs b/relay-server/src/services/store.rs index 2ee5f0d022e..9c37054179f 100644 --- a/relay-server/src/services/store.rs +++ b/relay-server/src/services/store.rs @@ -1125,8 +1125,8 @@ where .serialize(serializer) } -pub fn serialize_btreemap_skip_nulls( - map: &Option>>, +fn serialize_btreemap_skip_nulls( + map: &Option>>, serializer: S, ) -> Result where @@ -1144,6 +1144,28 @@ where m.end() } +fn serialize_log_attributes( + map: &Option>>, + serializer: S, +) -> Result +where + S: serde::Serializer, +{ + let Some(map) = map else { + return serializer.serialize_none(); + }; + let mut m = serializer.serialize_map(Some(map.len()))?; + for (key, value) in map.iter() { + if let Some(value) = value { + if let Some(LogAttributeValue::Unknown(_)) = value.value { + continue; + } + m.serialize_entry(key, value)?; + } + } + m.end() +} + /** * This shouldn't be necessary with enum serialization, but since serde's tag doesn't work with `type` as it has to be renamed due to being a keyword, * this allows us to not emit the 'type' field, which would otherwise break the ourlogs consumer. @@ -1159,6 +1181,10 @@ where return serializer.serialize_none(); }; + if let LogAttributeValue::Unknown(_) = attr { + return serializer.serialize_none(); + } + let mut map = serializer.serialize_map(Some(1))?; match attr { LogAttributeValue::String(value) => { @@ -1170,9 +1196,10 @@ where LogAttributeValue::Bool(value) => { map.serialize_entry("bool_value", value)?; } - LogAttributeValue::Float(value) => { - map.serialize_entry("float_value", value)?; + LogAttributeValue::Double(value) => { + map.serialize_entry("double_value", value)?; } + LogAttributeValue::Unknown(_) => (), } map.end() } @@ -1437,8 +1464,10 @@ enum LogAttributeValue { Bool(bool), #[serde(rename = "int")] Int(i64), - #[serde(rename = "float")] - Float(f64), + #[serde(rename = "double")] + Double(f64), + #[serde(rename = "unknown")] + Unknown(String), } /// This is a temporary struct to convert the old attribute format to the new one. @@ -1475,7 +1504,7 @@ struct LogKafkaMessage<'a> { #[serde( default, skip_serializing_if = "Option::is_none", - serialize_with = "serialize_btreemap_skip_nulls" + serialize_with = "serialize_log_attributes" )] attributes: Option>>, #[serde(default, skip_serializing_if = "Option::is_none")] diff --git a/tests/integration/test_ourlogs.py b/tests/integration/test_ourlogs.py index 5dfa0456261..3f51d4bf940 100644 --- a/tests/integration/test_ourlogs.py +++ b/tests/integration/test_ourlogs.py @@ -147,6 +147,8 @@ def test_ourlog_extraction_with_sentry_logs( "boolean.attribute": {"value": True, "type": "bool"}, "pii": {"value": "4242 4242 4242 4242", "type": "string"}, "sentry.severity_text": {"value": "info", "type": "string"}, + "unknown_type": {"value": "info", "type": "unknown"}, + "broken_type": {"value": "info", "type": "not_a_real_type"}, }, } ) From 502dd196e8053f719b337d4aa032e6ade6b904c8 Mon Sep 17 00:00:00 2001 From: Kev <6111995+k-fish@users.noreply.github.com> Date: Mon, 7 Apr 2025 10:07:18 -0400 Subject: [PATCH 06/21] Update relay-event-schema/src/protocol/ourlog.rs Co-authored-by: Michi Hoffmann --- relay-event-schema/src/protocol/ourlog.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 50444bdd392..202d46201a3 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -112,7 +112,7 @@ pub enum OurLogAttributeType { impl OurLogAttributeType { fn as_str(&self) -> &str { match self { - Self::Bool => "bool", + Self::Bool => "boolean", Self::Int => "int", Self::Double => "double", Self::String => "string", From 981b73ea27c221071bf1d32aef769d733d9a40b6 Mon Sep 17 00:00:00 2001 From: Kev <6111995+k-fish@users.noreply.github.com> Date: Mon, 7 Apr 2025 10:07:24 -0400 Subject: [PATCH 07/21] Update relay-event-schema/src/protocol/ourlog.rs Co-authored-by: Michi Hoffmann --- relay-event-schema/src/protocol/ourlog.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 202d46201a3..e8ef175ac37 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -113,7 +113,7 @@ impl OurLogAttributeType { fn as_str(&self) -> &str { match self { Self::Bool => "boolean", - Self::Int => "int", + Self::Int => "integer", Self::Double => "double", Self::String => "string", Self::Unknown(value) => value, From a3378de3a43a7cf195f2007a930bd85c582791e7 Mon Sep 17 00:00:00 2001 From: Kev Date: Mon, 7 Apr 2025 10:11:50 -0400 Subject: [PATCH 08/21] Fix tests --- relay-event-schema/src/protocol/ourlog.rs | 8 +-- relay-ourlogs/src/ourlog.rs | 68 +++++++------------ relay-server/src/services/processor/ourlog.rs | 7 +- 3 files changed, 32 insertions(+), 51 deletions(-) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index e8ef175ac37..6dbf47c93a6 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -130,7 +130,7 @@ impl fmt::Display for OurLogAttributeType { impl From for OurLogAttributeType { fn from(value: String) -> Self { match value.as_str() { - "bool" => Self::Bool, + "boolean" => Self::Bool, "int" => Self::Int, "double" => Self::Double, "string" => Self::String, @@ -296,7 +296,7 @@ mod tests { "attributes": { "boolean.attribute": { "value": true, - "type": "bool" + "type": "boolean" }, "sentry.severity_text": { "value": "info", @@ -336,7 +336,7 @@ mod tests { value: Bool( true, ), - type: "bool", + type: "boolean", }, "sentry.observed_timestamp_nanos": OurLogAttribute { value: String( @@ -376,7 +376,7 @@ mod tests { "body": "Example log record", "attributes": { "boolean.attribute": { - "type": "bool", + "type": "boolean", "value": true }, "sentry.observed_timestamp_nanos": { diff --git a/relay-ourlogs/src/ourlog.rs b/relay-ourlogs/src/ourlog.rs index 46ff8efe550..6baa7953dbb 100644 --- a/relay-ourlogs/src/ourlog.rs +++ b/relay-ourlogs/src/ourlog.rs @@ -533,31 +533,22 @@ mod tests { body: "Example log record", attributes: { "foo": OurLogAttribute { - value: OurLogAttributeValue { - ty: "string", - value: String( - "9", - ), - }, - other: {}, + value: String( + "9", + ), + type: "string", }, "sentry.severity_number": OurLogAttribute { - value: OurLogAttributeValue { - ty: "int", - value: I64( - 9, - ), - }, - other: {}, + value: I64( + 9, + ), + type: "integer", }, "sentry.severity_text": OurLogAttribute { - value: OurLogAttributeValue { - ty: "string", - value: String( - "info", - ), - }, - other: {}, + value: String( + "info", + ), + type: "string", }, }, other: { @@ -615,31 +606,22 @@ mod tests { body: ~, attributes: { "foo": OurLogAttribute { - value: OurLogAttributeValue { - ty: "string", - value: String( - "9", - ), - }, - other: {}, + value: String( + "9", + ), + type: "string", }, "sentry.severity_number": OurLogAttribute { - value: OurLogAttributeValue { - ty: "int", - value: I64( - 25, - ), - }, - other: {}, + value: I64( + 25, + ), + type: "integer", }, "sentry.severity_text": OurLogAttribute { - value: OurLogAttributeValue { - ty: "string", - value: String( - "info", - ), - }, - other: {}, + value: String( + "info", + ), + type: "string", }, }, other: { @@ -668,7 +650,7 @@ mod tests { "value": "9" }, "sentry.severity_number": { - "type": "int", + "type": "integer", "value": 25 }, "sentry.severity_text": { diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index f27638a7813..44d3e6b0998 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -206,8 +206,6 @@ mod tests { use bytes::Bytes; use relay_dynamic_config::GlobalConfig; - use relay_event_schema::protocol::OurLog; - use relay_protocol::Annotated; use relay_system::Addr; @@ -294,6 +292,7 @@ mod tests { ); } + #[cfg(feature = "processing")] #[test] fn test_process_attribute_types() { let json = r#"{ @@ -304,7 +303,7 @@ mod tests { "body": "Test log message", "attributes": { "valid_bool": { - "type": "bool", + "type": "boolean", "value": true }, "valid_int_i64": { @@ -466,7 +465,7 @@ mod tests { value: Bool( true, ), - type: "bool", + type: "boolean", }, "valid_double": OurLogAttribute { value: F64( From e2c32e0f56300e6054d32e8b24459bd87c616384 Mon Sep 17 00:00:00 2001 From: Kev Date: Mon, 7 Apr 2025 10:54:45 -0400 Subject: [PATCH 09/21] Fix types to match spec --- relay-event-schema/src/protocol/ourlog.rs | 24 +++++++++---------- relay-server/src/services/processor/ourlog.rs | 16 ++++++------- relay-server/src/services/store.rs | 4 ++-- tests/integration/test_ourlogs.py | 2 +- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 6dbf47c93a6..0c6999f6197 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -131,7 +131,7 @@ impl From for OurLogAttributeType { fn from(value: String) -> Self { match value.as_str() { "boolean" => Self::Bool, - "int" => Self::Int, + "integer" => Self::Int, "double" => Self::Double, "string" => Self::String, _ => Self::Unknown(value), @@ -304,15 +304,15 @@ mod tests { }, "sentry.severity_number": { "value": "10", - "type": "int" + "type": "integer" }, "sentry.observed_timestamp_nanos": { "value": "1544712660300000000", - "type": "int" + "type": "integer" }, "sentry.trace_flags": { "value": "10", - "type": "int" + "type": "integer" } } }"#; @@ -342,13 +342,13 @@ mod tests { value: String( "1544712660300000000", ), - type: "int", + type: "integer", }, "sentry.severity_number": OurLogAttribute { value: String( "10", ), - type: "int", + type: "integer", }, "sentry.severity_text": OurLogAttribute { value: String( @@ -360,7 +360,7 @@ mod tests { value: String( "10", ), - type: "int", + type: "integer", }, }, other: {}, @@ -380,11 +380,11 @@ mod tests { "value": true }, "sentry.observed_timestamp_nanos": { - "type": "int", + "type": "integer", "value": "1544712660300000000" }, "sentry.severity_number": { - "type": "int", + "type": "integer", "value": "10" }, "sentry.severity_text": { @@ -392,7 +392,7 @@ mod tests { "value": "info" }, "sentry.trace_flags": { - "type": "int", + "type": "integer", "value": "10" } } @@ -411,7 +411,7 @@ mod tests { "attributes": { "sentry.severity_number": { "value": 10, - "type": "int" + "type": "integer" } } }"#; @@ -427,7 +427,7 @@ mod tests { "body": "Example log record", "attributes": { "sentry.severity_number": { - "type": "int", + "type": "integer", "value": 10 } } diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index 44d3e6b0998..8c1e758ba9c 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -141,7 +141,7 @@ fn process_attribute_types(ourlog: &mut OurLog) { (OurLogAttributeType::Int, Value::I64(_)) => attr.value.clone(), (OurLogAttributeType::Int, Value::U64(_)) => attr.value.clone(), (OurLogAttributeType::Int, Value::String(v)) => { - if v.parse::().is_ok() { + if v.parse::().is_ok() || v.parse::().is_ok() { attr.value.clone() } else { value_meta.add_error(ErrorKind::InvalidData); @@ -307,15 +307,15 @@ mod tests { "value": true }, "valid_int_i64": { - "type": "int", + "type": "integer", "value": -42 }, "valid_int_u64": { - "type": "int", + "type": "integer", "value": 42 }, "valid_int_from_string": { - "type": "int", + "type": "integer", "value": "42" }, "valid_double": { @@ -339,7 +339,7 @@ mod tests { "value": "test" }, "invalid_int_from_invalid_string": { - "type": "int", + "type": "integer", "value": "abc" }, "missing_type": { @@ -489,19 +489,19 @@ mod tests { value: String( "42", ), - type: "int", + type: "integer", }, "valid_int_i64": OurLogAttribute { value: I64( -42, ), - type: "int", + type: "integer", }, "valid_int_u64": OurLogAttribute { value: I64( 42, ), - type: "int", + type: "integer", }, "valid_string": OurLogAttribute { value: String( diff --git a/relay-server/src/services/store.rs b/relay-server/src/services/store.rs index 9c37054179f..ffb29d0733a 100644 --- a/relay-server/src/services/store.rs +++ b/relay-server/src/services/store.rs @@ -1460,9 +1460,9 @@ struct SpanKafkaMessage<'a> { enum LogAttributeValue { #[serde(rename = "string")] String(String), - #[serde(rename = "bool")] + #[serde(rename = "boolean")] Bool(bool), - #[serde(rename = "int")] + #[serde(rename = "integer")] Int(i64), #[serde(rename = "double")] Double(f64), diff --git a/tests/integration/test_ourlogs.py b/tests/integration/test_ourlogs.py index 3f51d4bf940..3a4b13dd219 100644 --- a/tests/integration/test_ourlogs.py +++ b/tests/integration/test_ourlogs.py @@ -144,7 +144,7 @@ def test_ourlog_extraction_with_sentry_logs( "body": "Example log record", "severity_number": 10, "attributes": { - "boolean.attribute": {"value": True, "type": "bool"}, + "boolean.attribute": {"value": True, "type": "boolean"}, "pii": {"value": "4242 4242 4242 4242", "type": "string"}, "sentry.severity_text": {"value": "info", "type": "string"}, "unknown_type": {"value": "info", "type": "unknown"}, From d853843e21d521ba67605624d94929155f3edaa7 Mon Sep 17 00:00:00 2001 From: Kev Date: Tue, 8 Apr 2025 11:07:22 -0400 Subject: [PATCH 10/21] Fill out the rest of the types in tests and check for forward compatibility (allow other to be sent but not emit it to kafka) --- relay-event-schema/src/protocol/ourlog.rs | 8 ++++++++ relay-server/src/services/processor/ourlog.rs | 11 +++++++++++ tests/integration/test_ourlogs.py | 12 ++++++++++++ 3 files changed, 31 insertions(+) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 0c6999f6197..74d38ab7f6f 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -298,6 +298,14 @@ mod tests { "value": true, "type": "boolean" }, + "double.attribute": { + "value": 1.23, + "type": "double" + }, + "string.attribute": { + "value": "some string", + "type": "string" + }, "sentry.severity_text": { "value": "info", "type": "string" diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index 8c1e758ba9c..c2cc9a6cbde 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -334,6 +334,11 @@ mod tests { "type": "string", "value": "test" }, + "valid_string_with_other": { + "type": "string", + "value": "test", + "some_other_field": "some_other_value" + }, "unknown_type": { "type": "custom", "value": "test" @@ -509,6 +514,12 @@ mod tests { ), type: "string", }, + "valid_string_with_other": OurLogAttribute { + value: String( + "test", + ), + type: "string", + }, } "###); } diff --git a/tests/integration/test_ourlogs.py b/tests/integration/test_ourlogs.py index 3a4b13dd219..a91e68494ff 100644 --- a/tests/integration/test_ourlogs.py +++ b/tests/integration/test_ourlogs.py @@ -145,10 +145,18 @@ def test_ourlog_extraction_with_sentry_logs( "severity_number": 10, "attributes": { "boolean.attribute": {"value": True, "type": "boolean"}, + "integer.attribute": {"value": 42, "type": "integer"}, + "double.attribute": {"value": 1.23, "type": "double"}, + "string.attribute": {"value": "some string", "type": "string"}, "pii": {"value": "4242 4242 4242 4242", "type": "string"}, "sentry.severity_text": {"value": "info", "type": "string"}, "unknown_type": {"value": "info", "type": "unknown"}, "broken_type": {"value": "info", "type": "not_a_real_type"}, + "valid_string_with_other": { + "value": "test", + "type": "string", + "some_other_field": "some_other_value", + }, }, } ) @@ -169,6 +177,10 @@ def test_ourlog_extraction_with_sentry_logs( "severity_number": 9, "attributes": { "boolean.attribute": {"bool_value": True}, + "double.attribute": {"double_value": 1.23}, + "integer.attribute": {"int_value": 42}, + "string.attribute": {"string_value": "some string"}, + "valid_string_with_other": {"string_value": "test"}, "pii": {"string_value": "[creditcard]"}, "sentry.severity_number": {"int_value": 9}, "sentry.severity_text": {"string_value": "info"}, From c5e053973a45acbf706e93ca240c61e39bf190a7 Mon Sep 17 00:00:00 2001 From: Kev Date: Tue, 8 Apr 2025 11:11:55 -0400 Subject: [PATCH 11/21] Include 'other' in debug --- relay-event-schema/src/protocol/ourlog.rs | 28 +++++++++++++++++++ relay-server/src/services/processor/ourlog.rs | 17 +++++++++++ 2 files changed, 45 insertions(+) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 74d38ab7f6f..1312708c67c 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -79,6 +79,7 @@ impl fmt::Debug for OurLogAttribute { f.debug_struct("OurLogAttribute") .field("value", &self.value.value) .field("type", &self.value.ty) + .field("other", &self.other) .finish() } } @@ -345,30 +346,49 @@ mod tests { true, ), type: "boolean", + other: {}, + }, + "double.attribute": OurLogAttribute { + value: F64( + 1.23, + ), + type: "double", + other: {}, }, "sentry.observed_timestamp_nanos": OurLogAttribute { value: String( "1544712660300000000", ), type: "integer", + other: {}, }, "sentry.severity_number": OurLogAttribute { value: String( "10", ), type: "integer", + other: {}, }, "sentry.severity_text": OurLogAttribute { value: String( "info", ), type: "string", + other: {}, }, "sentry.trace_flags": OurLogAttribute { value: String( "10", ), type: "integer", + other: {}, + }, + "string.attribute": OurLogAttribute { + value: String( + "some string", + ), + type: "string", + other: {}, }, }, other: {}, @@ -387,6 +407,10 @@ mod tests { "type": "boolean", "value": true }, + "double.attribute": { + "type": "double", + "value": 1.23 + }, "sentry.observed_timestamp_nanos": { "type": "integer", "value": "1544712660300000000" @@ -402,6 +426,10 @@ mod tests { "sentry.trace_flags": { "type": "integer", "value": "10" + }, + "string.attribute": { + "type": "string", + "value": "some string" } } } diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index c2cc9a6cbde..ff31c9cbe87 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -386,6 +386,7 @@ mod tests { }, ), type: "unknown", + other: {}, }, "missing_type": OurLogAttribute { value: Annotated( @@ -415,6 +416,7 @@ mod tests { original_length: None, original_value: None, }, + other: {}, }, "missing_value": OurLogAttribute { value: Meta { @@ -442,6 +444,7 @@ mod tests { original_value: None, }, ), + other: {}, }, "unknown_type": OurLogAttribute { value: Annotated( @@ -465,60 +468,74 @@ mod tests { }, ), type: "unknown", + other: {}, }, "valid_bool": OurLogAttribute { value: Bool( true, ), type: "boolean", + other: {}, }, "valid_double": OurLogAttribute { value: F64( 42.5, ), type: "double", + other: {}, }, "valid_double_with_i64": OurLogAttribute { value: I64( -42, ), type: "double", + other: {}, }, "valid_double_with_u64": OurLogAttribute { value: I64( 42, ), type: "double", + other: {}, }, "valid_int_from_string": OurLogAttribute { value: String( "42", ), type: "integer", + other: {}, }, "valid_int_i64": OurLogAttribute { value: I64( -42, ), type: "integer", + other: {}, }, "valid_int_u64": OurLogAttribute { value: I64( 42, ), type: "integer", + other: {}, }, "valid_string": OurLogAttribute { value: String( "test", ), type: "string", + other: {}, }, "valid_string_with_other": OurLogAttribute { value: String( "test", ), type: "string", + other: { + "some_other_field": String( + "some_other_value", + ), + }, }, } "###); From 1d949ef3a83280e0e3e54484536895500c7a64ef Mon Sep 17 00:00:00 2001 From: Kev Date: Wed, 9 Apr 2025 09:43:19 -0400 Subject: [PATCH 12/21] Fix test snapshot --- relay-ourlogs/src/ourlog.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/relay-ourlogs/src/ourlog.rs b/relay-ourlogs/src/ourlog.rs index 6baa7953dbb..c4af63c5cad 100644 --- a/relay-ourlogs/src/ourlog.rs +++ b/relay-ourlogs/src/ourlog.rs @@ -537,18 +537,21 @@ mod tests { "9", ), type: "string", + other: {}, }, "sentry.severity_number": OurLogAttribute { value: I64( 9, ), type: "integer", + other: {}, }, "sentry.severity_text": OurLogAttribute { value: String( "info", ), type: "string", + other: {}, }, }, other: { @@ -610,18 +613,21 @@ mod tests { "9", ), type: "string", + other: {}, }, "sentry.severity_number": OurLogAttribute { value: I64( 25, ), type: "integer", + other: {}, }, "sentry.severity_text": OurLogAttribute { value: String( "info", ), type: "string", + other: {}, }, }, other: { From 04c45fd9a66b8732b56a9c6062cbb2b21079a218 Mon Sep 17 00:00:00 2001 From: Kev Date: Wed, 9 Apr 2025 11:26:28 -0400 Subject: [PATCH 13/21] process attributes without clone --- relay-event-schema/src/protocol/ourlog.rs | 2 +- relay-server/src/services/processor/ourlog.rs | 98 +++++++++---------- 2 files changed, 45 insertions(+), 55 deletions(-) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 1312708c67c..15f4e03013b 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -111,7 +111,7 @@ pub enum OurLogAttributeType { } impl OurLogAttributeType { - fn as_str(&self) -> &str { + pub fn as_str(&self) -> &str { match self { Self::Bool => "boolean", Self::Int => "integer", diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index ff31c9cbe87..b43e7019218 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -18,7 +18,7 @@ use { crate::services::processor::ProcessingError, relay_dynamic_config::ProjectConfig, relay_event_schema::processor::{process_value, ProcessingState}, - relay_event_schema::protocol::{OurLog, OurLogAttributeType, OurLogAttributeValue}, + relay_event_schema::protocol::{OurLog, OurLogAttributeType}, relay_ourlogs::OtelLog, relay_pii::PiiProcessor, relay_protocol::{Annotated, ErrorKind, Value}, @@ -133,65 +133,55 @@ fn process_attribute_types(ourlog: &mut OurLog) { if let Some(data) = ourlog.attributes.value_mut() { for (_, attr) in data.iter_mut() { if let Some(attr) = attr.value_mut() { - attr.value = match (attr.value.ty.clone(), attr.value.value.clone()) { - (Annotated(Some(ty), ty_meta), Annotated(Some(value), mut value_meta)) => { - let attr_type = OurLogAttributeType::from(ty); - match (attr_type, value) { - (OurLogAttributeType::Bool, Value::Bool(_)) => attr.value.clone(), - (OurLogAttributeType::Int, Value::I64(_)) => attr.value.clone(), - (OurLogAttributeType::Int, Value::U64(_)) => attr.value.clone(), + match (&mut attr.value.ty, &mut attr.value.value) { + ( + Annotated(Some(ty), _), + Annotated(Some(ref mut value), ref mut value_meta), + ) => { + let attr_type = OurLogAttributeType::from(ty.clone()); + let should_keep = match (attr_type, &*value) { + (OurLogAttributeType::Bool, Value::Bool(_)) => true, + (OurLogAttributeType::Int, Value::I64(_)) => true, + (OurLogAttributeType::Int, Value::U64(_)) => true, (OurLogAttributeType::Int, Value::String(v)) => { - if v.parse::().is_ok() || v.parse::().is_ok() { - attr.value.clone() - } else { - value_meta.add_error(ErrorKind::InvalidData); - value_meta.set_original_value(Some(v.clone())); - OurLogAttributeValue::new( - Annotated( - Some(OurLogAttributeType::Unknown( - "unknown".to_string(), - )), - ty_meta, - ), - Annotated(Some(Value::String("".to_string())), value_meta), - ) - } + v.parse::().is_ok() || v.parse::().is_ok() } - (OurLogAttributeType::Double, Value::F64(_)) => attr.value.clone(), - (OurLogAttributeType::Double, Value::I64(_)) => attr.value.clone(), - (OurLogAttributeType::Double, Value::U64(_)) => attr.value.clone(), - (OurLogAttributeType::Double, Value::String(_)) => attr.value.clone(), - (OurLogAttributeType::String, Value::String(_)) => attr.value.clone(), - (_ty @ OurLogAttributeType::Unknown(_), value) => { - value_meta.add_error(ErrorKind::InvalidData); - value_meta.set_original_value(Some(value.clone())); - OurLogAttributeValue::new( - Annotated( - Some(OurLogAttributeType::Unknown("unknown".to_string())), - ty_meta, - ), - Annotated(Some(Value::String("".to_string())), value_meta), - ) - } - (_, value) => { - value_meta.add_error(ErrorKind::InvalidData); - value_meta.set_original_value(Some(value.clone())); - OurLogAttributeValue::new( - Annotated( - Some(OurLogAttributeType::Unknown("unknown".to_string())), - ty_meta, - ), - Annotated(Some(Value::String("".to_string())), value_meta), - ) + (OurLogAttributeType::Double, Value::F64(_)) => true, + (OurLogAttributeType::Double, Value::String(f)) => { + f.parse::().is_ok() } + (OurLogAttributeType::String, Value::String(_)) => true, + _ => false, + }; + if !should_keep { + value_meta.add_error(ErrorKind::InvalidData); + value_meta.set_original_value(Some(std::mem::replace( + value, + Value::String("".to_string()), + ))); + + attr.value.ty = Annotated::new( + OurLogAttributeType::Unknown("unknown".to_string()) + .as_str() + .to_string(), + ); } } - (mut ty, mut value) => { - ty.meta_mut().add_error(ErrorKind::MissingAttribute); - value.meta_mut().add_error(ErrorKind::MissingAttribute); - OurLogAttributeValue { ty, value } + (Annotated(ty_opt, ty_meta), Annotated(value_opt, value_meta)) => { + if ty_opt.is_none() { + ty_meta.add_error(ErrorKind::MissingAttribute); + } + if value_opt.is_none() { + value_meta.add_error(ErrorKind::MissingAttribute); + } + attr.value.ty = Annotated::new( + OurLogAttributeType::Unknown("unknown".to_string()) + .as_str() + .to_string(), + ); + *value_opt = Some(Value::String("".to_string())); } - }; + } } } } From 77cbb016e27ddc979c6ddefa3f758152e9a9cab6 Mon Sep 17 00:00:00 2001 From: Kev Date: Wed, 9 Apr 2025 11:44:54 -0400 Subject: [PATCH 14/21] Again without using clone --- relay-event-schema/src/protocol/ourlog.rs | 4 ++ relay-server/src/services/processor/ourlog.rs | 64 +++++++++++-------- 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 15f4e03013b..312dd80ff5c 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -120,6 +120,10 @@ impl OurLogAttributeType { Self::Unknown(value) => value, } } + + pub fn unknown_string() -> String { + "unknown".to_string() + } } impl fmt::Display for OurLogAttributeType { diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index b43e7019218..ee32507907e 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -139,33 +139,47 @@ fn process_attribute_types(ourlog: &mut OurLog) { Annotated(Some(ref mut value), ref mut value_meta), ) => { let attr_type = OurLogAttributeType::from(ty.clone()); - let should_keep = match (attr_type, &*value) { - (OurLogAttributeType::Bool, Value::Bool(_)) => true, - (OurLogAttributeType::Int, Value::I64(_)) => true, - (OurLogAttributeType::Int, Value::U64(_)) => true, + match (attr_type, &*value) { + (OurLogAttributeType::Bool, Value::Bool(_)) => (), + (OurLogAttributeType::Int, Value::I64(_)) => (), (OurLogAttributeType::Int, Value::String(v)) => { - v.parse::().is_ok() || v.parse::().is_ok() + if let Ok(int_value) = v.parse::() { + *value = Value::I64(int_value); + } else { + attr.value.ty = + Annotated::new(OurLogAttributeType::unknown_string()); + value_meta.add_error(ErrorKind::InvalidData); + value_meta.set_original_value(Some(std::mem::replace( + value, + Value::String("".to_string()), + ))); + } } - (OurLogAttributeType::Double, Value::F64(_)) => true, + (OurLogAttributeType::Double, Value::F64(_)) => (), (OurLogAttributeType::Double, Value::String(f)) => { - f.parse::().is_ok() + if let Ok(double_value) = f.parse::() { + *value = Value::F64(double_value); + } else { + attr.value.ty = + Annotated::new(OurLogAttributeType::unknown_string()); + value_meta.add_error(ErrorKind::InvalidData); + value_meta.set_original_value(Some(std::mem::replace( + value, + Value::String("".to_string()), + ))); + } + } + (OurLogAttributeType::String, Value::String(_)) => (), + _ => { + attr.value.ty = + Annotated::new(OurLogAttributeType::unknown_string()); + value_meta.add_error(ErrorKind::InvalidData); + value_meta.set_original_value(Some(std::mem::replace( + value, + Value::String("".to_string()), + ))); } - (OurLogAttributeType::String, Value::String(_)) => true, - _ => false, }; - if !should_keep { - value_meta.add_error(ErrorKind::InvalidData); - value_meta.set_original_value(Some(std::mem::replace( - value, - Value::String("".to_string()), - ))); - - attr.value.ty = Annotated::new( - OurLogAttributeType::Unknown("unknown".to_string()) - .as_str() - .to_string(), - ); - } } (Annotated(ty_opt, ty_meta), Annotated(value_opt, value_meta)) => { if ty_opt.is_none() { @@ -174,11 +188,7 @@ fn process_attribute_types(ourlog: &mut OurLog) { if value_opt.is_none() { value_meta.add_error(ErrorKind::MissingAttribute); } - attr.value.ty = Annotated::new( - OurLogAttributeType::Unknown("unknown".to_string()) - .as_str() - .to_string(), - ); + *ty_opt = Some(OurLogAttributeType::unknown_string()); *value_opt = Some(Value::String("".to_string())); } } From 5591034dc9318f430b85a5ed5f3ae7a76ac222b3 Mon Sep 17 00:00:00 2001 From: Kev Date: Wed, 9 Apr 2025 12:15:13 -0400 Subject: [PATCH 15/21] u64 i64 for f64 --- relay-server/src/services/processor/ourlog.rs | 64 +++++++------------ 1 file changed, 24 insertions(+), 40 deletions(-) diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index ee32507907e..765244fc488 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -155,9 +155,11 @@ fn process_attribute_types(ourlog: &mut OurLog) { ))); } } + (OurLogAttributeType::Double, Value::I64(_)) => (), + (OurLogAttributeType::Double, Value::U64(_)) => (), (OurLogAttributeType::Double, Value::F64(_)) => (), - (OurLogAttributeType::Double, Value::String(f)) => { - if let Ok(double_value) = f.parse::() { + (OurLogAttributeType::Double, Value::String(v)) => { + if let Ok(double_value) = v.parse::() { *value = Value::F64(double_value); } else { attr.value.ty = @@ -322,7 +324,7 @@ mod tests { "type": "double", "value": 42.5 }, - "valid_double_with_i64": { + "double_with_i64": { "type": "double", "value": -42 }, @@ -364,6 +366,13 @@ mod tests { insta::assert_debug_snapshot!(data.value().unwrap().attributes, @r###" { + "double_with_i64": OurLogAttribute { + value: I64( + -42, + ), + type: "double", + other: {}, + }, "invalid_int_from_invalid_string": OurLogAttribute { value: Annotated( String( @@ -389,10 +398,11 @@ mod tests { other: {}, }, "missing_type": OurLogAttribute { - value: Annotated( - String( - "value with missing type", - ), + value: String( + "", + ), + type: Annotated( + "unknown", Meta { remarks: [], errors: [ @@ -405,33 +415,13 @@ mod tests { original_value: None, }, ), - type: Meta { - remarks: [], - errors: [ - Error { - kind: MissingAttribute, - data: {}, - }, - ], - original_length: None, - original_value: None, - }, other: {}, }, "missing_value": OurLogAttribute { - value: Meta { - remarks: [], - errors: [ - Error { - kind: MissingAttribute, - data: {}, - }, - ], - original_length: None, - original_value: None, - }, - type: Annotated( - "string", + value: Annotated( + String( + "", + ), Meta { remarks: [], errors: [ @@ -444,6 +434,7 @@ mod tests { original_value: None, }, ), + type: "unknown", other: {}, }, "unknown_type": OurLogAttribute { @@ -484,13 +475,6 @@ mod tests { type: "double", other: {}, }, - "valid_double_with_i64": OurLogAttribute { - value: I64( - -42, - ), - type: "double", - other: {}, - }, "valid_double_with_u64": OurLogAttribute { value: I64( 42, @@ -499,8 +483,8 @@ mod tests { other: {}, }, "valid_int_from_string": OurLogAttribute { - value: String( - "42", + value: I64( + 42, ), type: "integer", other: {}, From defad3199ac2c28321fc7d604da77e62416973c7 Mon Sep 17 00:00:00 2001 From: David Herberth Date: Thu, 10 Apr 2025 17:51:16 +0200 Subject: [PATCH 16/21] minor clean ups --- CHANGELOG.md | 2 +- relay-event-schema/src/protocol/ourlog.rs | 82 ++---- relay-ourlogs/src/ourlog.rs | 22 +- relay-server/src/services/processor/ourlog.rs | 275 ++++++++---------- 4 files changed, 163 insertions(+), 218 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48cc7822796..3219f0d4b00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,7 @@ - Bump the revision of `sysinfo` to the revision at `15b3be3273ba286740122fed7bb7dccd2a79dc8f`. ([#4613](https://github.com/getsentry/relay/pull/4613)) - Switch the processor and store to `async`. ([#4552](https://github.com/getsentry/relay/pull/4552)) - Validate the spooling memory configuration on startup. ([#4617](https://github.com/getsentry/relay/pull/4617)) -- Adjust 'log' protocol / envelope item type per sdk feedback ([#4592](https://github.com/getsentry/relay/pull/4592)) +- Rework currently unused 'log' protocol / envelope item type schema. ([#4592](https://github.com/getsentry/relay/pull/4592)) - Improve descriptiveness of autoscaling metric name. ([#4629](https://github.com/getsentry/relay/pull/4629)) ## 25.3.0 diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 312dd80ff5c..9b917912e38 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -1,8 +1,5 @@ -use relay_protocol::{ - Annotated, Empty, Error, FromValue, IntoValue, Object, SkipSerialization, Value, -}; +use relay_protocol::{Annotated, Empty, FromValue, IntoValue, Object, SkipSerialization, Value}; use std::fmt::{self, Display}; -use std::str::FromStr; use serde::{Serialize, Serializer}; @@ -20,7 +17,7 @@ pub struct OurLog { #[metastructure(required = true, trim = false)] pub trace_id: Annotated, - /// The Span id. + /// The Span this log entry belongs to. #[metastructure(required = false, trim = false)] pub span_id: Annotated, @@ -37,7 +34,7 @@ pub struct OurLog { pub attributes: Annotated>, /// Additional arbitrary fields for forwards compatibility. - #[metastructure(additional_properties, retain = true, pii = "maybe", trim = false)] + #[metastructure(additional_properties, retain = true, pii = "maybe")] pub other: Object, } @@ -68,7 +65,10 @@ pub struct OurLogAttribute { impl OurLogAttribute { pub fn new(attribute_type: OurLogAttributeType, value: Value) -> Self { Self { - value: OurLogAttributeValue::new(Annotated::new(attribute_type), Annotated::new(value)), + value: OurLogAttributeValue { + ty: Annotated::new(attribute_type), + value: Annotated::new(value), + }, other: Object::new(), } } @@ -87,34 +87,27 @@ impl fmt::Debug for OurLogAttribute { #[derive(Debug, Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] pub struct OurLogAttributeValue { #[metastructure(field = "type", required = true, trim = false)] - pub ty: Annotated, + pub ty: Annotated, #[metastructure(required = true, pii = "true")] pub value: Annotated, } -impl OurLogAttributeValue { - pub fn new(attribute_type: Annotated, value: Annotated) -> Self { - Self { - ty: attribute_type.map_value(|at| at.as_str().to_string()), - value, - } - } -} - -#[derive(Debug)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum OurLogAttributeType { - Bool, - Int, + Boolean, + Integer, Double, String, Unknown(String), } +impl ProcessValue for OurLogAttributeType {} + impl OurLogAttributeType { pub fn as_str(&self) -> &str { match self { - Self::Bool => "boolean", - Self::Int => "integer", + Self::Boolean => "boolean", + Self::Integer => "integer", Self::Double => "double", Self::String => "string", Self::Unknown(value) => value, @@ -135,8 +128,8 @@ impl fmt::Display for OurLogAttributeType { impl From for OurLogAttributeType { fn from(value: String) -> Self { match value.as_str() { - "boolean" => Self::Bool, - "integer" => Self::Int, + "boolean" => Self::Boolean, + "integer" => Self::Integer, "double" => Self::Double, "string" => Self::String, _ => Self::Unknown(value), @@ -211,41 +204,26 @@ impl Display for OurLogLevel { write!(f, "{}", self.as_str()) } } -impl FromStr for OurLogLevel { - type Err = ParseOurLogLevelError; - fn from_str(s: &str) -> Result { - Ok(match s { +impl From for OurLogLevel { + fn from(value: String) -> Self { + match value.as_str() { "trace" => OurLogLevel::Trace, "debug" => OurLogLevel::Debug, "info" => OurLogLevel::Info, "warn" => OurLogLevel::Warn, "error" => OurLogLevel::Error, "fatal" => OurLogLevel::Fatal, - other => OurLogLevel::Unknown(other.to_owned()), - }) + _ => OurLogLevel::Unknown(value), + } } } impl FromValue for OurLogLevel { fn from_value(value: Annotated) -> Annotated { - match value { - Annotated(Some(Value::String(value)), mut meta) => { - match OurLogLevel::from_str(&value) { - Ok(value) => Annotated(Some(value), meta), - Err(err) => { - meta.add_error(Error::invalid(err)); - meta.set_original_value(Some(value)); - Annotated(None, meta) - } - } - } + match String::from_value(value) { + Annotated(Some(value), meta) => Annotated(Some(value.into()), meta), Annotated(None, meta) => Annotated(None, meta), - Annotated(Some(value), mut meta) => { - meta.add_error(Error::expected("a level")); - meta.set_original_value(Some(value)); - Annotated(None, meta) - } } } } @@ -273,18 +251,6 @@ impl Empty for OurLogLevel { } } -/// An error used when parsing `OurLogLevel`. -#[derive(Debug)] -pub struct ParseOurLogLevelError; - -impl fmt::Display for ParseOurLogLevelError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "invalid our log level") - } -} - -impl std::error::Error for ParseOurLogLevelError {} - #[cfg(test)] mod tests { use super::*; diff --git a/relay-ourlogs/src/ourlog.rs b/relay-ourlogs/src/ourlog.rs index c4af63c5cad..4b7583cad58 100644 --- a/relay-ourlogs/src/ourlog.rs +++ b/relay-ourlogs/src/ourlog.rs @@ -1,5 +1,3 @@ -use std::str::FromStr; - use chrono::{TimeZone, Utc}; use opentelemetry_proto::tonic::common::v1::any_value::Value as OtelValue; @@ -13,7 +11,7 @@ use relay_protocol::{Annotated, Object, Value}; fn otel_value_to_log_attribute(value: OtelValue) -> Option { match value { OtelValue::BoolValue(v) => Some(OurLogAttribute::new( - OurLogAttributeType::Bool, + OurLogAttributeType::Boolean, Value::Bool(v), )), OtelValue::DoubleValue(v) => Some(OurLogAttribute::new( @@ -21,7 +19,7 @@ fn otel_value_to_log_attribute(value: OtelValue) -> Option { Value::F64(v), )), OtelValue::IntValue(v) => Some(OurLogAttribute::new( - OurLogAttributeType::Int, + OurLogAttributeType::Integer, Value::I64(v), )), OtelValue::StringValue(v) => Some(OurLogAttribute::new( @@ -81,7 +79,7 @@ pub fn otel_to_sentry_log(otel_log: OtelLog) -> OurLog { attribute_data.insert( "sentry.severity_number".to_string(), Annotated::new(OurLogAttribute::new( - OurLogAttributeType::Int, + OurLogAttributeType::Integer, Value::I64(severity_number as i64), )), ); @@ -102,7 +100,7 @@ pub fn otel_to_sentry_log(otel_log: OtelLog) -> OurLog { attribute_data.insert( "sentry.trace_flags".to_string(), Annotated::new(OurLogAttribute::new( - OurLogAttributeType::Int, + OurLogAttributeType::Integer, Value::I64(0), )), ); @@ -126,7 +124,15 @@ pub fn otel_to_sentry_log(otel_log: OtelLog) -> OurLog { 13..=16 => OurLogLevel::Warn, 17..=20 => OurLogLevel::Error, 21..=24 => OurLogLevel::Fatal, - _ => OurLogLevel::from_str(&severity_text).unwrap_or(OurLogLevel::Info), + _ => match severity_text.as_str() { + "trace" => OurLogLevel::Trace, + "debug" => OurLogLevel::Debug, + "info" => OurLogLevel::Info, + "warn" => OurLogLevel::Warn, + "error" => OurLogLevel::Error, + "fatal" => OurLogLevel::Fatal, + _ => OurLogLevel::Info, + }, }; let mut other = Object::default(); @@ -171,7 +177,7 @@ pub fn ourlog_merge_otel(ourlog: Annotated) -> Annotated { attributes.insert( "sentry.severity_number".to_string(), Annotated::new(OurLogAttribute::new( - OurLogAttributeType::Int, + OurLogAttributeType::Integer, Value::I64(level_to_otel_severity_number( ourlog_value.level.value().cloned(), )), diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index 765244fc488..f222e44049a 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -130,70 +130,39 @@ fn scrub( #[cfg(feature = "processing")] fn process_attribute_types(ourlog: &mut OurLog) { - if let Some(data) = ourlog.attributes.value_mut() { - for (_, attr) in data.iter_mut() { - if let Some(attr) = attr.value_mut() { - match (&mut attr.value.ty, &mut attr.value.value) { - ( - Annotated(Some(ty), _), - Annotated(Some(ref mut value), ref mut value_meta), - ) => { - let attr_type = OurLogAttributeType::from(ty.clone()); - match (attr_type, &*value) { - (OurLogAttributeType::Bool, Value::Bool(_)) => (), - (OurLogAttributeType::Int, Value::I64(_)) => (), - (OurLogAttributeType::Int, Value::String(v)) => { - if let Ok(int_value) = v.parse::() { - *value = Value::I64(int_value); - } else { - attr.value.ty = - Annotated::new(OurLogAttributeType::unknown_string()); - value_meta.add_error(ErrorKind::InvalidData); - value_meta.set_original_value(Some(std::mem::replace( - value, - Value::String("".to_string()), - ))); - } - } - (OurLogAttributeType::Double, Value::I64(_)) => (), - (OurLogAttributeType::Double, Value::U64(_)) => (), - (OurLogAttributeType::Double, Value::F64(_)) => (), - (OurLogAttributeType::Double, Value::String(v)) => { - if let Ok(double_value) = v.parse::() { - *value = Value::F64(double_value); - } else { - attr.value.ty = - Annotated::new(OurLogAttributeType::unknown_string()); - value_meta.add_error(ErrorKind::InvalidData); - value_meta.set_original_value(Some(std::mem::replace( - value, - Value::String("".to_string()), - ))); - } - } - (OurLogAttributeType::String, Value::String(_)) => (), - _ => { - attr.value.ty = - Annotated::new(OurLogAttributeType::unknown_string()); - value_meta.add_error(ErrorKind::InvalidData); - value_meta.set_original_value(Some(std::mem::replace( - value, - Value::String("".to_string()), - ))); - } - }; - } - (Annotated(ty_opt, ty_meta), Annotated(value_opt, value_meta)) => { - if ty_opt.is_none() { - ty_meta.add_error(ErrorKind::MissingAttribute); - } - if value_opt.is_none() { - value_meta.add_error(ErrorKind::MissingAttribute); - } - *ty_opt = Some(OurLogAttributeType::unknown_string()); - *value_opt = Some(Value::String("".to_string())); - } - } + let Some(attributes) = ourlog.attributes.value_mut() else { + return; + }; + + let attributes = attributes.iter_mut().map(|(_, attr)| attr); + + for attribute in attributes { + use OurLogAttributeType::*; + + let Some(inner) = attribute.value_mut() else { + continue; + }; + + match (&mut inner.value.ty, &mut inner.value.value) { + (Annotated(Some(Boolean), _), Annotated(Some(Value::Bool(_)), _)) => (), + (Annotated(Some(Integer), _), Annotated(Some(Value::I64(_)), _)) => (), + (Annotated(Some(Integer), _), Annotated(Some(Value::U64(_)), _)) => (), + (Annotated(Some(Double), _), Annotated(Some(Value::I64(_)), _)) => (), + (Annotated(Some(Double), _), Annotated(Some(Value::U64(_)), _)) => (), + (Annotated(Some(Double), _), Annotated(Some(Value::F64(_)), _)) => (), + (Annotated(Some(String), _), Annotated(Some(Value::String(_)), _)) => (), + (Annotated(ty_opt @ Some(Unknown(_)), ty_meta), _) => { + ty_meta.add_error(ErrorKind::InvalidData); + ty_meta.set_original_value(ty_opt.take()); + } + (Annotated(Some(_), _), Annotated(value @ Some(_), value_meta)) => { + value_meta.add_error(ErrorKind::InvalidData); + value_meta.set_original_value(value.take()); + } + (Annotated(None, _), _) | (_, Annotated(None, _)) => { + let original = attribute.value_mut().take(); + attribute.meta_mut().add_error(ErrorKind::MissingAttribute); + attribute.meta_mut().set_original_value(original); } } } @@ -294,8 +263,8 @@ mod tests { ); } - #[cfg(feature = "processing")] #[test] + #[cfg(feature = "processing")] fn test_process_attribute_types() { let json = r#"{ "timestamp": 1544719860.0, @@ -370,151 +339,155 @@ mod tests { value: I64( -42, ), - type: "double", + type: Double, other: {}, }, "invalid_int_from_invalid_string": OurLogAttribute { - value: Annotated( - String( - "", - ), - Meta { - remarks: [], - errors: [ - Error { - kind: InvalidData, - data: {}, - }, - ], - original_length: None, - original_value: Some( - String( - "abc", - ), + value: Meta { + remarks: [], + errors: [ + Error { + kind: InvalidData, + data: {}, + }, + ], + original_length: None, + original_value: Some( + String( + "abc", ), - }, - ), - type: "unknown", + ), + }, + type: Integer, other: {}, }, - "missing_type": OurLogAttribute { - value: String( - "", - ), - type: Annotated( - "unknown", - Meta { - remarks: [], - errors: [ - Error { - kind: MissingAttribute, - data: {}, - }, - ], - original_length: None, - original_value: None, + "missing_type": Meta { + remarks: [], + errors: [ + Error { + kind: MissingAttribute, + data: {}, }, + ], + original_length: None, + original_value: Some( + Object( + { + "type": ~, + "value": String( + "value with missing type", + ), + }, + ), ), - other: {}, }, - "missing_value": OurLogAttribute { - value: Annotated( - String( - "", - ), - Meta { - remarks: [], - errors: [ - Error { - kind: MissingAttribute, - data: {}, - }, - ], - original_length: None, - original_value: None, + "missing_value": Meta { + remarks: [], + errors: [ + Error { + kind: MissingAttribute, + data: {}, }, + ], + original_length: None, + original_value: Some( + Object( + { + "type": String( + "string", + ), + "value": ~, + }, + ), ), - type: "unknown", - other: {}, }, "unknown_type": OurLogAttribute { - value: Annotated( - String( - "", - ), - Meta { - remarks: [], - errors: [ - Error { - kind: InvalidData, - data: {}, - }, - ], - original_length: None, - original_value: Some( - String( - "test", - ), - ), - }, + value: String( + "test", ), - type: "unknown", + type: Meta { + remarks: [], + errors: [ + Error { + kind: InvalidData, + data: {}, + }, + ], + original_length: None, + original_value: Some( + String( + "custom", + ), + ), + }, other: {}, }, "valid_bool": OurLogAttribute { value: Bool( true, ), - type: "boolean", + type: Boolean, other: {}, }, "valid_double": OurLogAttribute { value: F64( 42.5, ), - type: "double", + type: Double, other: {}, }, "valid_double_with_u64": OurLogAttribute { value: I64( 42, ), - type: "double", + type: Double, other: {}, }, "valid_int_from_string": OurLogAttribute { - value: I64( - 42, - ), - type: "integer", + value: Meta { + remarks: [], + errors: [ + Error { + kind: InvalidData, + data: {}, + }, + ], + original_length: None, + original_value: Some( + String( + "42", + ), + ), + }, + type: Integer, other: {}, }, "valid_int_i64": OurLogAttribute { value: I64( -42, ), - type: "integer", + type: Integer, other: {}, }, "valid_int_u64": OurLogAttribute { value: I64( 42, ), - type: "integer", + type: Integer, other: {}, }, "valid_string": OurLogAttribute { value: String( "test", ), - type: "string", + type: String, other: {}, }, "valid_string_with_other": OurLogAttribute { value: String( "test", ), - type: "string", + type: String, other: { "some_other_field": String( "some_other_value", From 9f351774f54d9154318ec3b4047b6ee8b18421c5 Mon Sep 17 00:00:00 2001 From: Kev Date: Thu, 10 Apr 2025 16:14:44 -0400 Subject: [PATCH 17/21] Update instas --- relay-event-schema/src/protocol/ourlog.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/relay-event-schema/src/protocol/ourlog.rs b/relay-event-schema/src/protocol/ourlog.rs index 9b917912e38..7e2fc74de85 100644 --- a/relay-event-schema/src/protocol/ourlog.rs +++ b/relay-event-schema/src/protocol/ourlog.rs @@ -315,49 +315,49 @@ mod tests { value: Bool( true, ), - type: "boolean", + type: Boolean, other: {}, }, "double.attribute": OurLogAttribute { value: F64( 1.23, ), - type: "double", + type: Double, other: {}, }, "sentry.observed_timestamp_nanos": OurLogAttribute { value: String( "1544712660300000000", ), - type: "integer", + type: Integer, other: {}, }, "sentry.severity_number": OurLogAttribute { value: String( "10", ), - type: "integer", + type: Integer, other: {}, }, "sentry.severity_text": OurLogAttribute { value: String( "info", ), - type: "string", + type: String, other: {}, }, "sentry.trace_flags": OurLogAttribute { value: String( "10", ), - type: "integer", + type: Integer, other: {}, }, "string.attribute": OurLogAttribute { value: String( "some string", ), - type: "string", + type: String, other: {}, }, }, From 4fea35282df05ed7ab23d3166df8e00a40696697 Mon Sep 17 00:00:00 2001 From: Kev Date: Thu, 10 Apr 2025 16:20:47 -0400 Subject: [PATCH 18/21] Update severity to default to 0, include a test --- relay-ourlogs/src/ourlog.rs | 40 ++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/relay-ourlogs/src/ourlog.rs b/relay-ourlogs/src/ourlog.rs index 4b7583cad58..979ab8f9db7 100644 --- a/relay-ourlogs/src/ourlog.rs +++ b/relay-ourlogs/src/ourlog.rs @@ -274,7 +274,9 @@ fn level_to_otel_severity_number(level: Option) -> i64 { Some(OurLogLevel::Warn) => 13, Some(OurLogLevel::Error) => 17, Some(OurLogLevel::Fatal) => 21, - _ => 25, + // 0 is the default value. + // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/68e1d6cd94bfca9bdf725327d4221f97ce0e0564/pkg/stanza/docs/types/severity.md + _ => 0, } } @@ -578,6 +580,42 @@ mod tests { "###); } + #[test] + fn ourlog_merge_otel_log_with_unknown_severity_number() { + let json = r#"{ + "timestamp": 946684800.0, + "level": "abc", + "trace_id": "5B8EFFF798038103D269B633813FC60C", + "span_id": "EEE19B7EC3C1B174", + "body": "Example log record", + "attributes": { + "foo": { + "value": "9", + "type": "string" + } + } + }"#; + + let data = Annotated::::from_json(json).unwrap(); + let merged_log = ourlog_merge_otel(data); + insta::assert_debug_snapshot!(merged_log.value().unwrap().other, @r###" + { + "observed_timestamp_nanos": U64( + 1744316429000000000, + ), + "severity_number": I64( + 0, + ), + "severity_text": String( + "abc", + ), + "timestamp_nanos": U64( + 946684800000000000, + ), + } + "###); + } + #[test] #[allow(deprecated)] fn ourlog_merge_otel_log_with_timestamp() { From 9e3ce409a5e38623539eb7e93d68101c02427d67 Mon Sep 17 00:00:00 2001 From: Kev Date: Thu, 10 Apr 2025 16:40:47 -0400 Subject: [PATCH 19/21] Add schema processor --- relay-server/src/services/processor/ourlog.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index f222e44049a..4977d498c6e 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -17,6 +17,7 @@ use { crate::services::outcome::{DiscardReason, Outcome}, crate::services::processor::ProcessingError, relay_dynamic_config::ProjectConfig, + relay_event_normalization::SchemaProcessor, relay_event_schema::processor::{process_value, ProcessingState}, relay_event_schema::protocol::{OurLog, OurLogAttributeType}, relay_ourlogs::OtelLog, @@ -98,6 +99,8 @@ pub fn process(managed_envelope: &mut TypedEnvelope, project_info: Arc #[cfg(feature = "processing")] fn normalize(annotated_log: &mut Annotated) -> Result<(), ProcessingError> { + process_value(annotated_log, &mut SchemaProcessor, ProcessingState::root())?; + let Some(log) = annotated_log.value_mut() else { return Err(ProcessingError::NoEventPayload); }; From 1632a309821fe7571f86433569221f1ad3282688 Mon Sep 17 00:00:00 2001 From: Kev Date: Thu, 10 Apr 2025 17:09:52 -0400 Subject: [PATCH 20/21] Fix tests --- relay-ourlogs/src/ourlog.rs | 40 +++++++++++++------------------------ 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/relay-ourlogs/src/ourlog.rs b/relay-ourlogs/src/ourlog.rs index 979ab8f9db7..9baca62aa3d 100644 --- a/relay-ourlogs/src/ourlog.rs +++ b/relay-ourlogs/src/ourlog.rs @@ -544,21 +544,21 @@ mod tests { value: String( "9", ), - type: "string", + type: String, other: {}, }, "sentry.severity_number": OurLogAttribute { value: I64( 9, ), - type: "integer", + type: Integer, other: {}, }, "sentry.severity_text": OurLogAttribute { value: String( "info", ), - type: "string", + type: String, other: {}, }, }, @@ -598,22 +598,10 @@ mod tests { let data = Annotated::::from_json(json).unwrap(); let merged_log = ourlog_merge_otel(data); - insta::assert_debug_snapshot!(merged_log.value().unwrap().other, @r###" - { - "observed_timestamp_nanos": U64( - 1744316429000000000, - ), - "severity_number": I64( - 0, - ), - "severity_text": String( - "abc", - ), - "timestamp_nanos": U64( - 946684800000000000, - ), - } - "###); + assert_eq!( + merged_log.value().unwrap().other.get("severity_number"), + Some(&Annotated::new(Value::I64(0))) + ); } #[test] @@ -656,21 +644,21 @@ mod tests { value: String( "9", ), - type: "string", + type: String, other: {}, }, "sentry.severity_number": OurLogAttribute { value: I64( - 25, + 0, ), - type: "integer", + type: Integer, other: {}, }, "sentry.severity_text": OurLogAttribute { value: String( "info", ), - type: "string", + type: String, other: {}, }, }, @@ -679,7 +667,7 @@ mod tests { 1742481864000000000, ), "severity_number": I64( - 25, + 0, ), "severity_text": String( "info", @@ -701,7 +689,7 @@ mod tests { }, "sentry.severity_number": { "type": "integer", - "value": 25 + "value": 0 }, "sentry.severity_text": { "type": "string", @@ -709,7 +697,7 @@ mod tests { } }, "observed_timestamp_nanos": 1742481864000000000, - "severity_number": 25, + "severity_number": 0, "severity_text": "info", "timestamp_nanos": 1638144000000000000 } From 53c5ce0c575b42ef408dcbfa043fb663794b5c40 Mon Sep 17 00:00:00 2001 From: David Herberth Date: Fri, 11 Apr 2025 12:30:40 +0200 Subject: [PATCH 21/21] remove attributes instead of just type/value --- relay-server/src/services/processor/ourlog.rs | 126 ++++++++++-------- tests/integration/test_ourlogs.py | 1 + 2 files changed, 71 insertions(+), 56 deletions(-) diff --git a/relay-server/src/services/processor/ourlog.rs b/relay-server/src/services/processor/ourlog.rs index 4977d498c6e..77d8208362b 100644 --- a/relay-server/src/services/processor/ourlog.rs +++ b/relay-server/src/services/processor/ourlog.rs @@ -154,13 +154,20 @@ fn process_attribute_types(ourlog: &mut OurLog) { (Annotated(Some(Double), _), Annotated(Some(Value::U64(_)), _)) => (), (Annotated(Some(Double), _), Annotated(Some(Value::F64(_)), _)) => (), (Annotated(Some(String), _), Annotated(Some(Value::String(_)), _)) => (), - (Annotated(ty_opt @ Some(Unknown(_)), ty_meta), _) => { - ty_meta.add_error(ErrorKind::InvalidData); - ty_meta.set_original_value(ty_opt.take()); + // Note: currently the mapping to Kafka requires that invalid or unknown combinations + // of types and values are removed from the mapping. + // + // Usually Relay would only modify the offending values, but for now, until there + // is better support in the pipeline here, we need to remove the entire attribute. + (Annotated(Some(Unknown(_)), _), _) => { + let original = attribute.value_mut().take(); + attribute.meta_mut().add_error(ErrorKind::InvalidData); + attribute.meta_mut().set_original_value(original); } - (Annotated(Some(_), _), Annotated(value @ Some(_), value_meta)) => { - value_meta.add_error(ErrorKind::InvalidData); - value_meta.set_original_value(value.take()); + (Annotated(Some(_), _), Annotated(Some(_), _)) => { + let original = attribute.value_mut().take(); + attribute.meta_mut().add_error(ErrorKind::InvalidData); + attribute.meta_mut().set_original_value(original); } (Annotated(None, _), _) | (_, Annotated(None, _)) => { let original = attribute.value_mut().take(); @@ -345,24 +352,27 @@ mod tests { type: Double, other: {}, }, - "invalid_int_from_invalid_string": OurLogAttribute { - value: Meta { - remarks: [], - errors: [ - Error { - kind: InvalidData, - data: {}, + "invalid_int_from_invalid_string": Meta { + remarks: [], + errors: [ + Error { + kind: InvalidData, + data: {}, + }, + ], + original_length: None, + original_value: Some( + Object( + { + "type": String( + "integer", + ), + "value": String( + "abc", + ), }, - ], - original_length: None, - original_value: Some( - String( - "abc", - ), ), - }, - type: Integer, - other: {}, + ), }, "missing_type": Meta { remarks: [], @@ -404,26 +414,27 @@ mod tests { ), ), }, - "unknown_type": OurLogAttribute { - value: String( - "test", - ), - type: Meta { - remarks: [], - errors: [ - Error { - kind: InvalidData, - data: {}, + "unknown_type": Meta { + remarks: [], + errors: [ + Error { + kind: InvalidData, + data: {}, + }, + ], + original_length: None, + original_value: Some( + Object( + { + "type": String( + "custom", + ), + "value": String( + "test", + ), }, - ], - original_length: None, - original_value: Some( - String( - "custom", - ), ), - }, - other: {}, + ), }, "valid_bool": OurLogAttribute { value: Bool( @@ -446,24 +457,27 @@ mod tests { type: Double, other: {}, }, - "valid_int_from_string": OurLogAttribute { - value: Meta { - remarks: [], - errors: [ - Error { - kind: InvalidData, - data: {}, + "valid_int_from_string": Meta { + remarks: [], + errors: [ + Error { + kind: InvalidData, + data: {}, + }, + ], + original_length: None, + original_value: Some( + Object( + { + "type": String( + "integer", + ), + "value": String( + "42", + ), }, - ], - original_length: None, - original_value: Some( - String( - "42", - ), ), - }, - type: Integer, - other: {}, + ), }, "valid_int_i64": OurLogAttribute { value: I64( diff --git a/tests/integration/test_ourlogs.py b/tests/integration/test_ourlogs.py index a91e68494ff..01784fe6297 100644 --- a/tests/integration/test_ourlogs.py +++ b/tests/integration/test_ourlogs.py @@ -152,6 +152,7 @@ def test_ourlog_extraction_with_sentry_logs( "sentry.severity_text": {"value": "info", "type": "string"}, "unknown_type": {"value": "info", "type": "unknown"}, "broken_type": {"value": "info", "type": "not_a_real_type"}, + "mismatched_type": {"value": "some string", "type": "boolean"}, "valid_string_with_other": { "value": "test", "type": "string",