Redis Tracing Dependency Injection Improvements

[CodingFlow]

When using a dependency injection framework, I found it took quite a bit of effort to figure out the best approach that can be implemented with the current Redis tracing API. I've discovered a good solution and have idea proposals to improve it even further.

Best Current Solution

The best way I've found to register the Redis connection with it's current API is to use a factory class that returns the connection either lazily when accessed or via a method call in the startup class (in my case, it was in the hosted service). As for tracing instrumentation, the concrete instance is registered with DI and can be called to set the connection in the factory class:

using Microsoft.Extensions.Options;
using OpenTelemetry.Instrumentation.StackExchangeRedis;
using Service.Abstractions;
using StackExchange.Redis;

namespace Service.Libraries.Redis;

internal class RedisConnection(IOptions<RedisOptions> redisOptions, StackExchangeRedisInstrumentation instrumentation) : IStartupService, IRedisConnection
{
    private readonly RedisOptions redisSettings = redisOptions.Value;
    public IDatabase Database { get; private set; }
    public IServer Server { get; private set; }

    public async Task Startup()
    {
        var muxer = await ConnectionMultiplexer.ConnectAsync($"{redisSettings.Host}:{redisSettings.Port}");

        Database = muxer.GetDatabase();
        Server = muxer.GetServer(muxer.GetEndPoints().First());

        instrumentation.AddConnection(muxer);
    }
}

Tracing DI Registration:

services.AddOpenTelemetry()
    .WithTracing(tracing => tracing.AddRedisInstrumentation());

Solution Proposal 1: Short Term, Quick Win

• Add interface to StackExchangeRedisInstrumentation and use it in DI instead of the concrete type.
• Document this recommended solution when using DI.

This will make onboarding of new users of the Redis tracing library a lot easier and enable easier unit testing by being able to mock the interface for the injected instrumentation. This should also resolve the root issue of #2656

Solution Proposal 2: Long Term, But Even Better

I'm not sure exactly how they do it, but Npgsql.OpenTelemetry works magically without having the user of the library get a reference to a connection object:

public static void RegisterTelemetry(IOpenTelemetryBuilder telemetry)
{
    telemetry.WithTracing(tracing => tracing.AddNpgsql());
}

And that's all that needs to be done. 😕

[martincostello]

Taking a look at the existing code it seems like this is possible today with the current overloads via named instances and keyed services, so I think this is maybe more of a documentation issue and we should have an example for such a use case.

Pseudo-code to enable this with what we already have would be something like the following, assuming that the application calls Startup() somewhere before it's needed:

+services.AddSingleton<IConnectionMultiplexer>(serviceProvider =>
+{
+    var connection = serviceProvider.GetRequiredService<RedisConnection>();
+    return factory.Connection;
+});

services.AddOpenTelemetry()
        .WithTracing(tracing => tracing.AddRedisInstrumentation());

- internal class RedisConnection(IOptions<RedisOptions> redisOptions, StackExchangeRedisInstrumentation instrumentation) : IStartupService, IRedisConnection
+ internal class RedisConnection(IOptions<RedisOptions> redisOptions) : IStartupService, IRedisConnection
{
    private readonly RedisOptions redisSettings = redisOptions.Value;
    public IDatabase Database { get; private set; }
    public IServer Server { get; private set; }
+   public IConnectionMultiplexer Connection { get; private set; }

    public async Task Startup()
    {
        Connection = await ConnectionMultiplexer.ConnectAsync($"{redisSettings.Host}:{redisSettings.Port}");

        Database = Connection.GetDatabase();
        Server = Connection.GetServer(Connection.GetEndPoints().First());

-       instrumentation.AddConnection(muxer);
    }
}

If we did want to add a new API, providing a registration overload that takes a Func<IServiceProvider, IConnectionMultiplexer> might be a good way to go about it and make the resolution explicitly lazy, rather than the current approach of explicitly accessing the service provider:

opentelemetry-dotnet-contrib/src/OpenTelemetry.Instrumentation.StackExchangeRedis/TracerProviderBuilderExtensions.cs

Lines 155 to 169 in 593fb31

	.AddInstrumentation(sp =>
	{
	var instrumentation = sp.GetRequiredService<StackExchangeRedisInstrumentation>();
	connection ??= serviceKey == null
	? sp.GetService<IConnectionMultiplexer>()
	: sp.GetKeyedService<IConnectionMultiplexer>(serviceKey);
	if (connection != null)
	{
	instrumentation.AddConnection(name, connection);
	}
	return instrumentation;
	});

.AddInstrumentation(serviceProvider =>
{
    var instrumentation = sp.GetRequiredService<StackExchangeRedisInstrumentation>();
    var connection = factory(serviceProvider);

    if (connection is not null)
    {
        instrumentation.AddConnection(name, connection);
    }

    return instrumentation;
});

[CodingFlow]

I was pretty sure I tried registering the factory to return the connection, and I just reconfirmed. It's null during DI registration as expected, which is when the telemetry library tries to get it automatically. So, unfortunately, your suggestion with the current API doesn't work. The only option with the current API for lazy initialization is by using instrumentation.AddConnection, which requires a reference to the instrumentation instance.

FYI, calling the lazy init during DI registration does not work because DI is synchronous only, and the lazy init method Startup is async.