Fixes: Identifying Users with OpenTelemetry doesn't work (#2618)

CHANGELOG.md

@@ -7,6 +7,10 @@
 - The SDK now provides and overload of `ContinueTrace` that accepts headers as `string` ([#2601](https://github.com/getsentry/sentry-dotnet/pull/2601))
 - Sentry tracing middleware now gets configured automatically ([#2602](https://github.com/getsentry/sentry-dotnet/pull/2602))
 
+### Fixes
+
+- Resolved issue identifying users with OpenTelemetry ([#2618](https://github.com/getsentry/sentry-dotnet/pull/2618))
+
 ### Dependencies
 
 - Bump CLI from v2.20.6 to v2.20.7 ([#2604](https://github.com/getsentry/sentry-dotnet/pull/2604))

Sentry.sln.DotSettings

@@ -1,2 +1,4 @@
 <wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=Enricher/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=enrichers/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=instrumenter/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>

samples/Sentry.Samples.OpenTelemetry.AspNetCore/FakeAuthHandler.cs

@@ -0,0 +1,38 @@
+using System.Security.Claims;
+using System.Text.Encodings.Web;
+using Microsoft.AspNetCore.Authentication;
+using Microsoft.Extensions.Options;
+
+namespace Sentry.Samples.OpenTelemetry.AspNetCore;
+
+public class FakeAuthHandler : AuthenticationHandler<AuthenticationSchemeOptions>
+{
+    public const string UserId = "UserId";
+
+    public const string AuthenticationScheme = "Test";
+
+    public FakeAuthHandler(
+        IOptionsMonitor<AuthenticationSchemeOptions> options,
+        ILoggerFactory logger,
+        UrlEncoder encoder,
+        ISystemClock clock) : base(options, logger, encoder, clock)
+    {
+    }
+
+    protected override Task<AuthenticateResult> HandleAuthenticateAsync()
+    {
+        var claims = new List<Claim>
+        {
+            new(ClaimTypes.Name, "Fake user"),
+            new(ClaimTypes.NameIdentifier, "fake-user")
+        };
+
+        var identity = new ClaimsIdentity(claims, AuthenticationScheme);
+        var principal = new ClaimsPrincipal(identity);
+        var ticket = new AuthenticationTicket(principal, AuthenticationScheme);
+
+        var result = AuthenticateResult.Success(ticket);
+
+        return Task.FromResult(result);
+    }
+}

samples/Sentry.Samples.OpenTelemetry.AspNetCore/Program.cs

@@ -1,4 +1,5 @@
 using System.Diagnostics;
+using Microsoft.AspNetCore.Authentication;
 using OpenTelemetry.Resources;
 using OpenTelemetry.Trace;
 using Sentry.OpenTelemetry;
@@ -22,11 +23,18 @@
 {
     //options.Dsn = "...Your DSN...";
     options.Debug = builder.Environment.IsDevelopment();
+    options.SendDefaultPii = true;
     options.TracesSampleRate = 1.0;
     options.UseOpenTelemetry(); // <-- Configure Sentry to use OpenTelemetry trace information
 });
 
+builder.Services
+    .AddAuthorization()
+    .AddAuthentication(FakeAuthHandler.AuthenticationScheme)
+    .AddScheme<AuthenticationSchemeOptions, FakeAuthHandler>(FakeAuthHandler.AuthenticationScheme, _ => { });
+
 var app = builder.Build();
+app.UseAuthorization();
 
 var httpClient = new HttpClient();
 app.MapGet("/hello", async context =>
@@ -48,6 +56,13 @@
 
 app.MapGet("/echo/{name}", (string name) => $"Hi {name}!");
 
+app.MapGet("/private", async context  =>
+{
+    var user = context.User;
+    var result = $"Hello {user.Identity?.Name}";
+    await context.Response.WriteAsync(result);
+}).RequireAuthorization();
+
 app.MapGet("/throw", _ => throw new Exception("test"));
 
 app.Run();

samples/Sentry.Samples.OpenTelemetry.AspNetCore/Properties/launchSettings.json

@@ -1,11 +1,11 @@
 {
   "profiles": {
-    "http": {
+    "https": {
       "commandName": "Project",
       "dotnetRunMessages": true,
       "launchBrowser": true,
       "launchUrl": "hello",
-      "applicationUrl": "http://localhost:5092",
+      "applicationUrl": "https://localhost:5092",
       "environmentVariables": {
         "ASPNETCORE_ENVIRONMENT": "Development"
       }

src/Sentry.AspNetCore/DefaultUserFactory.cs

@@ -2,8 +2,23 @@
 
 namespace Sentry.AspNetCore;
 
-internal class DefaultUserFactory : IUserFactory
+#pragma warning disable CS0618
+internal class DefaultUserFactory : IUserFactory, ISentryUserFactory
+#pragma warning restore CS0618
 {
+    private readonly IHttpContextAccessor? _httpContextAccessor;
+
+    public DefaultUserFactory()
+    {
+    }
+
+    public DefaultUserFactory(IHttpContextAccessor httpContextAccessor)
+    {
+        _httpContextAccessor = httpContextAccessor;
+    }
+
+    public User? Create() => _httpContextAccessor?.HttpContext is {} httpContext ? Create(httpContext) : null;
+
     public User? Create(HttpContext context)
     {
         var principal = context.User;

src/Sentry.AspNetCore/Extensions/DependencyInjection/ServiceCollectionExtensions.cs

@@ -1,4 +1,5 @@
 using Microsoft.Extensions.DependencyInjection.Extensions;
+using Sentry;
 using Sentry.AspNetCore;
 using Sentry.Extensibility;
 using Sentry.Extensions.Logging.Extensions.DependencyInjection;
@@ -20,7 +21,12 @@ public static ISentryBuilder AddSentry(this IServiceCollection services)
     {
         services.AddSingleton<ISentryEventProcessor, AspNetCoreEventProcessor>();
         services.AddSingleton<ISentryEventExceptionProcessor, AspNetCoreExceptionProcessor>();
+
+        services.AddHttpContextAccessor();
+#pragma warning disable CS0618
         services.TryAddSingleton<IUserFactory, DefaultUserFactory>();
+#pragma warning restore CS0618
+        services.TryAddSingleton<ISentryUserFactory, DefaultUserFactory>();
 
         services
             .AddSingleton<IRequestPayloadExtractor, FormRequestPayloadExtractor>()

src/Sentry.AspNetCore/IUserFactory.cs

@@ -5,6 +5,7 @@ namespace Sentry.AspNetCore;
 /// <summary>
 /// Sentry User Factory
 /// </summary>
+[Obsolete("This interface is tightly coupled to AspNetCore and will be removed in version 4.0.0. Please consider using ISentryUserFactory with IHttpContextAccessor instead.")]
 public interface IUserFactory
 {
     /// <summary>

src/Sentry.AspNetCore/ScopeExtensions.cs

@@ -38,8 +38,10 @@ public static void Populate(this Scope scope, HttpContext context, SentryAspNetC
 
         if (options.SendDefaultPii && !scope.HasUser())
         {
+#pragma warning disable CS0618
             var userFactory = context.RequestServices.GetService<IUserFactory>();
             var user = userFactory?.Create(context);
+#pragma warning restore CS0618
 
             if (user != null)
             {

src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs

@@ -0,0 +1,22 @@
+namespace Sentry.OpenTelemetry;
+
+internal class AspNetCoreEnricher : IOpenTelemetryEnricher
+{
+    private readonly ISentryUserFactory _userFactory;
+
+    internal AspNetCoreEnricher(ISentryUserFactory userFactory) => _userFactory = userFactory;
+
+    public void Enrich(ISpan span, Activity activity, IHub hub, SentryOptions? options)
+    {
+        if (options?.SendDefaultPii is true)
+        {
+            hub.ConfigureScope(scope =>
+            {
+                if (!scope.HasUser() && _userFactory.Create() is {} user)
+                {
+                    scope.User = user;
+                }
+            });
+        }
+    }
+}

src/Sentry.OpenTelemetry/IOpenTelemetryEnricher.cs

@@ -0,0 +1,6 @@
+namespace Sentry.OpenTelemetry;
+
+internal interface IOpenTelemetryEnricher
+{
+    void Enrich(ISpan span, Activity activity, IHub hub, SentryOptions? options);
+}

src/Sentry.OpenTelemetry/SentrySpanProcessor.cs

@@ -1,6 +1,7 @@
 using OpenTelemetry;
 using OpenTelemetry.Trace;
 using Sentry.Extensibility;
+using Sentry.Internal;
 using Sentry.Internal.Extensions;
 
 namespace Sentry.OpenTelemetry;
@@ -11,6 +12,7 @@ namespace Sentry.OpenTelemetry;
 public class SentrySpanProcessor : BaseProcessor<Activity>
 {
     private readonly IHub _hub;
+    private readonly IEnumerable<IOpenTelemetryEnricher> _enrichers;
 
     // ReSharper disable once MemberCanBePrivate.Global - Used by tests
     internal readonly ConcurrentDictionary<ActivitySpanId, ISpan> _map = new();
@@ -27,9 +29,14 @@ public SentrySpanProcessor() : this(SentrySdk.CurrentHub)
     /// <summary>
     /// Constructs a <see cref="SentrySpanProcessor"/>.
     /// </summary>
-    public SentrySpanProcessor(IHub hub)
+    public SentrySpanProcessor(IHub hub) : this(hub, null)
+    {
+    }
+
+    internal SentrySpanProcessor(IHub hub, IEnumerable<IOpenTelemetryEnricher>? enrichers)
     {
         _hub = hub;
+        _enrichers = enrichers ?? Enumerable.Empty<IOpenTelemetryEnricher>();
         _options = hub.GetSentryOptions();
 
         if (_options is not { })
@@ -165,6 +172,10 @@ public override void OnEnd(Activity data)
         GenerateSentryErrorsFromOtelSpan(data, attributes);
 
         var status = GetSpanStatus(data.Status, attributes);
+        foreach (var enricher in _enrichers)
+        {
+            enricher.Enrich(span, data, _hub, _options);
+        }
         span.Finish(status);
 
         _map.TryRemove(data.SpanId, out _);

src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs

@@ -1,3 +1,4 @@
+using Microsoft.Extensions.DependencyInjection;
 using OpenTelemetry;
 using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Trace;
@@ -29,6 +30,18 @@ public static TracerProviderBuilder AddSentry(this TracerProviderBuilder tracerP
     {
         defaultTextMapPropagator ??= new SentryPropagator();
         Sdk.SetDefaultTextMapPropagator(defaultTextMapPropagator);
-        return tracerProviderBuilder.AddProcessor<SentrySpanProcessor>();
+        return tracerProviderBuilder.AddProcessor(services =>
+        {
+            List<IOpenTelemetryEnricher> enrichers = new();
+
+            // AspNetCoreEnricher
+            var userFactory = services.GetService<ISentryUserFactory>();
+            if (userFactory is not null)
+            {
+                enrichers.Add(new AspNetCoreEnricher(userFactory));
+            }
+
+            return new SentrySpanProcessor(SentrySdk.CurrentHub, enrichers);
+        });
     }
 }

src/Sentry/ISentryUserFactory.cs

@@ -0,0 +1,6 @@
+namespace Sentry;
+
+internal interface ISentryUserFactory
+{
+    public User? Create();
+}

src/Sentry/Sentry.csproj

@@ -152,7 +152,6 @@
     <InternalsVisibleTo Include="Sentry.NLog" PublicKey="$(SentryPublicKey)" />
     <InternalsVisibleTo Include="Sentry.NLog.Tests" PublicKey="$(SentryPublicKey)" />
     <InternalsVisibleTo Include="Sentry.OpenTelemetry" PublicKey="$(SentryPublicKey)" />
-    <InternalsVisibleTo Include="Sentry.OpenTelemetry.AspNetCore" PublicKey="$(SentryPublicKey)" />
     <InternalsVisibleTo Include="Sentry.OpenTelemetry.Tests" PublicKey="$(SentryPublicKey)" />
     <InternalsVisibleTo Include="Sentry.Profiling" PublicKey="$(SentryPublicKey)" />
     <InternalsVisibleTo Include="Sentry.Profiling.Tests" PublicKey="$(SentryPublicKey)" />

test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Core3_1.verified.txt

@@ -32,6 +32,9 @@ namespace Sentry.AspNetCore
     {
         Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
     }
+    [System.Obsolete("This interface is tightly coupled to AspNetCore and will be removed in version 4." +
+        "0.0. Please consider using ISentryUserFactory with IHttpContextAccessor instead." +
+        "")]
     public interface IUserFactory
     {
         Sentry.User? Create(Microsoft.AspNetCore.Http.HttpContext context);

test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt

@@ -32,6 +32,9 @@ namespace Sentry.AspNetCore
     {
         Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
     }
+    [System.Obsolete("This interface is tightly coupled to AspNetCore and will be removed in version 4." +
+        "0.0. Please consider using ISentryUserFactory with IHttpContextAccessor instead." +
+        "")]
     public interface IUserFactory
     {
         Sentry.User? Create(Microsoft.AspNetCore.Http.HttpContext context);

test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt

@@ -32,6 +32,9 @@ namespace Sentry.AspNetCore
     {
         Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
     }
+    [System.Obsolete("This interface is tightly coupled to AspNetCore and will be removed in version 4." +
+        "0.0. Please consider using ISentryUserFactory with IHttpContextAccessor instead." +
+        "")]
     public interface IUserFactory
     {
         Sentry.User? Create(Microsoft.AspNetCore.Http.HttpContext context);

test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Net4_8.verified.txt

@@ -32,6 +32,9 @@ namespace Sentry.AspNetCore
     {
         Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
     }
+    [System.Obsolete("This interface is tightly coupled to AspNetCore and will be removed in version 4." +
+        "0.0. Please consider using ISentryUserFactory with IHttpContextAccessor instead." +
+        "")]
     public interface IUserFactory
     {
         Sentry.User? Create(Microsoft.AspNetCore.Http.HttpContext context);

test/Sentry.AspNetCore.Tests/DefaultUserFactoryTests.cs

@@ -85,6 +85,16 @@ public void Create_NoClaims_IpAddress()
         Assert.Equal(IPAddress.IPv6Loopback.ToString(), actual.IpAddress);
     }
 
+    [Fact]
+    public void Create_ContextAccessorNoClaims_IpAddress()
+    {
+        _ = HttpContext.User.Claims.Returns(Enumerable.Empty<Claim>());
+        var contextAccessor = Substitute.For<IHttpContextAccessor>();
+        contextAccessor.HttpContext.Returns(HttpContext);
+        var actual = new DefaultUserFactory(contextAccessor).Create();
+        Assert.Equal(IPAddress.IPv6Loopback.ToString(), actual?.IpAddress);
+    }
+
     [Fact]
     public void Create_ClaimNameAndIdentityDontMatch_UsernameFromIdentity()
     {

test/Sentry.AspNetCore.Tests/ServiceCollectionExtensionsTests.cs

@@ -68,11 +68,13 @@ public void AddSentry_DefaultRequestPayloadExtractor_LastRegistration()
         Assert.Same(typeof(DefaultRequestPayloadExtractor), last.ImplementationType);
     }
 
+#pragma warning disable CS0618
     [Fact]
     public void AddSentry_DefaultUserFactory_Registered()
     {
         _ = _sut.AddSentry();
         _sut.Received().Add(Arg.Is<ServiceDescriptor>(d => d.ServiceType == typeof(IUserFactory)
                                                            && d.ImplementationType == typeof(DefaultUserFactory)));
     }
+#pragma warning restore CS0618
 }