search

openzipkin / zipkin-ruby

zipkin-tracer ruby gem
Apache License 2.0
100 stars 38 forks source link

Fixes JSON v1 encoding of remote address binary annotations #138

Closed codefromthecrypt closed 5 years ago

codefromthecrypt commented 5 years ago

The only use case for BOOL binary annotations is identifying the remote endpoint. In thrift encoding, BOOL true is represented by 1. This accidentally carried into JSON encoding, which tripped up endpoint interpretation. This fixes it by special casing BOOL.

See https://github.com/openzipkin/zipkin/issues/2414

codefromthecrypt commented 5 years ago

I manually tested by changing the gem version up a notch, installing, running zipkin-ruby-example, and noticing it looking fine

jcarres-mdsol commented 5 years ago

Interesting approach, as we are hardcoding the 1 value, I'd rather change that. I'll do that in a couple of days. Leaving this open till next pr is available.

codefromthecrypt commented 5 years ago

Interesting approach, as we are hardcoding the 1 value, I'd rather change that. I'll do that in a couple of days. Leaving this open till next pr is available.

okie dokie, though meanwhile anyone using this will be broke

jcarres-mdsol commented 5 years ago

I'm closing this in favor of #139 But more of your ruby is always welcome!

codefromthecrypt commented 5 years ago

:)