Redis instrumentation: allow IConnexionMultiplexer to be added *after* instrumentation creation

[tbolon]

Feature Request

Related: #624

We have a problem using the Redis instrumentation library, because in our app, IConnexionMultiplexer is not available on startup, and is instead created on first demand later in the app lifecycle.

We still want to use this library, because it handles perfectly how to profile the redis commands.

Observations

The current implementation of StackExchangeRedisCallsInstrumentation requires that the IConnexionMultiplexer is passed in the constructor despite only being used to call connection.RegisterProfiler(this.GetProfilerSessionsFactory());¸

An alternative ctor could be added without this connection and a public method RegisterConnection(IConnectionMultiplexer).

But then we need a way to call this Instrumentation instance from our code, to register the connection when created.

Because the instance is internal, everything is supposed to happen on the registration extension method.

Summary

We want to register a IConnexionMultiplexer later on StackExchangeRedisCallsInstrumentation.

• There are cases where the IConnexionMultiplexer is not available at startup
• There is no way to reuse the code of the Redis Instrumentation library because everything is internal

Proposed solution

I see two ways of allowing that, both of them requires that StackExchangeRedisCallsInstrumentation add public a void RegisterConnection(IConnectionMultiplexer connection) to register a connection later.

• Make StackExchangeRedisCallsInstrumentation public, so we can create an instance before, store it somewhere to be able to call the RegisterConnection method, then pass directly this instance to AddInstrumentation().

• Keeps StackExchangeRedisCallsInstrumentation internal, but provide a callback on AddRedisInstrumentation to allow capturing the instrumentation instance (hidden in an interface or a method signature) where we can register a IConnectionMultiplexer later.

Here is a naive example for solution 2:

public interface IRedisCallsInstrumentation
{
    void RegisterConnection(IConnectionMultiplexer connection);
}

/// <summary>
/// Redis calls instrumentation.
/// </summary>
internal class StackExchangeRedisCallsInstrumentation : IRedisCallsInstrumentation, IDisposable
{
    // ...
    public void RegisterConnection(IConnectionMultiplexer connection)
    {
        connection.RegisterProfiler(this.GetProfilerSessionsFactory());
    }
}


/// <summary>
/// Extension methods to simplify registering of dependency instrumentation.
/// </summary>
public static class TracerProviderBuilderExtensions
{
    /// <summary>
    /// Enables the outgoing requests automatic data collection for Redis.
    /// </summary>
    /// <param name="builder"><see cref="TracerProviderBuilder"/> being configured.</param>
    /// <param name="registration">Callback to receive the instrumentation for redis, allowing to register a connection after the instrumentation is started.</param>
    /// <param name="configureOptions">Redis configuration options.</param>
    /// <returns>The instance of <see cref="TracerProviderBuilder"/> to chain the calls.</returns>
    public static TracerProviderBuilder AddRedisInstrumentation(
        this TracerProviderBuilder builder,
        Action<IRedisCallsInstrumentation> registration,
        Action<StackExchangeRedisCallsInstrumentationOptions> configureOptions = null)
    {
        if (builder == null)
        {
            throw new ArgumentNullException(nameof(builder));
        }

        StackExchangeRedisCallsInstrumentationOptions options = new StackExchangeRedisCallsInstrumentationOptions();
        configureOptions?.Invoke(options);

        var instrumentation = new StackExchangeRedisCallsInstrumentation(options);
        
        registration(instrumentation);

        return builder
            .AddInstrumentation(instrumentation)
            .AddSource(StackExchangeRedisCallsInstrumentation.ActivitySourceName);
    }
}

public static class Program
{
    private static IRedisCallsInstrumentation redisCallsIntrumentation;

    public static void Main()
    {
        using var telemetry = Sdk.CreateTracerProviderBuilder()
            .AddRedisInstrumentation(r => { redisCallsIntrumentation = r })
            .Build();

        // ...
        // now you can use redisCallsInstrumentation
    }
}

Other considerations

I understand and agree that it is easier when a library exposes as few public endpoint as possible. But I see extension methods as a shortcut, not the only way to use a library.

I think (but can understand that you don't agree) that you should provide a way to register an instrumentation instance by yourself, by providing the instrumentation instance and adding the source yourself: StackExchangeRedisCallsInstrumentation should be public and the ActivitySourceName constant should be also public.

[philipbawn]

I think I need the same thing open-telemetry/opentelemetry-dotnet-contrib#1785

[adamhathcock]

Another use case: the SignalR redis backplace exposes a factory method to get the redis connection at runtime.

I'd like to be able to create a multiplexer on demand and add instrumentation.

[Norbe3t]

If this is still relevant, you can take a look at my implementation using reflection.

[lucasoares]

Really need this. I have many applications with many different connections created on-demand after the startup and I can't instrument Redis with the current state of this library :(

[Kielek]

@lucasoares, for now, there is no direct support for this in this library.
If you are fine with Automatic Instrumentation you can try this: https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/blob/v0.6.0/docs/config.md#traces-instrumentations

[lucasoares]

@lucasoares, for now, there is no direct support for this in this library. If you are fine with Automatic Instrumentation you can try this: https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/blob/v0.6.0/docs/config.md#traces-instrumentations

Unfortunately I will not be able to setup automatic instrumentation for my projects, really need the manual instrumentation. I will instrument everything manually in a connection wrapper for now :(

[danatcofo]

If this is still relevant, you can take a look at my implementation using reflection.

@Norbe3t did you by chance happen to make and publish a nuget package with this? Its exactly what we need with a multi tenant distributed caching solution (i.e. multiple lazy loaded connections). If not would you mind if I stole this solution and generated a public nuget package?

[Kielek]

Please try to use version 1.0.0-rc9.9 or newer https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/main/src/OpenTelemetry.Instrumentation.StackExchangeRedis/CHANGELOG.md#100-rc99

It addresses this issue.

[Norbe3t]

@danatcofo I didn't create any nuget package, as I think that this is not quite the right way to solve this problem, it's better for the library to do it) fortunately, the developers have already made some changes, but you can do whatever you want with my example.

[pellared]

Closing per #1900 (comment)

Please open a new issue if there are any further problems.