Review Data Model (Protos)

[tedsuo]

Everyone, please review the current protobuf and metadata definitions and leave comments here.

[tedsuo]

Current Protos:

https://github.com/bogdandrutu/openconsensus/tree/master/sdk/src/main/proto

Related Metadata Discussions:

https://github.com/bogdandrutu/openconsensus/issues?q=metadata

[bg451]

I chatted with @iredelmeier about the model and a concern about using protobuf came up.

If a language does not support protobuf or a user doesn't want to include the protobuf library due to size (e.g. browser javascript), there needs to be a common way to serialize spans and metrics using something like JSON. Clear mappings to JSON exist for most protobuf types, however, the protobuf definitions in the SDK heavily uses oneof which do not have defined mappings to JSON. See the protobuf JSON reference for what is and isn't defined. I think this is something that should be discussed when the protos are moved to their own repo.

[yurishkuro]

@bg451 good point, and it's not just JSON, in some languages mapping oneof to types is exceptionally ugly. In Jaeger protos we decided to live with minor possible ambiguities of the data rather than have to deal with oneof.

[bogdandrutu]

I think the oneof implementation is as simple as in the JSON you enforce to send only one of these fields. I may be wrong but I can ask.

[tmc]

It's a bit unfortunate that there isn't a great way to attach structured nested metadata as attributes. Perhaps consider supporting Any values as a means of extension?

[bogdandrutu]

@tmc we discuss to add some support of AttributeValue to be another Map<String, AttributeValue> is that enough?

We do not want to add Any because that means that a backend cannot analyze the data, maybe not even display them.

[tmc]

I imagine that for many use cases allowing maps would suffice.

[tete17]

Sorry to step into this conversation but is the idea of this project that all other language implementations to use this definition as a basis of is it only for the Java one?

I do not see it in use as a submodule for the node-js one

[SergeyKanzhelev]

Seems to be complete. See open-telemetry/oteps#59 where we extensively reviewed it.

Closing.