Skip to content

Commit 8fd63bf

Browse files
committed
feat(eap): Add support for native scalar arrays
1 parent 40dfb72 commit 8fd63bf

File tree

10 files changed

+246
-107
lines changed

10 files changed

+246
-107
lines changed

relay-event-normalization/src/eap/mod.rs

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,12 @@ pub fn normalize_attribute_types(attributes: &mut Annotated<Attributes>) {
6666
(Annotated(Some(Double), _), Annotated(Some(Value::U64(_)), _)) => (),
6767
(Annotated(Some(Double), _), Annotated(Some(Value::F64(_)), _)) => (),
6868
(Annotated(Some(String), _), Annotated(Some(Value::String(_)), _)) => (),
69+
(Annotated(Some(Array), _), Annotated(Some(Value::Array(arr)), _)) => {
70+
if !is_supported_array(arr) {
71+
let _ = attribute.value_mut().take();
72+
attribute.meta_mut().add_error(ErrorKind::InvalidData);
73+
}
74+
}
6975
// Note: currently the mapping to Kafka requires that invalid or unknown combinations
7076
// of types and values are removed from the mapping.
7177
//
@@ -90,6 +96,43 @@ pub fn normalize_attribute_types(attributes: &mut Annotated<Attributes>) {
9096
}
9197
}
9298

99+
/// Returns `true` if the passed array is an array we currently support.
100+
///
101+
/// Currently all arrays must be homogeneous types.
102+
fn is_supported_array(arr: &[Annotated<Value>]) -> bool {
103+
let mut iter = arr.iter();
104+
105+
let Some(first) = iter.next() else {
106+
// Empty arrays are supported.
107+
return true;
108+
};
109+
110+
let item = iter.try_fold(first, |prev, current| {
111+
let r = match (prev.value(), current.value()) {
112+
(None, None) => prev,
113+
(None, Some(_)) => current,
114+
(Some(_), None) => prev,
115+
(Some(Value::String(_)), Some(Value::String(_))) => prev,
116+
(Some(Value::Bool(_)), Some(Value::Bool(_))) => prev,
117+
(
118+
Some(Value::I64(_) | Value::U64(_) | Value::F64(_)),
119+
Some(Value::I64(_) | Value::U64(_) | Value::F64(_)),
120+
) => prev,
121+
// Everything else is unsupported.
122+
//
123+
// This includes nested arrays, nested objects and mixed arrays for now.
124+
(Some(_), Some(_)) => return None,
125+
};
126+
127+
Some(r)
128+
});
129+
130+
matches!(
131+
item.and_then(|v| v.value()),
132+
Some(Value::String(_) | Value::Bool(_) | Value::I64(_) | Value::U64(_) | Value::F64(_))
133+
)
134+
}
135+
93136
/// Adds the `received` time to the attributes.
94137
pub fn normalize_received(attributes: &mut Annotated<Attributes>, received: DateTime<Utc>) {
95138
attributes
@@ -582,13 +625,33 @@ mod tests {
582625
},
583626
"missing_value": {
584627
"type": "string"
628+
},
629+
"supported_array_string": {
630+
"type": "array",
631+
"value": ["foo", "bar"]
632+
},
633+
"supported_array_double": {
634+
"type": "array",
635+
"value": [3, 3.0, 3]
636+
},
637+
"unsupported_array_mixed": {
638+
"type": "array",
639+
"value": ["foo", 1.0]
640+
},
641+
"unsupported_array_object": {
642+
"type": "array",
643+
"value": [{}]
644+
},
645+
"unsupported_array_in_array": {
646+
"type": "array",
647+
"value": [[]]
585648
}
586649
}"#;
587650

588651
let mut attributes = Annotated::<Attributes>::from_json(json).unwrap();
589652
normalize_attribute_types(&mut attributes);
590653

591-
insta::assert_json_snapshot!(SerializableAnnotated(&attributes), @r###"
654+
insta::assert_json_snapshot!(SerializableAnnotated(&attributes), @r#"
592655
{
593656
"double_with_i64": {
594657
"type": "double",
@@ -597,7 +660,25 @@ mod tests {
597660
"invalid_int_from_invalid_string": null,
598661
"missing_type": null,
599662
"missing_value": null,
663+
"supported_array_double": {
664+
"type": "array",
665+
"value": [
666+
3,
667+
3.0,
668+
3
669+
]
670+
},
671+
"supported_array_string": {
672+
"type": "array",
673+
"value": [
674+
"foo",
675+
"bar"
676+
]
677+
},
600678
"unknown_type": null,
679+
"unsupported_array_in_array": null,
680+
"unsupported_array_mixed": null,
681+
"unsupported_array_object": null,
601682
"valid_bool": {
602683
"type": "boolean",
603684
"value": true
@@ -673,6 +754,27 @@ mod tests {
673754
}
674755
}
675756
},
757+
"unsupported_array_in_array": {
758+
"": {
759+
"err": [
760+
"invalid_data"
761+
]
762+
}
763+
},
764+
"unsupported_array_mixed": {
765+
"": {
766+
"err": [
767+
"invalid_data"
768+
]
769+
}
770+
},
771+
"unsupported_array_object": {
772+
"": {
773+
"err": [
774+
"invalid_data"
775+
]
776+
}
777+
},
676778
"valid_int_from_string": {
677779
"": {
678780
"err": [
@@ -686,7 +788,7 @@ mod tests {
686788
}
687789
}
688790
}
689-
"###);
791+
"#);
690792
}
691793

692794
#[test]

relay-event-schema/src/protocol/attributes.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,29 @@ pub fn attribute_pii_from_conventions(state: &ProcessingState) -> Pii {
120120

121121
#[derive(Debug, Clone, PartialEq, Eq)]
122122
pub enum AttributeType {
123+
/// A boolean.
124+
///
125+
/// The respective value must be of type [`Value::Bool`].
123126
Boolean,
127+
/// An integer type.
128+
///
129+
/// The respective value must be of type [`Value::I64`] or [`Value::U64`].
124130
Integer,
131+
/// A floating point/double type.
132+
///
133+
/// The respective value must be of type [`Value::F64`].
125134
Double,
135+
/// A string type.
136+
///
137+
/// The respective value must be of type [`Value::String`].
126138
String,
139+
/// A string type.
140+
///
141+
/// The respective value must be of type [`Value::Array`].
142+
Array,
143+
/// An unknown type.
144+
///
145+
/// Kept for forward compatibility.
127146
Unknown(String),
128147
}
129148

@@ -136,6 +155,7 @@ impl AttributeType {
136155
Self::Integer => "integer",
137156
Self::Double => "double",
138157
Self::String => "string",
158+
Self::Array => "array",
139159
Self::Unknown(value) => value,
140160
}
141161
}
@@ -158,6 +178,7 @@ impl From<String> for AttributeType {
158178
"integer" => Self::Integer,
159179
"double" => Self::Double,
160180
"string" => Self::String,
181+
"array" => Self::Array,
161182
_ => Self::Unknown(value),
162183
}
163184
}

relay-otel/src/lib.rs

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,31 +37,26 @@ pub fn otel_value_to_attribute(otel_value: OtelValue) -> Option<Attribute> {
3737
(AttributeType::String, Value::String(s))
3838
}
3939
OtelValue::ArrayValue(array) => {
40-
// Filter out nested arrays and key-value lists for safety.
41-
// This is not usually allowed by the OTLP protocol, but we filter
42-
// these values out before serializing for robustness.
43-
let safe_values: Vec<serde_json::Value> = array
40+
let values: Vec<Annotated<Value>> = array
4441
.values
4542
.into_iter()
46-
.filter_map(|v| match v.value? {
47-
OtelValue::StringValue(s) => Some(serde_json::Value::String(s)),
48-
OtelValue::BoolValue(b) => Some(serde_json::Value::Bool(b)),
49-
OtelValue::IntValue(i) => {
50-
Some(serde_json::Value::Number(serde_json::Number::from(i)))
51-
}
52-
OtelValue::DoubleValue(d) => {
53-
serde_json::Number::from_f64(d).map(serde_json::Value::Number)
54-
}
55-
OtelValue::BytesValue(bytes) => {
56-
String::from_utf8(bytes).ok().map(serde_json::Value::String)
57-
}
58-
// Skip nested complex types for safety
59-
OtelValue::ArrayValue(_) | OtelValue::KvlistValue(_) => None,
43+
.filter_map(|v| {
44+
Some(match v.value? {
45+
OtelValue::StringValue(s) => Value::String(s),
46+
OtelValue::BoolValue(b) => Value::Bool(b),
47+
OtelValue::IntValue(i) => Value::I64(i),
48+
OtelValue::DoubleValue(d) => Value::F64(d),
49+
OtelValue::BytesValue(bytes) => {
50+
Value::String(String::from_utf8(bytes).ok()?)
51+
}
52+
// Currently not supported.
53+
OtelValue::ArrayValue(_) | OtelValue::KvlistValue(_) => return None,
54+
})
6055
})
56+
.map(Annotated::new)
6157
.collect();
6258

63-
let json = serde_json::to_string(&safe_values).ok()?;
64-
(AttributeType::String, Value::String(json))
59+
(AttributeType::Array, Value::Array(values))
6560
}
6661
OtelValue::KvlistValue(kvlist) => {
6762
// Convert key-value list to JSON object and serialize as string.

relay-server/src/processing/logs/store.rs

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@ use std::collections::HashMap;
22

33
use chrono::{DateTime, Utc};
44
use relay_event_schema::protocol::{Attributes, OurLog, OurLogLevel, SpanId};
5-
use relay_protocol::{Annotated, IntoValue, Value};
5+
use relay_protocol::Annotated;
66
use relay_quotas::Scoping;
77
use sentry_protos::snuba::v1::{AnyValue, TraceItem, TraceItemType, any_value};
88
use uuid::Uuid;
99

1010
use crate::envelope::WithHeader;
1111
use crate::processing::logs::{Error, Result};
12-
use crate::processing::utils::store::{AttributeMeta, extract_meta_attributes, proto_timestamp};
13-
use crate::processing::{Counted, Retention};
12+
use crate::processing::utils::store::{extract_meta_attributes, proto_timestamp};
13+
use crate::processing::{self, Counted, Retention};
1414
use crate::services::outcome::DiscardReason;
1515
use crate::services::store::StoreTraceItem;
1616

@@ -114,42 +114,7 @@ fn attributes(
114114
// +N, one for each field attribute added and some extra for potential meta.
115115
result.reserve(attributes.0.len() + 5 + 3);
116116

117-
for (name, attribute) in attributes {
118-
let meta = AttributeMeta {
119-
meta: IntoValue::extract_meta_tree(&attribute),
120-
};
121-
if let Some(meta) = meta.to_any_value() {
122-
result.insert(format!("sentry._meta.fields.attributes.{name}"), meta);
123-
}
124-
125-
let value = attribute
126-
.into_value()
127-
.and_then(|v| v.value.value.into_value());
128-
129-
let Some(value) = value else {
130-
// Meta has already been handled, no value -> skip.
131-
// There are also no current plans to handle `null` in EAP.
132-
continue;
133-
};
134-
135-
let Some(value) = (match value {
136-
Value::Bool(v) => Some(any_value::Value::BoolValue(v)),
137-
Value::I64(v) => Some(any_value::Value::IntValue(v)),
138-
Value::U64(v) => i64::try_from(v).ok().map(any_value::Value::IntValue),
139-
Value::F64(v) => Some(any_value::Value::DoubleValue(v)),
140-
Value::String(v) => Some(any_value::Value::StringValue(v)),
141-
// These cases do not happen, as they are not valid attributes
142-
// and they should have been filtered out before already.
143-
Value::Array(_) | Value::Object(_) => {
144-
debug_assert!(false, "unsupported log value");
145-
None
146-
}
147-
}) else {
148-
continue;
149-
};
150-
151-
result.insert(name, AnyValue { value: Some(value) });
152-
}
117+
processing::utils::store::convert_attributes_into(&mut result, attributes);
153118

154119
let FieldAttributes {
155120
level,

relay-server/src/processing/trace_metrics/store.rs

Lines changed: 5 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,15 @@ use chrono::{DateTime, Utc};
44
use prost_types::Timestamp;
55
use relay_base_schema::metrics::MetricUnit;
66
use relay_event_schema::protocol::{Attributes, MetricType, SpanId, TraceMetric};
7-
use relay_protocol::{Annotated, IntoValue, Value};
7+
use relay_protocol::{Annotated, Value};
88
use relay_quotas::Scoping;
99
use sentry_protos::snuba::v1::{AnyValue, TraceItem, TraceItemType, any_value};
1010
use uuid::Uuid;
1111

1212
use crate::envelope::WithHeader;
1313
use crate::processing::trace_metrics::{Error, Result};
14-
use crate::processing::utils::store::{
15-
AttributeMeta, extract_client_sample_rate, extract_meta_attributes,
16-
};
17-
use crate::processing::{Counted, Retention};
14+
use crate::processing::utils::store::{extract_client_sample_rate, extract_meta_attributes};
15+
use crate::processing::{self, Counted, Retention};
1816
use crate::services::outcome::DiscardReason;
1917
use crate::services::store::StoreTraceItem;
2018

@@ -114,40 +112,9 @@ fn attributes(
114112
fields: FieldAttributes,
115113
) -> HashMap<String, AnyValue> {
116114
let mut result = meta;
117-
result.reserve(attributes.0.len() + 5);
115+
result.reserve(attributes.0.len() + 15);
118116

119-
for (name, attribute) in attributes {
120-
let meta = AttributeMeta {
121-
meta: IntoValue::extract_meta_tree(&attribute),
122-
};
123-
if let Some(meta) = meta.to_any_value() {
124-
result.insert(format!("sentry._meta.fields.attributes.{name}"), meta);
125-
}
126-
127-
let value = attribute
128-
.into_value()
129-
.and_then(|v| v.value.value.into_value());
130-
131-
let Some(value) = value else {
132-
continue;
133-
};
134-
135-
let Some(value) = (match value {
136-
Value::Bool(v) => Some(any_value::Value::BoolValue(v)),
137-
Value::I64(v) => Some(any_value::Value::IntValue(v)),
138-
Value::U64(v) => i64::try_from(v).ok().map(any_value::Value::IntValue),
139-
Value::F64(v) => Some(any_value::Value::DoubleValue(v)),
140-
Value::String(v) => Some(any_value::Value::StringValue(v)),
141-
Value::Array(_) | Value::Object(_) => {
142-
debug_assert!(false, "unsupported trace metric value");
143-
None
144-
}
145-
}) else {
146-
continue;
147-
};
148-
149-
result.insert(name, AnyValue { value: Some(value) });
150-
}
117+
processing::utils::store::convert_attributes_into(&mut result, attributes);
151118

152119
let FieldAttributes {
153120
metric_name,

0 commit comments

Comments
 (0)