imperugo / StackExchange.Redis.Extensions

MIT License
612 stars 178 forks source link

OpenTelemetry not work #504

Closed zhoudi94 closed 11 months ago

zhoudi94 commented 2 years ago

Is your feature request related to a problem? Please describe. OpenTelemetry not work, because it has to use the same instance IConnectionMultiplexer

https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/8d3510ac6b0a90e5fe8ac913dec3565d3fba85e2/src/OpenTelemetry.Instrumentation.StackExchangeRedis/README.md

I try to set StateAwareConnectionFactory,but not working. image

I read the source and it creates a new one every time instead of using the same IConnectionMultiplexer image

Describe the solution you'd like Is it possible to use the same IConnectionMultiplexer

Additional context

imperugo commented 1 year ago

Hi @zhoudi94

I already use OpenTelemetry with StackExchange.Redis in this way:

services
.AddOpenTelemetry()
.WithTracing(b =>
{
    // receive traces from our own custom sources
    b.AddSource(Constants.OpenTelementry.SERVICE_NAME);

    // decorate our service name so we can find it when we look inside Jaeger
    b.SetResourceBuilder(resource);

    // receive traces from built-in sources
    b.AddHttpClientInstrumentation();
    b.AddGrpcClientInstrumentation();
    b.AddAspNetCoreInstrumentation((options) => options.Filter = httpContext =>
    {
        if (httpContext.Request.Method == "OPTIONS")
            return false;

        if (httpContext.Request.Path.StartsWithSegments("/docs", StringComparison.OrdinalIgnoreCase))
            return false;

        if (httpContext.Request.Path.StartsWithSegments("/metrics", StringComparison.OrdinalIgnoreCase))
            return false;

        if (httpContext.Request.Path.StartsWithSegments("/swagger", StringComparison.OrdinalIgnoreCase))
            return false;

        if (httpContext.Request.Path.StartsWithSegments("/favicon", StringComparison.OrdinalIgnoreCase))
            return false;

        return true;
    });
    b.AddMongoDBInstrumentation();

    b.ConfigureBuilder((sp, builder) =>
    {
        foreach (var connection in sp.GetRequiredService<IRedisClient>().ConnectionPoolManager.GetConnections())
            builder.AddRedisInstrumentation(connection, opt => opt.SetVerboseDatabaseStatements = true);
    });

    b.AddJaegerExporter(opt =>
    {
        // This is needed for VPN
        // opt.AgentHost = "10.1.10.46";
        opt.AgentHost = Environment.GetEnvironmentVariable("JAEGER_HOST") ?? "jaeger-agent";
        opt.Protocol = JaegerExportProtocol.UdpCompactThrift;
    });
})
.StartWithHost();

Moreover, if you want to retrieve the available connections (the libraary use a connection pool so there could be more than one) you can use the GetConnections() method from the ConnectionPoolManager property of the IRedisClient (same thing of the example above).

Bayonle commented 1 year ago

The suggested way to instrument redis is throwing an exception. The docs says to do it like below

services
    .AddOpenTelemetry()
    .WithTracing(b =>
    {
        // Do your stuff here

        b.AddInstrumentation((sp, traceProvider) =>
        {
            // Iterate all connection and add the instrumentation
            foreach (var connection in sp.GetRequiredService<IRedisClient>().ConnectionPoolManager.GetConnections())
                b.AddRedisInstrumentation(connection, opt => opt.SetVerboseDatabaseStatements = true);

            return traceProvider;
        });
    });

However, this doesn't work in dotnet 6.0 using OpenTelemetry.Instrumentation.StackExchangeRedis" Version="1.0.0-rc9.10" The exception thrown is Services cannot be configured during TracerProvider construction at the b.AddRedisInstrumentation() line.

Any idea how to solve this?

imperugo commented 11 months ago

It seems the linbrary changed way to instrument redis. I've tried this solution and works

builder
            .AddRedisInstrumentation()
            .ConfigureRedisInstrumentation((sp, instrumentation) =>
            {
                var redisClient = sp.GetRequiredService<IRedisClient>();

                foreach (var connection in redisClient.ConnectionPoolManager.GetConnections())
                    instrumentation.AddConnection(connection);
            });

Try it