envoyproxy/envoy

Utility to render uint64_t as number (not string) when serializing protobuf message (e.g. from Struct)

Open

#10.411 aperta il 16 mar 2020

Vedi su GitHub
 (12 commenti) (0 reazioni) (0 assegnatari)C++ (5373 fork)batch import
area/protobufhelp wanted

Metriche repository

Star
 (27.997 star)
Metriche merge PR
 (Merge medio 8g) (378 PR mergiate in 30 g)

Descrizione

Since the movement on removing rapidjson as a dependency here: https://github.com/envoyproxy/envoy/issues/4705. We prefer to use protobuf way of serializing objects to JSON strings. Most of the time we choose to use protobuf Struct. The issue with protobuf Struct is how it holds number value, it only supports double. When we want to hold uint64_t we need to statically cast that into a double. The problem is when we serialize it, through protobuf MessageToJsonString it is possible for that value to be rendered in scientific notation as reported in: https://github.com/envoyproxy/envoy/issues/9341#issuecomment-577051483. This is due to how protobuf's strutil renders double: https://github.com/protocolbuffers/protobuf/blob/422053f3bcb39cac483d2769e936c473e7c8bcdb/src/google/protobuf/stubs/strutil.cc#L1248-L1290. We can create a new message structure with uint64 field, but MessageToJsonString strictly follows json proto3 spec which renders uint64 to string only: https://github.com/protocolbuffers/protobuf/issues/5015#issuecomment-411847550.

Per https://github.com/envoyproxy/envoy/issues/9341 should we keep it as a string, however: https://github.com/openzipkin/zipkin-go/pull/161, seems like uint64 is final for zipkin? Or we are OK to have a "patch" in our codebase on that part: https://github.com/envoyproxy/envoy/pull/10400? Or going down to introduce control on how protobuf renders unit64_t/double.

cc. @junr03 @htuch

Guida contributor