opentelemetry-go
opentelemetry-go copied to clipboard
open-telemetry
SpanID, TraceID, KeyValue and other structs cannot be unmarshalled from JSON
Description
A number of structs, e.g. trace.SpanID, define MarshalJSON methods but not the corresponding UnmarshalJSON. This means that the JSON written by the stdout exporter, which consists of a stream of []*SpanSnapshot values, cannot be unmarshalled back into Go objects easily.
Environment
- OS: Linux
- Architecture: amd64
- Go Version: 1.15.7
- opentelemetry-go version: v0.19.0
Steps To Reproduce
ss := trace.SpanSnapshot{}
b, _ := json.Marshal(ss)
var ss2 trace.SpanSnapshot
err := json.Unmarshal(b, &ss2)
require.NoError(t, err)
require.Equal(t, ss, ss2)
Expected behavior
It would be helpful if the JSON produced by marshalling a slice of SpanSnapshots was easily unmashalled back into a slice of SpanSnapshots.
I'm not sure that this is a feature we want to provide. The stdout exporter is intended for testing and debugging and not as the basis for a communication protocol. OTLP would be the preferred mechanism for communicating this information. It has versioning, backwards compatibility guarantees, and enables data interchange with other languages and implementations. The JSON produced by marshaling the types referenced in this issue has none of that and I would prefer that we retain the flexibility to change the representations of these types at any time without needing to worry about whether doing so will break something that someone has constructed using these serializations.
Thoughts @open-telemetry/go-approvers?
From SIG:
- The STDOUT format is not a well thought out or agreed upon format. We do not want to standardize on this.
- The OTLP is an agreed upon format, we should try to leverage that.
- Have the STDOUT exporter take a Marshaler (SpanSnapshot in, bytes out) and an io.Writer. All it does is hook these things up. We could also provide a marshaler that would be the OTLP transform as the Marshaler.
- Possibly, we could also add an io.Writer to the OTLP exporter.
It seems, based on the PRs suggested above that there is still some use for this functionality, potentially outside of just the stdout/debug function. If the concerns are Semantic/standardization, I would be happy to adjust the JSON fields to match the OpenTelemetry Specification
I think TraceID / SpanID should instead be implementing the more generic encoding.TextMarshaler / encoding.TextUnmarshaler interfaces which encoding/json supports.
