Stacked traces should not be deactivated

[zalando]

Stacked traces should not be deactivated

[whiskeysierra]

It actually supports stacking, see Usage, but it's different from what you want. It effectively suspends the first, starts/stops a new one and resumes the first one afterwards. You'll basically only see one trace id at all times.

[JonasJurczok]

This is exactly why I opened this issue :)

[whiskeysierra]

Implementing stacking in the way you probably need it would require a breaking API change, namely Trace.getValue() can no longer return a single value but rather an array. Alternatively we could add Tracer.getValues() and restrict the semantics of getValue() to return the inner trace.

Anyway, I'd like to avoid this, as stacking (in the current sense) was meant to be transparent to users/integrations. E.g. listeners that were written for non-stacked traces are expecting pairs of onStart/onStop calls. By suspending and resuming the outer trace we were able to still honor that contract and we didn't need to touch the generators at all.

Am I assuming correctly that for you to be satisfied it would be enough to see both Trace IDs in the log file? If that's the case I had an idea back then that I didn't implement but that might solve your issue:

I'd suggest to introduce a special type of TraceListener (StackedTraceListener extends TraceListener). It won't define any additional methods or whatever, it just specifies a different contract and that would expect stacked calls like this (assuming we have an outer trace A and an inner one B):

• onStart A
• onStart B
• onStop B
• onStop A

compared to the sequence of calls that a traditional generator expects:

• onStart A
• onStop A
• onStart B
• onStop B
• onStart A
• onStop A

The stacked tracer implementation could partition both types of listeners and honor their contract separately.

Now, given that we have that interface and the implementation, I'd suggest to introduce a new module tracer-log4j (or tracer-log4j2) that has an NDCTraceListener that implements this. It's a seperate module because NDC (or Thread Context Stack as it is called now) is log4j specific and not abstracted away in slf4j.

What do you think?

[whiskeysierra]

I just realized that the Thread Context Stack (as well as the NDC) is not named, i.e. you only have one. That might be a problem if you have two different Traces (e.g. X-Trace-ID and X-Flow-ID) that are both stacked.

[whiskeysierra]

@JonasJurczok ^

[JonasJurczok]

Hey, sorry.. I was busy with other parts of the software..

This sounds pretty much like what I had in mind.

Why not use the MDC?
It basically behaves like a map (so your "two traces" problem is fixed). The downside is that it only supports one value per name... You'll have to build some strange string concatenation system then to build this..

What do you think?

[whiskeysierra]

You'll have to build some strange string concatenation system then to build this..

Basically a stack build as a single string. Something that I'd like to avoid, but yes that's a possibility.

[whiskeysierra]

Another downside is that the concept of multiple active traces is only visible in the logs but nowhere else.

[whiskeysierra]

@JonasJurczok I created #59, please take a look

[whiskeysierra]

@JonasJurczok I deployed 0.11.0 to central.