openzipkin / brave

Java distributed tracing implementation compatible with Zipkin backend services.
Apache License 2.0
2.35k stars 714 forks source link

json: fixes incorrect handling of error tag #1415

Closed codefromthecrypt closed 5 months ago

codefromthecrypt commented 5 months ago

Before, we didn't document that an existing tag with the same name as the errorTag parameter of the JSON writer would be retained. This documents it. It also fixes a bug where that key is not literally "error", for example, adding a tag named "exception".

I backfilled tests and adjusted benchmarks to make sure the result isn't worse than before.

Before

Benchmark                                                          Mode     Cnt    Score   Error   Units
ZipkinV2JsonWriterBenchmarks.sizeInBytes_bigClientSpan:gc.alloc.rate.norm  sample      15   0.042 ± 0.007    B/op
ZipkinV2JsonWriterBenchmarks.sizeInBytes_bigClientSpan:p0.99               sample           0.334           us/op
ZipkinV2JsonWriterBenchmarks.write_bigClientSpan:gc.alloc.rate.norm        sample      15  24.105 ± 0.008    B/op
ZipkinV2JsonWriterBenchmarks.write_bigClientSpan:p0.99                     sample           0.791           us/op

After

Benchmark                                                                    Mode     Cnt    Score   Error   Units
ZipkinV2JsonWriterBenchmarks.sizeInBytes_bigClientSpan:gc.alloc.rate.norm  sample      15    0.041 ± 0.006    B/op
ZipkinV2JsonWriterBenchmarks.sizeInBytes_bigClientSpan:p0.99               sample            0.334           us/op
ZipkinV2JsonWriterBenchmarks.write_bigClientSpan:gc.alloc.rate.norm        sample      15   24.101 ± 0.017    B/op
ZipkinV2JsonWriterBenchmarks.write_bigClientSpan:p0.99                     sample            0.750           us/op
codefromthecrypt commented 5 months ago

merging to unblock