Skip to content
This repository has been archived by the owner on Jan 4, 2023. It is now read-only.

Use RFC 3339 string representation of google.protobuf.Timestamp #98

Closed
shaneqld opened this issue Jul 10, 2019 · 10 comments
Closed

Use RFC 3339 string representation of google.protobuf.Timestamp #98

shaneqld opened this issue Jul 10, 2019 · 10 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@shaneqld
Copy link

When using type google.protobuf.Timestamp, BloomRPC maps this to an object, e.g.:

"create_time": {
  "seconds": "1562723637",
  "nanos": 928852400
}

However, the Google API Design Guide specifies that the string representation should be RFC 3339 format, e.g. "2014-07-30T10:43:17Z"

This JSON mapping is used by Cloud Endpoints and Envoy grpc-json transcoding. Regardless, an RFC 3339 string is much easier to work with by hand than a Unix timestamp.

I'd like to submit this as a feature/request for changing how timestamps are transcoded to/from json. ps- love the tool, very handy!

@fenos
Copy link
Contributor

fenos commented Jul 10, 2019

@shaneqld Hi there!

I'm not sure i follow, the google.protobuf.Timestamp message type has got 2 fields, which represent the date, you can see it here:

https://github.com/protocolbuffers/protobuf/blob/master/src/google/protobuf/timestamp.proto#L127

That's why bloomrpc tries to fill the fields for this type.

i think you are looking for is a kind of helper function which will convert a date RFC 3339 into this kind of message type?

ex (not implemented just yet):

{
    "date": "{{dateToProtoTimestamp('2014-07-30T10:43:17Z')}}"
}

@shaneqld
Copy link
Author

Hi @fenos,

You are correct. My suggestion would be to use a custom serialiser for google.protobuf.Timestamp types. Converting automatically to/from RFC 3339 is performed by Envoy and Cloud Endpoints when transcoding between gRPC and http1/json.

For example, with this simple proto message:

message Project {
  string name = 1;
  google.protobuf.Timestamp create_time = 4;
}

With Envoy or Cloud Endpoints, we receive the following json:

{
  "name": "projects/test-project",
  "create_time": "2014-07-30T10:43:17Z"
}

With BloomRPC, it looks more like:

{
  "name": "Hello",
  "create_time": {
    "seconds": 54646512,
    "nanos": 1235
  }
}

This is not a problem. However, if the purpose of BloomRPC is to allow calling/testing gRPC services using json, then it's worth considering following how others (especially those in the cloud native computing foundation) have implemented serialization.

It's also much simpler writing an RFC 3339 string than manually doing the conversion first or remembering custom syntax.

Just my 2 cents :)

@fenos
Copy link
Contributor

fenos commented Jul 23, 2019

@shaneqld this feature is coming soon

@foresx
Copy link

foresx commented Jan 19, 2020

any updates news now?

@vbhale
Copy link

vbhale commented Sep 17, 2020

@fenos When will the feature be released ?
JSONFormat provided by google converts the Timestamp in RFC 3339 format rather than this object style of timestamp representation used any bloomrpc. Also it is true for any other object used by proto for time representation for e.g. google.protobuf.Duration
it would be used by bloomrpc as
"sla": { "max_timeout": { "seconds": 5, "nanos": 10 } }

while JSONFormat would represent it in JSON format as
"sla": { "max_timeout": "5s" }

This feature would be useful since it would make users use the dates in user friendly format and other tooling provided google to generate the JSON message from protobuf supports this.

@jackielii jackielii added enhancement New feature or request good first issue Good for newcomers labels Nov 9, 2020
@jameslamine
Copy link

jameslamine commented Apr 18, 2021

I'm running into the same issue. Support for displaying protobufs in their canonical json representation would be really useful. Right now the json which bloomRPC outputs cannot be parsed by google's protobuf json parser because it doesn't correctly format Timestamp, and wrapper types like StringValue, IntValue.

For example timestamps should be formatted as RFC-3339 strings like "1972-01-01T10:00:20.021Z".
Wrapper types like StringValue should be formatted like:

{
   "myStringValue": "example string"
}

instead of:

{
  "myStringValue": {
     "value": "example string"
  }
}

For documentation on the canonical json encoding for proto3, see https://developers.google.com/protocol-buffers/docs/proto3#json

After some digging, it looks like this may be an issue with the upstream protobufjs project. See protobufjs/protobuf.js#1304

Rather than waiting for this to be fixed in protobufjs, my suggestion is to set up custom classes for Timestamp and the various wrapper types. https://github.com/protobufjs/protobuf.js#using-custom-classes

@Tochemey
Copy link

Heyo what is the outcome of this issue. It seems it has been overlooked.

@JakubSzczepek
Copy link

Any updates in this issue. Still timestamp need to be converted to key: {"value": timestamp} object

@dan-autonomic
Copy link

dan-autonomic commented Sep 24, 2022

Also contributing. The fact that BloomRPC doesn't parse it is something I like from BloomRPC because sometimes the nanoseconds field could be out of the proper range per specification, and other clients, like gRPCurl break when those are not perfect, but BloomRPC returns the two values, regardless if those meet specification, which is good, because a client is not guilty for a service providing a wrong timestamp.
Thus BloomRPC enables us to consume even when nanoseconds value is not falling within the spec range [0, 1000000000)

Not saying the parsing is not needed, in fact is better to have it, but returned data must still be shown even if it is ill set by the gRPC server. Just to take it into account.

@JakubSzczepek
Copy link

On my side I can say if I use {"value": timestamp} object my application see that element with default google.timestamp value soo it will be great to have some parsing

@gh-issue-closer gh-issue-closer bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

9 participants