Spring-cloud-sleuth: Spans sent via Stream are Zipkin incompatible

[marcingrzejszczak]

ATM the Spans object in StreamSpanReporter is not compatible with the structure that is expected by Zipkin which is List<zipkin.Span>. At the moment ZipkinMessageListener is only doing a conversion:

@ServiceActivator(inputChannel = SleuthSink.INPUT)
    public void sink(Spans input) {
        List<zipkin.Span> converted = ConvertToZipkinSpanList.convert(input);

We could do this conversion before sending a message to any topic. That way, quoting @adriancole :

I mean sleuth's stream with this relatively small change can be a general purpose collector for any instrumentation

cc @dsyer

[codefromthecrypt]

fwiw, here are the supported encodings for kafka in zipkin:

---

Encoding spans into Kafka messages

The message's binary data includes a list of spans. Supported encodings
are the same as the http POST /spans body.

Json

The message's binary data is a list of spans in json. The first character must be '[' (decimal 91).

Codec.JSON.writeSpans(spans) performs the correct json encoding.

Here's an example, sending a list of a single span to the zipkin topic:

$ kafka-console-producer.sh --broker-list $ADVERTISED_HOST:9092 --topic zipkin
[{"traceId":"1","name":"bang","id":"2","timestamp":1234,"binaryAnnotations":[{"key":"lc","value":"bamm-bamm","endpoint":{"serviceName":"flintstones","ipv4":"127.0.0.1"}}]}]

Thrift

The message's binary data includes a list header followed by N spans serialized in TBinaryProtocol

Codec.THRIFT.writeSpans(spans) encodes spans in the following fashion:

write_byte(12) // type of the list elements: 12 == struct
write_i32(count) // count of spans that will follow
for (int i = 0; i < count; i++) {
  writeTBinaryProtocol(spans(i))
}

[neilmookerjea]

Is the naming convention a bit overlapping? If I understand it to some extent, spring-cloud-sleuth-stream is be used with the trace producer app config. Whereas spring-cloud-sleuth-zipkin-stream is about the collector which can be on its own (with binders and span store configuration of course). Could there be just spring-cloud-sleuth-zipkin-stream for client and a spring-cloud-sleuth-zipkin-stream-collector for the collector, whereas, both of them has the configuration to enable or disable zipkin compatible traces.
But curious to know why the sleuth traces do not really carry cs, sr, ss, cr like what zipkin does. Why did the trace need to differ?
At least, perhaps having ConvertToZipkinSpanList based on a configuration in the spring-cloud-sleuth-stream library (then does it need to be renamed?) would be fair.

[codefromthecrypt]

lastest thoughts on this is that.. xxxx-zipkin-xxxx should always really use zipkin format :) regardless of transport.

If for historical (or outdated technical) reasons we aren't sending or receiving native zipkin structs, we can probably change the library name or something to one that does.

recently, we support zipkin.reporter.Reporter (which could be implemented by spring-cloud-stream), giving us a new option: make a spring-cloud-stream reporter bridge! Then, regardless of if zipkin or a stream listener is at the other side of Kafka, we're good.

cc @mbogoevici

[codefromthecrypt]

re-pinging interest on this. Just got a request for a rabbit collector in zipkin, and it would be nice to re-use the code without the sleuth-specific wire format

[patilvikram]

@adriancole @marcingrzejszczak I am planning to work on this improvment . Any suggestion how to start with this one will be helpful.

[codefromthecrypt]

cool. just to recap, the format is the same as mentioned earlier.

The main change that would happen is that you convert prior to sending on a stream instead of after. for example, ZipkinMessageListener does ConvertToZipkinSpanList.convert. This would take place on the producing side.

Hope this helps

[codefromthecrypt]

ps once you have the list, you'd use zipkin.Codec.THRIFT.writeSpans(listOfSpans) (or json depending on config) to write the message body

[marcingrzejszczak]

The Stream server got deprecated so eventually this problem will go away - https://github.com/spring-cloud/spring-cloud-sleuth/issues/727