search

opentracing-contrib / java-spring-jaeger

Apache License 2.0
256 stars 95 forks source link

use NoopTracer for disabling tracing #80

Closed gerloh closed 4 years ago

gerloh commented 4 years ago

The suggested implementation for disabeling in the readme uses a ConstSamplerand a InMemoryReporter . This is still tracing but not sampling (sending) the traces. Why not NoopTracerImpl by opentracing instead ? Also the InMemoryReporter has a memory leak ? It is only gathering spans in an arraylist, never releases them ... https://github.com/opentracing-contrib/java-spring-jaeger#completely-disable-tracing

geoand commented 4 years ago

Hi,

Would you like to send a PR with your suggestion?

pavolloffay commented 4 years ago

Isn't it better to just disable the Jager or opentracing-spring-cloud artifacts in spring configuration?

IIRC if the jaeger is disabled the noop tracer might be used by default. I am not sure though.

gerloh commented 4 years ago

I mean this : https://github.com/opentracing-contrib/java-spring-jaeger/pull/82

gerloh commented 4 years ago

Isn't it better to just disable the Jager or opentracing-spring-cloud artifacts in spring configuration?

IIRC if the jaeger is disabled the noop tracer might be used by default. I am not sure though.

Probably, but readme suggested that's not a solution ?
https://github.com/opentracing-contrib/java-spring-jaeger#completely-disable-tracing

geoand commented 4 years ago

PR merged