rnburn / zipkin-cpp-opentracing

OpenTracing Tracer implementation for Zipkin in C++
Apache License 2.0
51 stars 45 forks source link

Constructing a tracer with a sample rate from JSON fails #24

Closed pingles closed 6 years ago

pingles commented 6 years ago

I'm in the process of updating the nginx opentracing module and noticed we had errors reporting that constructing a tracer was failing having updated to JSON.

I've just added a new test that fails: https://github.com/pingles/zipkin-cpp-opentracing/blob/master/zipkin_opentracing/test/ot_tracer_factory_test.cc#L34

I'm going to try and sort it asap but wanted to report here for visibility.

rnburn commented 6 years ago

Thanks for pointing it out.

Not sure if this is causing the problem, but noticed that there's a typo here in the schema.

pingles commented 6 years ago

D’oh- totally missed that. I’ll give it a try in a bit! I wondered also if it was a float/double conversion problem.

On Thu, 19 Jul 2018 at 17:06, Ryan notifications@github.com wrote:

Thanks for pointing it out.

Not sure if this is causing the problem, but noticed that there's a typo here https://github.com/pingles/zipkin-cpp-opentracing/blob/master/zipkin_opentracing/tracer_configuration.schema.json#L41 in the schema.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rnburn/zipkin-cpp-opentracing/issues/24#issuecomment-406330225, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAEfoN44qy-Z5carCmX8jJJcdM3eB8oks5uIK5-gaJpZM4VWgHz .

pingles commented 6 years ago

float isn't a valid type either, sorry: https://cswr.github.io/JsonSchema/spec/basic_types/

rnburn commented 6 years ago

Fixed in #25