Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
philipbawn opened this issue Feb 23, 2021 · 7 comments
Closed

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

philipbawn opened this issue Feb 23, 2021 · 7 comments

Comments

@philipbawn
Copy link
Contributor

philipbawn commented Feb 23, 2021

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.

@devinsba
Copy link
Member

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

@jcchavezs
Copy link
Contributor

jcchavezs commented Feb 24, 2021 via email

@philipbawn
Copy link
Contributor Author

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

@philipbawn
Copy link
Contributor Author

philipbawn commented Feb 25, 2021

This is solved by this PR #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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@codefromthecrypt
Copy link
Member

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 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants