Allow for more customisation of the tracing span

[JaidenAshmore]

Feature Request

Currently you are providing an ability to include the arguments of the Redis command in the tag of the span, see CommandHandler. This is controlled by a single boolean flag to either include it or exclude those arguments. An example of me doing this is by applying the following Bean in a Spring Application:

@Bean(destroyMethod = "shutdown")
public DefaultClientResources lettuceClientResources(Tracing tracing) {
    return DefaultClientResources.builder()
           .tracing(BraveTracing.builder()
                    .tracing(tracing)
                    .includeCommandArgsInSpanTags(true)
                    .build())
           .build();
}

The usage of this boolean value in the underlying CommandHandler looks like:

if (includeCommandArgsInSpanTags && command.getArgs() != null) {
    span.tag("redis.args", command.getArgs().toCommandString());
}

As this includes all the args, it will include both the key and value for set commands. I would prefer to not have the values of my caches included in my tracing spans but are happy to have the key in there as this will help with debugging.

It would be great if we had more control as a consumer to customise what tags we can include in our Redis Tracing Spans.

Is your feature request related to a problem? Please describe

No

Describe the solution you'd like

One solution that I have been thinking of would be to allow consumers to provide their own functions that can decorate the Spans. It could just even be a BiConsumer<Span, RedisCommand<?, ?>> instead of a boolean value. The consumer API would look something like:

@Bean(destroyMethod = "shutdown")
public DefaultClientResources lettuceClientResources(Tracing tracing) {
    return DefaultClientResources.builder()
           .tracing(BraveTracing.builder()
                    .tracing(tracing)
                    .addSpanDecorator((span, command) -> {
                          // I could add the existing logic in here if I want
                          if (command.getArgs() != null) {
                              span.tag("redis.args", command.getArgs().toCommandString());
                          }
                     })
                    .addSpanDecorator((span, command) -> {
                         // or I can supply my own custom logic, multiple decorators should be allowed
                         span.tag("redis.something", "value");
                     })
                    .build())
           .build();
}

Depending on what we want to do with the existing boolean flag we could allow the existing functionality and new functionality to work together:

@Bean(destroyMethod = "shutdown")
public DefaultClientResources lettuceClientResources(Tracing tracing) {
    return DefaultClientResources.builder()
           .tracing(BraveTracing.builder()
                    .tracing(tracing)
                    .includeCommandArgsInSpanTags(true)
                    .addSpanDecorator((span, command) -> {
                         span.tag("redis.something", "value");
                     })
                    .build())
           .build();
}

Under the hood both the includeCommandArgsInSpanTags and addSpanDecorator could use the same underlying list of "SpanDecorators". For example:

public class BraveTracing implements Tracing {

    private final BraveTracer tracer;
    private final BraveTracingOptions tracingOptions;
    private final List<BiConsumer<Span, RedisCommand<?, ?>> spanDecorators;

...

        public Builder includeCommandArgsInSpanTags(boolean includeCommandArgsInSpanTags) {

          // obviously we would need to be smart with how this would work if I make
          // multiple calls to this or the excludeCommandArgsFromSpanTags method.
           if (includeCommandArgsInSpanTags) {
                     spanDecorators.add((span, command) -> {
                            if (command.getArgs() != null) {
                              span.tag("redis.args", command.getArgs().toCommandString());
                          }
                     });
           } 
            return this;
        }

Note instead of a BiConsumer we could create our own Functional Interface, e.g. name it SpanDecorator or SpanCommandDecorator, etc. Up for suggestions.

Describe alternatives you've considered

There is currently the spanCustomizer but that does not seem to have any extra information coming from the actual command that is being performed. This could be something that could be extended with more information but I am not sure if that is possible.

Teachability, Documentation, Adoption, Migration Strategy

If the main solution is considered the way to go, this would not be a breaking change and therefore no migration documentation or other information would be needed. This should just be documented like any other feature with this libraries tracing.

[mp911de]

Thanks for bringing up this issue. It touches both, the SPI and the Brave Tracer implementations. Looking at the code, we have Span.start() and Span.finish() and a bit of code that associates the spans with some metadata in CommandHandler.

I'm considering to overload start and stop methods so they accept a RedisCommand object. Tracer.Span is an abstract class so we can make both methods default implementations calling start() and stop() without the command so downstream implementations are not broken. This way, we can move invocation of .name, .tag and .remoteEndpoint to the inside of BraveSpan which is the first part to associate spans with more command details and we could introduce this change in a service release without waiting for Lettuce 6 GA.

The second step would be adding an overload to spanCustomizer that accepts a BiConsumer<Span, RedisCommand<Object, Object, Object>> so we can reuse the existing customizer approach but with more context.

Let me know what you think about it.

[mp911de]

That's in place now.