Support optional fields in proto

[codeboten]

A change proposed in the opentelemetry-proto uses optional fields, which are not supported by the gogo proto library. A few options have been proposed on how to move forward:

1. fork the gogo library and add support for optional fields
2. replace the library with manual serialization

[MovieStoreGuy]

@codeboten,

Gogo has been looking for a new owner for some time now, would be better just to use the official protobuf lib for go?

https://github.com/golang/protobuf

[codeboten]

There have now been 3 prototypes to attempt at adding support for optional fields:

1. Fork gogoproto and add optional feature to it: use gogo proto from jsuereth/protobuf fork for optional support #4259
2. Use custom type field in gogo: pdata: Add support for optional double fields #4495
3. Use google protobuf directly in combination with vtproto for faster serialization/deserialization: use google proto instead of gogo #4658

With options 1 & 2, the burden of maintenance moving forward continues to be high, in addition to continuing a dependency on a library which is no longer maintained. The performance of option 3 essentially means we'll have to spend significant amounts of effort in another library (vtproto).

As mentioned in the SIG call on Jan 12, as a result of the previous 3 prototypes, the current thinking is to move ahead with prototyping serialization/deserialization OTLP from/to pdata directly. This has been done by the Java SIG as per the discussion here

Benefits

• reduces the need to allocate both pdata and proto objects
• reduces external dependencies
• improves the process when new changes are available in the proto. it removes steps 1 & 2 from the process we have today which involves:
  • generating the new proto code
  • generating new pdata code
  • updating pdata library to make use of new proto features

Drawbacks

• additional code for managing the serialization and deserialization
• new code introduces opportunity for many new issues

[tigrannajaryan]

As mentioned in the SIG call on Jan 12, as a result of the previous 3 prototypes, the current thinking is to move ahead with prototyping serialization/deserialization OTLP from/to pdata directly. This has been done by the Java SIG as per the discussion here

Can we use the generated code as a starting point, which we then tweak to get rid of gogoproto dependency? This should significantly lower the implementation effort. I think Bogdan's idea of implementing a limited code generator just for our use-case is also worth exploring.

If we decide to go with manually written serialization code (or with our own code generator) it will open some more optimization possibilities which are difficult/impossible with gogoproto today (e.g. I want to replace AnyValue by much faster Variant I wrote or experiment with changing AttributeMap key/value list into an open-addressing hashmap to improve performance with large maps, etc).

My concern with manually written deserialization code is the potential to introduce bugs which can be very impactful since OTLP is an untrusted input. We need to look into fuzz testing and other verification measures if we go this route so that we have reasonable confidence in our implementation.

[bogdandrutu]

@codeboten one option which I don't know if we thought/discuss about is to actually rely on oneof for the "optional/pointer":

optional fixed64 foo = 1 -> oneof foo_ { fixed64 foo = 1;}

@codeboten: showed me, something similar suggested by @anuraaga, though I discovered this while working on a faster/lazy proto generator and reading the oneof description.

I think this is a good short term and we can do this as we do the nullable, since this optional is implemented anyway using a pointer (if I am not mistaken), this solution only adds 8B to the struct (since interfaces in golang have 16B vs pointers 8B), and still only one allocation.

[bogdandrutu]

I think this achieved the goal, and we should focus on updating to 0.15.0.