Create shared libraries for reporting zipkin spans on the JVM

codefromthecrypt commented 8 years ago

From @adriancole on April 30, 2016 0:50

Those writing instrumentation on the JVM have fairly undifferentiated tasks, putting spans on a transport for collection. For example, code in htrace, brave and spring-cloud-sleuth are nearly identical, except the recording step (taking a library-specific type into a zipkin representation). Most of these projects lag in transport diversity because they have to individually write support for kafka, etc.

By adding a Reporter to zipkin-java, those who wish to share a library (or repackage it internally), can opt-into a tested, dependency-light plugin to report data to zipkin, which they don't have to maintain.

Some possible outcomes include..

A load generator tool, which can compare performance using any of the zipkin transports #45
a generic reporter which makes it possible for finagle to send to any zipkin transport, instead of just scribe
reducing maintenance in brave by removing the 3 span collectors (reporters) in favor of a generic one
increase diversity of spring-cloud-sleuth beyond just reporting to http
replace or augment the htrace zipkin transport, so that it reaches beyond scribe and kafka and shares tests

Copied from original issue: openzipkin/zipkin-java#181

codefromthecrypt commented 8 years ago

cc @devinsba @kristofa @eirslett @michaelsembwever @marcingrzejszczak @clehene @mansu @klingerf @CodingFabian

codefromthecrypt commented 8 years ago

From @clehene on May 8, 2016 18:45

This could be a good way to provide some useful cross library reusability.

One of my goals with the htrace zipkin transport was to push it up and provide a general transport for htrace, so I kept it decoupled from the actual htrace - zipkin translation logic (note here that the list of spans is just a list of byte arrays.

I think it would be worth to maintain this decoupling so that the transporters ;) could be reused for other trace payloads. In fact this has prompted me into thinking a common tracing protocol may be useful. 1) it would provide lossless translation between tracing libraries 2) it would enable common tracing logic that could be shared across libraries.

2) is the aspect that matters here as the protocol could be associated with a common interface, so instead of List<byte[]> we'd have a less opaque List<Span> (note that that would be the "common" - think opentracing) which would allow us to have something less opaque in the message structure too. Having something "less opaque" in the message structure could be useful when you want to fanout tracing data to different backend systems (e.g. use it for aggregating analytics in addition to tracing, or something similar) and you'd need some common way to drive routing decisions.

I realize this would probably be a bit more than the intended scope for this issue, and I'm not trying to (necessarily) scope creep this ;), just to have you think about it and perhaps have that as a next step.

If, on the other hand, we were to have something in that direction, I guess the generic transport part could live in opentracing and zipkin would provide just the specifics? Again, I haven't fully thought this through, so if it sounds ambiguous, it's likely because it is ambiguous :).

On a different note, there seems to be a terminology mismatch between zipkin-java and brave. For zipkin, a collector seems to be the consumer part, while in brave it seems to be the producer

codefromthecrypt commented 8 years ago

On a different note, there seems to be a terminology mismatch between zipkin-java and brave. For zipkin, a collector seems to be the consumer part, while in brave it seems to be the producer https://github.com/openzipkin/brave/blob/master/brave-spancollector-kafka/src/main/java/com/github/kristofa/brave/kafka/KafkaSpanCollector.java#L98

Agreed. here's the issue to correct this: https://github.com/openzipkin/brave/issues/173

codefromthecrypt commented 8 years ago

Thanks for breaking this down. I had thought through the List<byte[]> thing, but maybe from a different direction. The TL;DR; is I don't think the idea of a generic transport is incompatible with this, we just have to be careful how it is designed.

--- Details

In HTrace List<byte[]> indicates mapping each byte[] to a separate message, and that the byte[] is presumed to describe a single span.

In Zipkin, a byte[] collected is generally a List, even if it was formerly a single Span per byte[].

The difference here is that the encoding might be list or not, and.. how do you choose how to join the byte[]s, if you don't know the encoding (as it is opaque). Do you join them as character buffers on json commas? Or do you join them as TBinaryProtocol? or maybe protobuf3?

I've not been paying attention for a few weeks, but I recall it was a non-goal of OpenTracing to indicate transport details such as encoding, but let's say it did. We wouldn't get past the reality that we need routing info to be present. Ex some sort of tag to indicate how those byte[] were created.. at least if the aim was to report them into a single topic. I'm thinking media types, for example.

All this said, don't get me wrong :) my aim isn't to preclude the idea of a "tracing topic" where multiple systems report in their native language and collectors can take the task of trying to parse them or not. We'd need this for zipkin 2 anyway. Main thing I'm calling out is..

We can't assume a 1-1 mapping of List -> List<byte[]> -> List, where message in Kafka, Scribe, Http, etc, even if it looks very tempting :)
However, if the encoder is known, or we have a way to tag the bytes to indicate the content, we can still achieve the same goal.

Ex. A generic reporter could act in the following flow:

SpanRecorder {
  record(S span) // where in zipkin, this realizes to zipkin.Span, and schedules the span to be reported
  (byte[], encoding) drainToMessage(Queue<S> pending); // this pulls spans into a message, where N >=1
}

SpanEncoder {
  encoding();

  byte[] encode(S);
  byte[] encode(List<S> span); // because the list form is an encoding detail
}

report(byte[] message, encoding) // the actual reporter interface

All of the above could be defined in a java interface without knowing the type of the span, eventhough in practice, the plugin would know that. ex a zipkin-specific implementation, such as what brave would expose, wouldn't need to expose givens.

ZipkinSpanReporter {
  public record(Span span)

      byte[] drainToMessage(Queue<Span> pending); // internal

  pubic report(byte[] message) // encoding is set implicitly
}

Note that the implementation isn't likely to keep like this anyway, as we've learned that byte bounds are very important. We're more likely to implement this as an incrementally encoding thing like finagle, where the "finished spans" are encoded into a buffer that's eventually flushed to a message.

codefromthecrypt commented 8 years ago

another related issue to this https://github.com/openzipkin/zipkin/issues/1126

codefromthecrypt commented 8 years ago

another request for common reporters https://github.com/Nike-Inc/wingtips/pull/10

codefromthecrypt commented 8 years ago

pinging @devinsba @kristofa @eirslett @michaelsembwever @marcingrzejszczak @clehene @mansu @klingerf @CodingFabian @nicmunroe as I created a new repostory to carry this effort forward

codefromthecrypt commented 8 years ago

initial work here: https://github.com/openzipkin/zipkin-java-reporter/pull/2

codefromthecrypt commented 8 years ago

Here's a couple changes which allow for opaque reporting (as requested by @clehene and implemented in HTrace)

https://github.com/openzipkin/zipkin-reporter-java/pull/6 <- extract new Sender interface with void sendSpans(byte[] spans, Callback callback) https://github.com/openzipkin/zipkin/pull/1247 <- add Adds byte[] gather(List<byte[]> value) to Codec to combine externally encoded spans into one message

codefromthecrypt commented 7 years ago

umm this is probably closed :)

openzipkin / zipkin-reporter-java

Create shared libraries for reporting zipkin spans on the JVM #1