zipkin-api icon indicating copy to clipboard operation
zipkin-api copied to clipboard

int64 format not specified in some (at least one?) location

Open philipbawn opened this issue 4 years ago • 6 comments

https://github.com/openzipkin/zipkin-api/blob/master/zipkin2-api.yaml line 319 has type: integer, but format: int64 is not specified. due to this, my client generator (nswag) made the Annotation.timestamp property an int32 which of course blows up. Is this yml file the source of truth or is something generating it? If the yaml file can just be edited i can go through it and see if there is this problem anywhere else and make a PR.

Thanks.

philipbawn avatar Feb 23 '21 23:02 philipbawn

The YAML file is maintained by hand. A contribution would be very welcome, thanks

devinsba avatar Feb 23 '21 23:02 devinsba

+1

José Carlos Chávez

ons. 24. feb. 2021 kl. 00:44 skrev Brian Devins-Suresh < [email protected]>:

The YAML file is maintained by hand. A contribution would be very welcome, thanks

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openzipkin/zipkin-api/issues/93#issuecomment-784611571, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYATJGXZDASFR7NPENX3TAQ4WFANCNFSM4YDMG75A .

jcchavezs avatar Feb 24 '21 04:02 jcchavezs

I'm looking through it now as well as the output from my client generator :)

philipbawn avatar Feb 24 '21 05:02 philipbawn

This is solved by this PR https://github.com/openzipkin/zipkin-api/pull/94 which makes the swagger file work well with nswag. I at first thought write operations were already OK, but before encountering issues with the yaml I didn't try any of those. It turns out the timestamp annotation is used both for retrieval as well as posting spans, which makes sense.

philipbawn avatar Feb 25 '21 01:02 philipbawn

You know what.. I don't see how writes would work if someone generated a client using this before the modification. Here is what the current master 7692ca7 looks like on my screen:

span

Maybe I'm agonizing over this too much but I do worry about breaking other people's stuff.

Here is my client after the fix from the PR. span2

That looks really good. I'm using NSwag v13.0.5.0

philipbawn avatar Feb 25 '21 02:02 philipbawn

The opentelemetry-dotnet people appear to have wrote their own class (rather than generate one from the swagger file) which makes use of long data type, similar to my result after adding the format to this YAML file. This must be why I did not encounter any problems sending telemetry to zipkin but only querying from it when I raised the issue. I am using their package for sending telemetry.

philipbawn avatar Feb 25 '21 02:02 philipbawn

thanks for the PR @philipbawn, so sorry that it didn't resolve earlier. cc @reta also, as I noticed I wasn't watching this repo :blush:

codefromthecrypt avatar Jan 17 '24 15:01 codefromthecrypt