Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SpanID, TraceID, KeyValue and other structs cannot be unmarshalled from JSON #1819

Open
toby-allsopp opened this issue Apr 19, 2021 · 3 comments
Labels
area:trace Part of OpenTelemetry tracing bug Something isn't working help wanted Extra attention is needed

Comments

@toby-allsopp
Copy link

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.

@toby-allsopp toby-allsopp added the bug Something isn't working label Apr 19, 2021
@MrAlias MrAlias added help wanted Extra attention is needed area:trace Part of OpenTelemetry tracing labels Apr 21, 2021
@Aneurysm9
Copy link
Member

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?

@MrAlias
Copy link
Contributor

MrAlias commented Apr 29, 2021

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.

@Kimbohlovette
Copy link

@pellared is this issue available?
I would like to take it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:trace Part of OpenTelemetry tracing bug Something isn't working help wanted Extra attention is needed
Projects
Status: Low priority
Development

No branches or pull requests

5 participants