Serde support

[jtescher]

Should the core opentelemetry crate support serde? There are standard serialization formats, it's unclear that this is necessary (or compliant with the intention of the spec).

[TommyCpp]

I don't believe the spec explicitly requires to provide a serialization formats but rather it leave this questions to each propagator and exporter. I think in some case the serde could make things easier for exporter/propagator writers but usually exporters/propagators need some kind of conversion before serialization. Thus, serde is not a must here.

We could provide some supports like using a serde-otlp crate to do serialization and deserialization for opentelemetry-otlp. That should help ppl who want to implement their own otlp exporter for any reason.

[frigus02]

Good question. I'd say we probably don't need it. How does serde support work with backwards compatibility? If we add another field to a struct, which implements Serialize and Deserialize, would that count as a breaking change?

I like the idea of a separate crate in case there is demand for serialization and deserialization for some of the types.

[jtescher]

Removed in #738

[slinkydeveloper]

Hi, is there any alternative available right now for this?

[slinkydeveloper]

For whoever stumbles on this, I needed to serialize SpanContext and my workaround is to use serde_with paired with an intermediate struct:

#[derive(Serialize, Deserialize)]
struct SpanContextDef {
    trace_id: [u8; 16],
    span_id: [u8; 8],
    trace_flags: u8,
    is_remote: bool,
    trace_state: String,
}

// Provide a conversion to construct the remote type.
impl From<SpanContextDef> for SpanContext {
    fn from(def: SpanContextDef) -> Self {
        SpanContext::new(
            TraceId::from_bytes(def.trace_id), SpanId::from_bytes(def.span_id), TraceFlags::new(def.trace_flags), def.is_remote, TraceState::from_str(&def.trace_state).unwrap()
        )
    }
}

impl From<SpanContext> for SpanContextDef {
    fn from(ctx: SpanContext) -> Self {
        Self {
            trace_id: ctx.trace_id().to_bytes(),
            span_id: ctx.span_id().to_bytes(),
            trace_flags: ctx.trace_flags().to_u8(),
            is_remote: ctx.is_remote(),
            trace_state: ctx.trace_state().header()
        }
    }
}

// In the struct where you need SpanContext
#[serde_as(as = "FromInto<SpanContextDef>")] 
SpanContext spanContext

See https://docs.rs/serde_with/2.0.1/serde_with/guide/serde_as_transformations/index.html#convert-to-an-intermediate-type-using-into for more details about serde_with

[joshtriplett]

I've been actively using this, and was surprised to find that it had disappeared when I upgraded opentelemetry. I'd love to register interest in this feature returning in the future, if at all possible.

[jtescher]

I think providing serde support for SpanContext could make sense as that data is meant to be serialized in various formats (and potentially the data that is intended to be used by exporters as well). The other structs that supported it previously probably go against the spirit of the otel spec as I understand it.

[joshtriplett]

@jtescher I've been using it to both serialize and deserialize Vec<SpanData>, to implement a very simple TcpSpanExporter to ship traces from a constrained server to a less constrained one. If that's what you meany by "the data that is intended to be used by exporters as well", that's exactly what I'm looking for.

[jtescher]

@jtescher I've been using it to both serialize and deserialize Vec<SpanData>, to implement a very simple TcpSpanExporter to ship traces from a constrained server to a less constrained one. If that's what you meany by "the data that is intended to be used by exporters as well", that's exactly what I'm looking for.

Yep that's included in what I was referring to, I think we could support that.

[joshtriplett]

@jtescher Thank you!

[joshtriplett]

@jtescher Checking back in on this. Would it be possible to get serde support for Vec<SpanData>, please?

[shaun-cox]

(I should have searched closed issues first, but stumbled upon this after starting to work on #1074, which I just opened.)