Annotations.Tag bug while string value has double quotation marks

[anthonywanted]

Version: 1.1.0 on nuget
When Annotations.Tag has a bug when string value contains double quotation marks
Code:

class Program
{
    static void Main(string[] args)
    {
        var logger = new ZipkinLogger(); //It should implement ILogger
        var sender = new HttpZipkinSender("http://192.168.230.36:9411", "application/json"); // It should implement IZipkinSender
        TraceManager.SamplingRate = 1.0f; //full tracing
        Statistics statistics = new Statistics();
        var tracer = new ZipkinTracer(sender, new JSONSpanSerializer(), statistics);
        TraceManager.RegisterTracer(tracer);
        TraceManager.Start(logger);
        var trace = Trace.Create();
        trace.Record(Annotations.ClientSend());
        trace.Record(Annotations.ServiceName("TestZipkinClient"));
        trace.Record(Annotations.Rpc("123"));
        trace.Record(Annotations.Tag("hello", "\"a\"")); // here
        trace.Record(Annotations.ClientRecv());
        //On shutdown
        TraceManager.Stop();
    }
}

It seems like some bug here:
https://github.com/openzipkin/zipkin4net/blob/master/Src/zipkin4net/Src/Tracers/Zipkin/JSONSpanSerializer.cs

    private static void SerializeBinaryAnnotation(StreamWriter writer, BinaryAnnotation binaryAnnotation, IPEndPoint endPoint, string serviceName)
    {
        writer.Write(openingBrace);
        writer.WriteField(key, binaryAnnotation.Key);
        writer.Write(comma);
        writer.WriteField(value, Encoding.UTF8.GetString(binaryAnnotation.Value)); // HERE!
        writer.Write(comma);
        writer.WriteAnchor(endpoint);
        SerializeEndPoint(writer, endPoint, serviceName);
        writer.Write(closingBrace);
    }

Zipkin (latest docker image) server throw Exception:

2017-11-02 03:36:56.847 WARN 7 --- [nio-9411-exec-6] zipkin.server.ZipkinHttpCollector : Cannot decode spans due to IllegalArgumentException(Unterminated object at line 1 column 354 path $[0].binaryAnnotations[0].value reading List from json: [{"id":"7b03e3fb6a7e604c","name":"123","annotations":[{"timestamp":"1509593478064678","value":"cs","endpoint":{"ipv4":"-1407295487","port":"0","serviceName":"TestZipkinClient"}},{"timestamp":"1509593478066500","value":"cr","endpoint":{"ipv4":"-1407295487","port":"0","serviceName":"TestZipkinClient"}}],"binaryAnnotations":[{"key":"http.url","value":""a"","endpoint":{"ipv4":"-1407295487","port":"0","serviceName":"TestZipkinClient"}}],"debug":False,"traceId":"8c049b5003363a81","timestamp":"1509593478064678","duration":"1822"}])

java.lang.IllegalArgumentException: Unterminated object at line 1 column 354 path $[0].binaryAnnotations[0].value reading List from json: [{"id":"7b03e3fb6a7e604c","name":"123","annotations":[{"timestamp":"1509593478064678","value":"cs","endpoint":{"ipv4":"-1407295487","port":"0","serviceName":"TestZipkinClient"}},{"timestamp":"1509593478066500","value":"cr","endpoint":{"ipv4":"-1407295487","port":"0","serviceName":"TestZipkinClient"}}],"binaryAnnotations":[{"key":"http.url","value":""a"","endpoint":{"ipv4":"-1407295487","port":"0","serviceName":"TestZipkinClient"}}],"debug":False,"traceId":"8c049b5003363a81","timestamp":"1509593478064678","duration":"1822"}]
at zipkin.internal.JsonCodec.exceptionReading(JsonCodec.java:716) ~[io.zipkin.java-zipkin-2.0.1.jar!/:na]
at zipkin.internal.JsonCodec.readList(JsonCodec.java:685) ~[io.zipkin.java-zipkin-2.0.1.jar!/:na]
at zipkin.internal.JsonCodec.readSpans(JsonCodec.java:501) ~[io.zipkin.java-zipkin-2.0.1.jar!/:na]
at zipkin.collector.Collector.decodeList(Collector.java:152) [io.zipkin.java-zipkin-2.0.1.jar!/:na]
at zipkin.collector.Collector.decodeList(Collector.java:41) [io.zipkin.java-zipkin-2.0.1.jar!/:na]
at zipkin.internal.Collector.acceptSpans(Collector.java:54) ~[io.zipkin.java-zipkin-2.0.1.jar!/:na]
at zipkin.collector.Collector.acceptSpans(Collector.java:114) [io.zipkin.java-zipkin-2.0.1.jar!/:na]
at zipkin.server.ZipkinHttpCollector.validateAndStoreSpans(ZipkinHttpCollector.java:98) [classes/:na]
at zipkin.server.ZipkinHttpCollector.uploadSpansJson(ZipkinHttpCollector.java:75) [classes/:na]
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_131]
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_131]
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~

[fedj]

Hi @anthonywanted,

Thank you for reporting this bug.
You're right, it needs to be escaped when using json format.

Would you be able to come with a PR ?

In the meantime, as a temporary workaround, you can use thrift transport (it should not have this bug).

[anthonywanted]

OK，I will make a PR soon