-
Notifications
You must be signed in to change notification settings - Fork 4
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
Factor out Kafka span collector and high-level API over Brave. #2
Comments
this might be mergable with the kafka collector which is recently in Brave https://github.com/openzipkin/brave/blob/master/brave-spancollector-kafka/src/main/java/com/github/kristofa/brave/kafka/KafkaSpanCollector.java |
Well, KafkaZipkinTracing we have there is more like a high-level interface for Brave. Basically it served two purposes: one is to provide Kafka is a transport. This is the part we can get rid of now when there's a KafkaSpanCollector in Brave. So this is cool. We don't need to merge something into existing collector, we'll use the default one. |
Thanks for the summary.
Yeah I noticed also how rpc semantics are baked into brave's apis. There's
a new LocalTracer which special cases non-rpc spans, but even that doesn't
fit precisely the scenario you've described.
I'm likely to help make a raw tracer in brave, similar to the one you have
in go (and similar to OpenTracing). That said, there's no rush to move this
code around.. It is nice that you've captured a way to accomplish things
now.
Fwiw zipkin v2 model is likely to be one-way by default, which would help
with some of this openzipkin/zipkin#939
Regardless of all ^^ Thanks for sharing your work!
|
We may want to consider publishing Brave-related tools (https://github.com/elodina/zipkin-mesos-framework/tree/master/src/main/scala/com/github/kristofa/brave) as a separate artifact that will be re-used by other JVM-based services. Other thing to consider is also moving this to a separate repo.
The text was updated successfully, but these errors were encountered: