Move the current tracing feature to an independent/flexible tracing interface

[pmvilaca]

At the moment, the tracing feature belongs to the RequestContext and it only produces a log and there is now way to extend it.

With the work that is being done to integrate Zipkin in Cougar, we think that the best way to do that is extending the tracing mechanism and start publishing trace events at every interesting points in the framework (jetty handler, marshall/unmarshalling, jetty client, ...).

In order to do that, we need to find a way to create the ExecutionContext earlier because we need the UUID to do the tracing and it's being created in the AbstractHttpCommandProcessor (before the execution inside the execution venue) OR create a queue to accumulate the tracing events until we have the ExecutionContext available.

[pmvilaca]

Thinking about this.. We don't need to create the ExecutionContext earlier of keep the tracing events in memory and wait for the ExecutionContext creation..

If we create a "tracer" and inject it where we need to publish a trace event (JettyTransport/SocketTransport and AbstractHttpExecutable for Zipkin) will give us what we want without moving the ExecutionContext creation or increasing the memory usage.

[eswdd]

I agree. Ideally we could have the "tracer" as a new interface with 3 initial implementations:

• One that provides the same functionality as the current trace logging
• One for your zipkin implementation
• A compound one which wraps zero or more others and calls each in turn.

As you say this can be used prior to ExecutionContext creation, and can also be passed into the EC/RC so it can be used in current places too. If we want to change/add to the trace methods on RC this can be done at the same time.

I suggest this work be done at the same time as zipkin integration rather than seperating it.

[pmvilaca]

Makes sense..

Should we delete this issue and mention this discussion in another issue related with the Zipkin integration?

On Tuesday, January 7, 2014, Simon Matic Langford wrote:

I agree. Ideally we could have the "tracer" as a new interface with 3 initial implementations:

• One that provides the same functionality as the current trace logging
• One for your zipkin implementation
• A compound one which wraps zero or more others and calls each in turn.

As you say this can be used prior to ExecutionContext creation, and can also be passed into the EC/RC so it can be used in current places too. If we want to change/add to the trace methods on RC this can be done at the same time.

I suggest this work be done at the same time as zipkin integration rather than seperating it.

— Reply to this email directly or view it on GitHubhttps://github.com/betfair/cougar/issues/59#issuecomment-31778342 .

[eswdd]

Don't have to, we can just fix 2 issues with one pull request. This feature stands on it's own without the zipkin integration so feels right to keep it as a seperate issue.

[pmvilaca]

I'm currently working on this issue.. As a first step, I'm moving the current tracing to an interface. As we'll need to use at the entry points where the ExecutionContext isn't created, I don't know exactly what we should do..

The inclusion of the ExecutionContext as a parameter to the trace method would be the easy way to keep the current tracing points but it isn't enough because we can use it in the JettyHandler, ...

public interface Tracer {

    public void trace(ExecutionContext ctx, String msg, Object... args);
}

Any suggestion to solve the problem when the ExecutionContext isn't created? Add a new method that receives the Request or the Headers and extract the UUID and the X-TRACE-ME values?

[eswdd]

Defo add a new method, but you don't want the tracer implementation digging stuff directly out of the request. Just make the new method take the uuid. Not sure if you want to pass in the bool or just not call the method if it's not set to true.. i suspect it will be a bit cleaner just to not set the method. In all likelyhood your impl of the method above will just call into the other one and worry about that bool for people to save code elsewhere.

So something like this:

public interface Tracer {
    public void trace(RequestUUID uuid, String msg, Object... args);
    public void trace(ExecutionContext ctx, String msg, Object... args);
}

public abstract class AbstractTracer implements Tracer {
    public void trace(ExecutionContext ctx, String msg, Object... args) {
        if (ctx.traceLoggingEnabled()) {
            trace(ctx.getRequestUUID(), msg, args);
        }
    }
}

[pmvilaca]

Sounds good. We can just skip the call to the tracer if the value of X-TRACE-ME is false.

[eswdd]

Build passed post-commit.