ref(spans): Unite span kind types (#5144)

* `SpanKind` was missing a catch-all type so outdated external relays
would have dropped it.
* `SpanV2Kind` promised a catchall-variant but didn't have one.

Unite them into one.

Author: jjbayer

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

@@ -1106,23 +1106,35 @@ impl FromValue for Route {
     }
 }
 
+/// The kind of a span.
+///
+/// This corresponds to OTEL's kind enum, plus a
+/// catchall variant for forward compatibility.
 #[derive(Clone, Debug, PartialEq, ProcessValue)]
 pub enum SpanKind {
+    /// An operation internal to an application.
     Internal,
+    /// Server-side processing requested by a client.
     Server,
+    /// A request from a client to a server.
     Client,
+    /// Scheduling of an operation.
     Producer,
+    /// Processing of a scheduled operation.
     Consumer,
+    /// Unknown kind, for forward compatibility.
+    Unknown(String),
 }
 
 impl SpanKind {
-    pub fn as_str(&self) -> &'static str {
+    pub fn as_str(&self) -> &str {
         match self {
             Self::Internal => "internal",
             Self::Server => "server",
             Self::Client => "client",
             Self::Producer => "producer",
             Self::Consumer => "consumer",
+            Self::Unknown(s) => s.as_str(),
         }
     }
 }
@@ -1146,7 +1158,7 @@ impl std::str::FromStr for SpanKind {
             "client" => SpanKind::Client,
             "producer" => SpanKind::Producer,
             "consumer" => SpanKind::Consumer,
-            _ => return Err(ParseSpanKindError),
+            other => SpanKind::Unknown(other.to_owned()),
         })
     }
 }
@@ -1622,4 +1634,20 @@ mod tests {
             r#"{"links":[{"trace_id":"5c79f60c11214eb38604f4ae0781bfb2","span_id":"ab90fdead5f74052","sampled":true,"attributes":{"sentry.link.type":"previous_trace"}},{"trace_id":"4c79f60c11214eb38604f4ae0781bfb2","span_id":"fa90fdead5f74052","sampled":true,"attributes":{"sentry.link.type":"next_trace"}}]}"#
         );
     }
+
+    #[test]
+    fn test_span_kind() {
+        let span = Annotated::<Span>::from_json(
+            r#"{
+                "kind": "???"
+            }"#,
+        )
+        .unwrap()
+        .into_value()
+        .unwrap();
+        assert_eq!(
+            span.kind.value().unwrap(),
+            &SpanKind::Unknown("???".to_owned())
+        );
+    }
 }

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

@@ -1,12 +1,11 @@
-use relay_protocol::{Annotated, Array, Empty, Error, FromValue, Getter, IntoValue, Object, Value};
+use relay_protocol::{Annotated, Array, Empty, FromValue, Getter, IntoValue, Object, Value};
 
 use std::fmt;
-use std::str::FromStr;
 
 use serde::Serialize;
 
 use crate::processor::ProcessValue;
-use crate::protocol::{Attributes, OperationType, SpanId, Timestamp, TraceId};
+use crate::protocol::{Attributes, OperationType, SpanId, SpanKind, Timestamp, TraceId};
 
 /// A version 2 (transactionless) span.
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
@@ -46,7 +45,7 @@ pub struct SpanV2 {
     ///
     /// See <https://opentelemetry.io/docs/specs/otel/trace/api/#spankind>
     #[metastructure(skip_serialization = "empty", trim = false)]
-    pub kind: Annotated<SpanV2Kind>,
+    pub kind: Annotated<SpanKind>,
 
     /// Timestamp when the span started.
     #[metastructure(required = true)]
@@ -177,116 +176,6 @@ impl IntoValue for SpanV2Status {
     }
 }
 
-/// The kind of a V2 span.
-///
-/// This corresponds to OTEL's kind enum, plus a
-/// catchall variant for forward compatibility.
-#[derive(Clone, Debug, PartialEq, ProcessValue)]
-pub enum SpanV2Kind {
-    /// An operation internal to an application.
-    Internal,
-    /// Server-side processing requested by a client.
-    Server,
-    /// A request from a client to a server.
-    Client,
-    /// Scheduling of an operation.
-    Producer,
-    /// Processing of a scheduled operation.
-    Consumer,
-}
-
-impl SpanV2Kind {
-    pub fn as_str(&self) -> &'static str {
-        match self {
-            Self::Internal => "internal",
-            Self::Server => "server",
-            Self::Client => "client",
-            Self::Producer => "producer",
-            Self::Consumer => "consumer",
-        }
-    }
-}
-
-impl Empty for SpanV2Kind {
-    fn is_empty(&self) -> bool {
-        false
-    }
-}
-
-impl Default for SpanV2Kind {
-    fn default() -> Self {
-        Self::Internal
-    }
-}
-
-impl fmt::Display for SpanV2Kind {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.as_str())
-    }
-}
-
-#[derive(Debug, Clone, Copy)]
-pub struct ParseSpanV2KindError;
-
-impl fmt::Display for ParseSpanV2KindError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "invalid span kind")
-    }
-}
-
-impl FromStr for SpanV2Kind {
-    type Err = ParseSpanV2KindError;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let kind = match s {
-            "internal" => Self::Internal,
-            "server" => Self::Server,
-            "client" => Self::Client,
-            "producer" => Self::Producer,
-            "consumer" => Self::Consumer,
-            _ => return Err(ParseSpanV2KindError),
-        };
-        Ok(kind)
-    }
-}
-
-impl FromValue for SpanV2Kind {
-    fn from_value(Annotated(value, meta): Annotated<Value>) -> Annotated<Self>
-    where
-        Self: Sized,
-    {
-        match &value {
-            Some(Value::String(s)) => match s.parse() {
-                Ok(kind) => Annotated(Some(kind), meta),
-                Err(_) => Annotated::from_error(Error::expected("a span kind"), value),
-            },
-            Some(_) => Annotated::from_error(Error::expected("a span kind"), value),
-            None => Annotated::empty(),
-        }
-    }
-}
-
-impl IntoValue for SpanV2Kind {
-    fn into_value(self) -> Value
-    where
-        Self: Sized,
-    {
-        Value::String(self.to_string())
-    }
-
-    fn serialize_payload<S>(
-        &self,
-        s: S,
-        _behavior: relay_protocol::SkipSerialization,
-    ) -> Result<S::Ok, S::Error>
-    where
-        Self: Sized,
-        S: serde::Serializer,
-    {
-        s.serialize_str(self.as_str())
-    }
-}
-
 /// A link from a span to another span.
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
 #[metastructure(trim = false)]
@@ -428,7 +317,7 @@ mod tests {
             span_id: Annotated::new("438f40bd3b4a41ee".parse().unwrap()),
             parent_span_id: Annotated::empty(),
             status: Annotated::new(SpanV2Status::Ok),
-            kind: Annotated::new(SpanV2Kind::Server),
+            kind: Annotated::new(SpanKind::Server),
             is_remote: Annotated::new(true),
             links: Annotated::new(links),
             attributes: Annotated::new(attributes),

relay-spans/src/otel_to_sentry_v2.rs

@@ -1,7 +1,7 @@
 use chrono::{TimeZone, Utc};
 use opentelemetry_proto::tonic::trace::v1::span::Link as OtelLink;
 use opentelemetry_proto::tonic::trace::v1::span::SpanKind as OtelSpanKind;
-use relay_event_schema::protocol::{Attributes, SpanV2Kind};
+use relay_event_schema::protocol::{Attributes, SpanKind};
 use relay_otel::otel_value_to_attribute;
 use relay_protocol::ErrorKind;
 
@@ -116,14 +116,14 @@ fn otel_flags_is_remote(value: u32) -> Option<bool> {
     }
 }
 
-fn otel_to_sentry_kind(kind: i32) -> Annotated<SpanV2Kind> {
+fn otel_to_sentry_kind(kind: i32) -> Annotated<SpanKind> {
     match kind {
         kind if kind == OtelSpanKind::Unspecified as i32 => Annotated::empty(),
-        kind if kind == OtelSpanKind::Internal as i32 => Annotated::new(SpanV2Kind::Internal),
-        kind if kind == OtelSpanKind::Server as i32 => Annotated::new(SpanV2Kind::Server),
-        kind if kind == OtelSpanKind::Client as i32 => Annotated::new(SpanV2Kind::Client),
-        kind if kind == OtelSpanKind::Producer as i32 => Annotated::new(SpanV2Kind::Producer),
-        kind if kind == OtelSpanKind::Consumer as i32 => Annotated::new(SpanV2Kind::Consumer),
+        kind if kind == OtelSpanKind::Internal as i32 => Annotated::new(SpanKind::Internal),
+        kind if kind == OtelSpanKind::Server as i32 => Annotated::new(SpanKind::Server),
+        kind if kind == OtelSpanKind::Client as i32 => Annotated::new(SpanKind::Client),
+        kind if kind == OtelSpanKind::Producer as i32 => Annotated::new(SpanKind::Producer),
+        kind if kind == OtelSpanKind::Consumer as i32 => Annotated::new(SpanKind::Consumer),
         _ => Annotated::from_error(ErrorKind::InvalidData, Some(Value::I64(kind as i64))),
     }
 }

relay-spans/src/v1_to_v2.rs

@@ -2,8 +2,7 @@ use std::borrow::Cow;
 
 use relay_event_schema::protocol::{
     Attribute, AttributeType, AttributeValue, Attributes, JsonLenientString, Span as SpanV1,
-    SpanData, SpanKind as SpanV1Kind, SpanLink, SpanStatus as SpanV1Status, SpanV2, SpanV2Kind,
-    SpanV2Link, SpanV2Status,
+    SpanData, SpanLink, SpanStatus as SpanV1Status, SpanV2, SpanV2Link, SpanV2Status,
 };
 use relay_protocol::{Annotated, Empty, Error, IntoValue, Meta, Value};
 
@@ -112,7 +111,7 @@ pub fn span_v1_to_span_v2(span_v1: SpanV1) -> SpanV2 {
             .into(),
         status: Annotated::map_value(status, span_v1_status_to_span_v2_status),
         is_remote,
-        kind: Annotated::map_value(kind, span_v1_kind_to_span_v2_kind),
+        kind,
         start_timestamp,
         end_timestamp: timestamp,
         links: links.map_value(span_v1_links_to_span_v2_links),
@@ -128,17 +127,6 @@ fn span_v1_status_to_span_v2_status(status: SpanV1Status) -> SpanV2Status {
     }
 }
 
-fn span_v1_kind_to_span_v2_kind(kind: SpanV1Kind) -> SpanV2Kind {
-    match kind {
-        SpanV1Kind::Internal => SpanV2Kind::Internal,
-        SpanV1Kind::Server => SpanV2Kind::Server,
-        SpanV1Kind::Client => SpanV2Kind::Client,
-        SpanV1Kind::Producer => SpanV2Kind::Producer,
-        SpanV1Kind::Consumer => SpanV2Kind::Consumer,
-        // TODO: implement catchall type so outdated customer relays can still forward the field.
-    }
-}
-
 fn span_v1_links_to_span_v2_links(links: Vec<Annotated<SpanLink>>) -> Vec<Annotated<SpanV2Link>> {
     links
         .into_iter()

relay-spans/src/v2_to_v1.rs

@@ -5,7 +5,7 @@ use relay_event_schema::protocol::{SpanKind, SpanV2Link};
 use crate::status_codes;
 use relay_event_schema::protocol::{
     Attributes, EventId, Span as SpanV1, SpanData, SpanId, SpanLink, SpanStatus, SpanV2,
-    SpanV2Kind, SpanV2Status,
+    SpanV2Status,
 };
 use relay_protocol::{Annotated, FromValue, Object, Value};
 use url::Url;
@@ -151,7 +151,7 @@ pub fn span_v2_to_span_v1(span_v2: SpanV2) -> SpanV1 {
         timestamp: end_timestamp,
         trace_id,
         platform,
-        kind: kind.map_value(span_v2_kind_to_span_v1_kind),
+        kind,
         links,
         ..Default::default()
     }
@@ -186,17 +186,6 @@ fn span_v2_status_to_span_v1_status(
         })
         .or_else(|| Annotated::new(SpanStatus::Unknown))
 }
-
-fn span_v2_kind_to_span_v1_kind(kind: SpanV2Kind) -> SpanKind {
-    match kind {
-        SpanV2Kind::Internal => SpanKind::Internal,
-        SpanV2Kind::Server => SpanKind::Server,
-        SpanV2Kind::Client => SpanKind::Client,
-        SpanV2Kind::Producer => SpanKind::Producer,
-        SpanV2Kind::Consumer => SpanKind::Consumer,
-    }
-}
-
 fn span_v2_link_to_span_v1_link(link: SpanV2Link) -> SpanLink {
     let SpanV2Link {
         trace_id,
@@ -242,8 +231,8 @@ fn derive_op_for_v2_span(span: &SpanV2) -> String {
 
     if attributes.contains_key("http.request.method") || attributes.contains_key("http.method") {
         return match span.kind.value() {
-            Some(SpanV2Kind::Client) => String::from("http.client"),
-            Some(SpanV2Kind::Server) => String::from("http.server"),
+            Some(SpanKind::Client) => String::from("http.client"),
+            Some(SpanKind::Server) => String::from("http.server"),
             _ => {
                 if attributes.contains_key("sentry.http.prefetch") {
                     String::from("http.prefetch")
@@ -304,7 +293,7 @@ fn derive_description_for_v2_span(span: &SpanV2) -> Option<String> {
     description
 }
 
-fn derive_http_description(attributes: &Attributes, kind: &Option<&SpanV2Kind>) -> Option<String> {
+fn derive_http_description(attributes: &Attributes, kind: &Option<&SpanKind>) -> Option<String> {
     // Get HTTP method
     let http_method = attributes
         .get_value("http.request.method")
@@ -315,8 +304,8 @@ fn derive_http_description(attributes: &Attributes, kind: &Option<&SpanV2Kind>)
 
     // Get URL path information
     let url_path = match kind {
-        Some(SpanV2Kind::Server) => get_server_url_path(attributes),
-        Some(SpanV2Kind::Client) => get_client_url_path(attributes),
+        Some(SpanKind::Server) => get_server_url_path(attributes),
+        Some(SpanKind::Client) => get_client_url_path(attributes),
         _ => None,
     };