opentelemetry-dotnet-contrib
opentelemetry-dotnet-contrib copied to clipboard
Support Redis instrumentation when IConnectionMultiplexer is added with keyed service
Issue with OpenTelemetry.Instrumentation.StackExchangeRedis
Is this a feature request or a bug?
- [x] Feature Request
- [ ] Bug
Currently there's two way to have Redis instrumentation. With passing the IConnectionMultiplexer instance to the method or By registering IConnectionMultiplexer to the IoC Container.
In .Net 8 there's support to keyed services in IoC Container. It would be nice to have support for instrumentation for IConnectionMultiplexer that is registered as a keyed service.
I suggest adding this API
public class TracerProviderBuilderExtensions
{
public static TracerProviderBuilder AddRedisInstrumentation(
this TracerProviderBuilder builder,
string? name,
IConnectionMultiplexer? connection,
Action<StackExchangeRedisInstrumentationOptions>? configure,
object? serviceKey)
}
And this line need to change so that if serviceKey is available get the IConnectionMultiplexer with servicekey.
https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/c3a4f335f5a1f4fc784d44cc99b5bf44e872a66c/src/OpenTelemetry.Instrumentation.StackExchangeRedis/TracerProviderBuilderExtensions.cs#L135
Also to support this the OpenTelemetry.Instrumentation.StackExchangeRedis project need to add .NET 8 to its target framekwork.
I'm willing to send a PR for this feature if approved.
The package does not have the component owner. I think that it will be fine to extend the API. The .NET8 can take some time as we need to merge at least 2 PRs first: #1446 and #1437 to make our pipeline ready for this.
@CodeBlanch, what is your perspective on this? I think you have the biggest experience on SIG with defining new API contracts.
I'm fine with the API addition. Feel free to open a PR @Kahbazi and I will review it.
Also to support this the OpenTelemetry.Instrumentation.StackExchangeRedis project need to add .NET 8 to its target framekwork.
Do we need net8 target or do we need Microsoft.Extensions.DependencyInjection.Abstractions v8? If it is the later that should come with the OpenTelemetry.Api.ProviderBuilderExtensions package reference once it is updated for 1.7 (>1.7.0-alpha.1 we haven't released new versions yet with the new package dependencies all bumped to 8).
Do we need net8 target or do we need Microsoft.Extensions.DependencyInjection.Abstractions v8?
The latter.
@Kahbazi are you still planning on helping with this? your PR went stale.
@martinjt Sorry for the delay. Yes, I will re-open the PR.