search

ahrefs / atd

Static types for JSON APIs
Other
316 stars 52 forks source link

Encode int64 #273

Open AndreyErmilov opened 2 years ago

AndreyErmilov commented 2 years ago

Hi, After upgrading atdgen from version 2.2.1 to 2.3.3, I saw that all int64 numbers in the service response are converted to strings. There is issue #231 about it. That's okay, but there is a case where it doesn't fit. Is it possible to add a way to customize this behavior? For example, by default int64 numbers will remain numbers, but when adding a parameter they will be converted to strings.

rr0gi commented 2 years ago

there is a case where it doesn't fit.

what do you mean "doesn't fit"?

AndreyErmilov commented 2 years ago

I use timestamp with nanosecond accuracy. This is needed for tracing to work correctly (in my case it is zipkin). In order to express timestamp with nanoseconds I need to use int64.

AndreyErmilov commented 2 years ago

I need to use Zipkin and Zipkin expects integers (timestamp field). I don't have the choice to convert int64 to string or not. If I had control over this behaviour it would be very useful.

Lupus commented 2 years ago

Zipkin V2 API requires timestamp field to be integer: https://github.com/openzipkin/zipkin-api/blob/master/zipkin2-api.yaml#L319

    properties:
      timestamp:
        type: integer
        description: |
                    Epoch **microseconds** of this event.
                    For example, 1502787600000000 corresponds to 2017-08-15 09:00 UTC
                    This value should be set directly by instrumentation, using the most precise
                    value possible. For example, gettimeofday or multiplying epoch millis by 1000.

Is there a way to express this in atd after #231 ? It does not fit in 32 bits.

RFC 8259, Section 6 tells us that valid JSON allows arbitrary precision, and many implementations allow 64-bit values to be expressed natively, without string encoding.

zindel commented 2 years ago

I am more comfortable with keeping the string representation of int64 by default. But I agree that there are cases when you need to have it be int.

How about we use the same trick as with float: https://atd.readthedocs.io/en/latest/atdgen.html#floats ? E.g. for the example above:

type timestamp = int <ocaml repr="int64"> <repr="int">
Lupus commented 2 years ago

Using string for int64 representation in JSON by default is a sane choice. Same trick as with float should do the trick (pun intended). Actually we worked around this by using float with int representation for now...

mjambon commented 1 year ago

I'm confused. Sorry I didn't realize this earlier.

The following ATD definition:

type int64 = int <ocaml repr="int64">

means that in all the target languages except OCaml, the data will use the default representation of int. For JSON, it's a number, not a string. The current implementation of atdgen uses the function Oj_run.write_int64 which writes a JSON string rather than a JSON number. This is incorrect.

Valid ATD code for representing an OCaml int64 as a JSON string would be:

type int64 = string <ocaml repr="int64">

or

type int64 = int <ocaml repr="int64"> <json repr="string">