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

equivalent of zipkin-java JsonCodec? #15

Closed
hakanson opened this issue Jun 17, 2016 · 10 comments
Closed

equivalent of zipkin-java JsonCodec? #15

hakanson opened this issue Jun 17, 2016 · 10 comments

Comments

@hakanson
Copy link
Contributor

zipkin-java has a JsonCodec class that is used by things like ElasticsearchSpanConsumer or spring-cloud-sleuth HttpZipkinSpanReporter. Can support for JsonCodec be added?

I am looking to leverage the "Spring Cloud Zipkin UI" (not sure of the proper name, but screenshots at https://spring.io/blog/2016/02/15/distributed-tracing-with-spring-cloud-sleuth-and-spring-cloud-zipkin) and this seems like a required prerequisite.

@eirslett
Copy link
Contributor

It would be practical, but not strictly needed. This is the way things are connected:

Your application -> Zipkin server -> Backend (e.g. ElasticSearch for Cassandra)

You can also add a queue between, e.g. Your application -> Scribe -> Zipkin server, or Your application -> Kafka -> Zipkin server. If you use the scribe transport, you don't need a queue, you can send span data directly to the Zipkin server on port 9410.

Both the Kafka transport and the Scribe transport depend on Thrift, which is a pretty heavy library; a JSON codec could potentially give us an almost dependency-free transport.

@hakanson
Copy link
Contributor Author

Would a JSON codec also help browser support (a la issue #1)?

@eirslett
Copy link
Contributor

It would help for debugging, at least - to have a human-readable message format. (But as I mentioned, it's not a requirement for browser support)

If you want to add it, feel free to send a PR and we'll have a look! :-)

@hakanson
Copy link
Contributor Author

Is JSON codec documented anywhere other that reading the java source code? I was looking for either a spec or an example last week and not having any luck.

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Jun 21, 2016 via email

@sveisvei
Copy link
Collaborator

I would also prefer JSON over http.

@hakanson
Copy link
Contributor Author

It looks like TraceId and Annotation have implemented toString methods that are used by the ConsoleRecorder. Should the JSON codec be an external class or a set of toJSON methods on the objects proper?

Also, after the hint, I found the .yaml behind that Swagge link: https://github.com/openzipkin/zipkin-api/blob/master/zipkin-api.yaml

@eirslett
Copy link
Contributor

Look at serializeSpan, which currently supports base64 and binary. Also have a look at internalRepresentations. That span is the object that must be serialized.

@jquatier
Copy link
Contributor

jquatier commented Jul 9, 2016

I actually could use this too, and I'm going to start working on it (unless anyone already has). I have a Spring app that uses the ZipkinHttpCollector and need to POST the correct JSON to it.

I was planning on making a new zipkin-transport-http package that handles translating the spans to the proper JSON format and then sending the spans using fetch or request to a configurable endpoint. If that sounds good, I also am wondering if I should keep the JSON object translation code in that package or put toJSON() methods in the internalRepresentations module (similar to the toThrift() methods). @eirslett Any thoughts?

Let me know, I'm open to suggestions and I'd like to contribute this back to zipkin-js as a PR when I'm done. Thanks all!

@eirslett
Copy link
Contributor

eirslett commented Jul 9, 2016

Great initiative! Put the serialization in the same place thrift serialization takes place. (Maybe even add unit tests where it makes sense) We look forward to a PR, thanks a lot!

This was referenced Jul 13, 2016
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

5 participants