envoyproxy/envoy

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

Open

#10.411 geöffnet am 16. März 2020

Auf GitHub ansehen
 (12 Kommentare) (0 Reaktionen) (0 zugewiesene Personen)C++ (5.373 Forks)batch import
area/protobufhelp wanted

Repository-Metriken

Stars
 (27.997 Stars)
PR-Merge-Metriken
 (Durchschn. Merge 8T) (378 gemergte PRs in 30 T)

Beschreibung

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

Contributor Guide