Feature Request: Add Tracing.noop option to command line

[andye2004]

Add a java -D option for setting noop true/false to command line.

[codefromthecrypt]

usually we don't had configuration bindings to brave as it is a library abstraction, though this particular one could make sense

what type of apps are you deploying where a framework agnostic flag like this would help?

[andye2004]

Hi @adriancole, thanks for the follow up.

Reason I'm asking is that we are using Brave/Zipkin to trace requests through a full blown enterprise application which, due to legacy design, doesn't run a request as a single thread-per-request / transaction end to end, but ends up partly processing requests then dropping results on to JMS queues for further processing. We want tracing to be OFF by default and only turned on via a 'switch' when required. Our problem is that due to context issues I need to Trace.setNoop(true) in multiple locations using various start-up hooks which just isn't ideal.

A single VM wide attribute used within the Trace class really would simplify things greatly.

[codefromthecrypt]

ok I think you mean Tracing, not Trace, but at any rate same concern.

"brave.enabled=false" or "brave.noop" come to mind. The former is usual in spring config, the latter is more explicit.

cc @sirtyro @jonathan-lo any ideas?

[andye2004]

@adriancole apologies mate, I did of course mean Tracing.

For what it's worth we set it to noop true by default and use "-Dbrave.trace.enabled=true" on the command line to turn it on. However we can obviously change it easily to match whatever is used within the brave library.

[ghost]

It seems reasonable to me - after all, with the Spring Boot world you can control a lot of things with properties so this doesn't seem that far out of such convention.

I would, however, go with brave.noop rather than brave.enabled, because technically 'enabled' would be lying right? Its really doing noop tracing, which has some active components rather than being turned off completely?

[devinsba]

Borrowing from census it might make sense to have it be brave.mode where options are: ENABLED, NOOP, and DISABLED since people might want to disable things completely

[andye2004]

@devinsba I'm assuming that although you would have the three options for mode that things would still be ENABLED by default?

[ghost]

@devinsba, what's the difference between NOOP and DISABLED in brave?

[devinsba]

Disabled to me would mean: no trace ID generation and no propagation of any kind

[codefromthecrypt]

I think disabled would be lying unless we wove such thing everywhere... census has disabled because they routinely use static globals to access the tracer.

[codefromthecrypt]

how about this.. -Dbrave.tracing.noop? maps pretty directly to brave.Tracing.setNoop(true) which we won't deprecate for a long time anyway

[andye2004]

@adriancole That sounds good to me 👍

[jorgheymans]

@adriancole so one should do setNoop(true) rather than Sampler.NEVER_SAMPLE ? Or are these equivalent ?

[codefromthecrypt]

When set at startup it is almost exactly like sample rate 0. I can look at the docs closely to tell the difference. In case a question is coming, I dont want to start cherry picking config to expose as system properties as it confuses configuration layer and rots the code. Iotw, kill switch is reasonable as it isnt a slippery slope to a half implemented property system ;)

On 27 Oct 2017 19:57, "Jorg Heymans" notifications@github.com wrote:

@adriancole https://github.com/adriancole so one should do setNoop(true) rather than Sampler.NEVER_SAMPLE ? Or are these equivalent ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openzipkin/brave/issues/527#issuecomment-339951500, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD618t2_5xF6xEgNdo8f0rdF-bDXOvbks5swcUvgaJpZM4QDpji .

[Logic-32]

FWIW, one solution my company had to this problem was to actually swap out the Reporter if the flag was "off". Basically, we have an implementation of AsyncReporter that is a noop similar to a noop span. In hindsight, I think it'd be better if we changed the Sampler. We'll look into that minor issue on our side but I will also be keeping an eye out for this global flag if it comes along.

One thing to keep in mind is that we also disable various brave-wrappers if we have it set to disabled (e.g. ExecutorService decorators).

[codefromthecrypt]

@Logic-32 thx for the feedback. One thing about noop is it doesn't disable ID provisioning, so executor wrappers are still valid (log correlation etc isn't disabled, it is data collection and reporting)