envoyproxy / envoyproxy/envoy

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

Open
#10,411 12 comments 0 reactions 0 assignees View on GitHub
area/protobuf help wanted
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 22h
Merged PRs (30d)
430

Description

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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.