From ad6968c04ecdc2775692dddf48852da7090aef2b Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 14 Sep 2023 18:12:23 +1200 Subject: [PATCH 01/10] Initial implementation for feedback --- Sentry.sln.DotSettings | 2 + .../FakeAuthHandler.cs | 38 +++++++++++++++++++ .../Program.cs | 15 ++++++++ .../Properties/launchSettings.json | 4 +- src/Sentry.AspNetCore/DefaultUserFactory.cs | 15 +++++++- .../ServiceCollectionExtensions.cs | 4 ++ src/Sentry.AspNetCore/IUserFactory.cs | 6 +++ .../AspNetCoreEnricher.cs | 26 +++++++++++++ .../IOpenTelemetryEnricher.cs | 6 +++ .../SentrySpanProcessor.cs | 13 ++++++- .../TracerProviderBuilderExtensions.cs | 15 +++++++- src/Sentry/ISentryUserFactory.cs | 6 +++ src/Sentry/Sentry.csproj | 1 - 13 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 samples/Sentry.Samples.OpenTelemetry.AspNetCore/FakeAuthHandler.cs create mode 100644 src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs create mode 100644 src/Sentry.OpenTelemetry/IOpenTelemetryEnricher.cs create mode 100644 src/Sentry/ISentryUserFactory.cs diff --git a/Sentry.sln.DotSettings b/Sentry.sln.DotSettings index a0775def1c..0f2c1d5e85 100644 --- a/Sentry.sln.DotSettings +++ b/Sentry.sln.DotSettings @@ -1,2 +1,4 @@  + True + True True \ No newline at end of file diff --git a/samples/Sentry.Samples.OpenTelemetry.AspNetCore/FakeAuthHandler.cs b/samples/Sentry.Samples.OpenTelemetry.AspNetCore/FakeAuthHandler.cs new file mode 100644 index 0000000000..5404e2dcfb --- /dev/null +++ b/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 +{ + public const string UserId = "UserId"; + + public const string AuthenticationScheme = "Test"; + + public FakeAuthHandler( + IOptionsMonitor options, + ILoggerFactory logger, + UrlEncoder encoder, + ISystemClock clock) : base(options, logger, encoder, clock) + { + } + + protected override Task HandleAuthenticateAsync() + { + var claims = new List + { + 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); + } +} diff --git a/samples/Sentry.Samples.OpenTelemetry.AspNetCore/Program.cs b/samples/Sentry.Samples.OpenTelemetry.AspNetCore/Program.cs index 9982a7e847..83145721a5 100644 --- a/samples/Sentry.Samples.OpenTelemetry.AspNetCore/Program.cs +++ b/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(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(); diff --git a/samples/Sentry.Samples.OpenTelemetry.AspNetCore/Properties/launchSettings.json b/samples/Sentry.Samples.OpenTelemetry.AspNetCore/Properties/launchSettings.json index aed5f1f62b..b71c1887dc 100644 --- a/samples/Sentry.Samples.OpenTelemetry.AspNetCore/Properties/launchSettings.json +++ b/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" } diff --git a/src/Sentry.AspNetCore/DefaultUserFactory.cs b/src/Sentry.AspNetCore/DefaultUserFactory.cs index f7aae644ab..1fef155e9e 100644 --- a/src/Sentry.AspNetCore/DefaultUserFactory.cs +++ b/src/Sentry.AspNetCore/DefaultUserFactory.cs @@ -2,8 +2,21 @@ namespace Sentry.AspNetCore; -internal class DefaultUserFactory : IUserFactory +internal class DefaultUserFactory : IUserFactory, ISentryUserFactory { + 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; diff --git a/src/Sentry.AspNetCore/Extensions/DependencyInjection/ServiceCollectionExtensions.cs b/src/Sentry.AspNetCore/Extensions/DependencyInjection/ServiceCollectionExtensions.cs index 8efed3a934..6f32b884ab 100644 --- a/src/Sentry.AspNetCore/Extensions/DependencyInjection/ServiceCollectionExtensions.cs +++ b/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,10 @@ public static ISentryBuilder AddSentry(this IServiceCollection services) { services.AddSingleton(); services.AddSingleton(); + + services.AddHttpContextAccessor(); services.TryAddSingleton(); + services.TryAddSingleton(); services .AddSingleton() diff --git a/src/Sentry.AspNetCore/IUserFactory.cs b/src/Sentry.AspNetCore/IUserFactory.cs index 0fa4071e89..5501a139b6 100644 --- a/src/Sentry.AspNetCore/IUserFactory.cs +++ b/src/Sentry.AspNetCore/IUserFactory.cs @@ -3,7 +3,13 @@ namespace Sentry.AspNetCore; /// +/// /// Sentry User Factory +/// +/// +/// Note: This interface is tightly coupled to AspNetCore and Will be removed in version 4.0.0. Please consider using +/// with instead. +/// /// public interface IUserFactory { diff --git a/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs b/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs new file mode 100644 index 0000000000..82c4d31cf7 --- /dev/null +++ b/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs @@ -0,0 +1,26 @@ +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) + { + var sendPii = options?.SendDefaultPii ?? false; + if (sendPii) + { + hub.ConfigureScope(scope => + { + if (!scope.HasUser() && _userFactory.Create() is {} user) + { + scope.User = user; + } + }); + } + } +} diff --git a/src/Sentry.OpenTelemetry/IOpenTelemetryEnricher.cs b/src/Sentry.OpenTelemetry/IOpenTelemetryEnricher.cs new file mode 100644 index 0000000000..2fa72949f0 --- /dev/null +++ b/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); +} diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index 1379b30207..81983be742 100644 --- a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs +++ b/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 { private readonly IHub _hub; + private readonly IEnumerable _enrichers; // ReSharper disable once MemberCanBePrivate.Global - Used by tests internal readonly ConcurrentDictionary _map = new(); @@ -27,9 +29,14 @@ public SentrySpanProcessor() : this(SentrySdk.CurrentHub) /// /// Constructs a . /// - public SentrySpanProcessor(IHub hub) + public SentrySpanProcessor(IHub hub) : this(hub, null) + { + } + + internal SentrySpanProcessor(IHub hub, List? enrichers) { _hub = hub; + _enrichers = enrichers ?? Enumerable.Empty(); _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 _); diff --git a/src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs b/src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs index 34b2e4a3cc..0d1c53dc79 100644 --- a/src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs +++ b/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(); + return tracerProviderBuilder.AddProcessor(services => + { + List enrichers = new(); + + // AspNetCoreEnricher + var userFactory = services.GetService(); + if (userFactory != null) + { + enrichers.Add(new AspNetCoreEnricher(userFactory)); + } + + return new SentrySpanProcessor(SentrySdk.CurrentHub, enrichers); + }); } } diff --git a/src/Sentry/ISentryUserFactory.cs b/src/Sentry/ISentryUserFactory.cs new file mode 100644 index 0000000000..abec65e6ff --- /dev/null +++ b/src/Sentry/ISentryUserFactory.cs @@ -0,0 +1,6 @@ +namespace Sentry; + +internal interface ISentryUserFactory +{ + public User? Create(); +} diff --git a/src/Sentry/Sentry.csproj b/src/Sentry/Sentry.csproj index 62907d0fc9..68054813d8 100644 --- a/src/Sentry/Sentry.csproj +++ b/src/Sentry/Sentry.csproj @@ -152,7 +152,6 @@ - From df7658f3e3d22483856a47062933df5a55ff9f4b Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 14 Sep 2023 18:25:27 +1200 Subject: [PATCH 02/10] Update CHANGELOG.md --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cc5ac8e26..8905d1317e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,16 @@ - Sentry tracing middleware now gets configured automatically ([#2602](https://github.com/getsentry/sentry-dotnet/pull/2602)) +### Fixes + +- Identifying Users with OpenTelemetry doesn't work ([#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)) - [changelog](https://github.com/getsentry/sentry-cli/blob/master/CHANGELOG.md#2207) - [diff](https://github.com/getsentry/sentry-cli/compare/2.20.6...2.20.7) + ## 3.39.1 ### Fixes From a291b0495a1f9bf3449bc7e7ca2b75911e4b47b6 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Fri, 15 Sep 2023 09:23:52 +1200 Subject: [PATCH 03/10] Update src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs Co-authored-by: Bruno Garcia --- src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs b/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs index 82c4d31cf7..0859d4b822 100644 --- a/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs +++ b/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs @@ -11,8 +11,7 @@ internal AspNetCoreEnricher(ISentryUserFactory userFactory) public void Enrich(ISpan span, Activity activity, IHub hub, SentryOptions? options) { - var sendPii = options?.SendDefaultPii ?? false; - if (sendPii) + if (options?.SendDefaultPii is true) { hub.ConfigureScope(scope => { From 7c7c89ee9e284187a15237a21d0a3b5ac681f60b Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Fri, 15 Sep 2023 09:24:15 +1200 Subject: [PATCH 04/10] Update src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs Co-authored-by: Bruno Garcia --- src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs b/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs index 0859d4b822..dd0fb833a5 100644 --- a/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs +++ b/src/Sentry.OpenTelemetry/AspNetCoreEnricher.cs @@ -4,10 +4,7 @@ internal class AspNetCoreEnricher : IOpenTelemetryEnricher { private readonly ISentryUserFactory _userFactory; - internal AspNetCoreEnricher(ISentryUserFactory userFactory) - { - _userFactory = userFactory; - } + internal AspNetCoreEnricher(ISentryUserFactory userFactory) => _userFactory = userFactory; public void Enrich(ISpan span, Activity activity, IHub hub, SentryOptions? options) { From fa804ffe0205ca5ef5f87d93381a286bcdefb384 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Fri, 15 Sep 2023 09:24:34 +1200 Subject: [PATCH 05/10] Update src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs Co-authored-by: Bruno Garcia --- src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs b/src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs index 0d1c53dc79..1ca6282a98 100644 --- a/src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs +++ b/src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs @@ -36,7 +36,7 @@ public static TracerProviderBuilder AddSentry(this TracerProviderBuilder tracerP // AspNetCoreEnricher var userFactory = services.GetService(); - if (userFactory != null) + if (userFactory is not null) { enrichers.Add(new AspNetCoreEnricher(userFactory)); } From d4a46a5187ed20ae998b35b5d47a72d84caf03f0 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Fri, 15 Sep 2023 09:24:54 +1200 Subject: [PATCH 06/10] Update src/Sentry.OpenTelemetry/SentrySpanProcessor.cs Co-authored-by: Bruno Garcia --- src/Sentry.OpenTelemetry/SentrySpanProcessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index 81983be742..e8abf08483 100644 --- a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs +++ b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs @@ -33,7 +33,7 @@ public SentrySpanProcessor(IHub hub) : this(hub, null) { } - internal SentrySpanProcessor(IHub hub, List? enrichers) + internal SentrySpanProcessor(IHub hub, IEnumerable? enrichers) { _hub = hub; _enrichers = enrichers ?? Enumerable.Empty(); From 1c4c20b6b74ac0747f096795515f3a7bcd6fd24b Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Mon, 18 Sep 2023 11:55:14 +1200 Subject: [PATCH 07/10] Added obsolete attribute to IUserFactory --- src/Sentry.AspNetCore/DefaultUserFactory.cs | 2 ++ .../DependencyInjection/ServiceCollectionExtensions.cs | 2 ++ src/Sentry.AspNetCore/IUserFactory.cs | 7 +------ src/Sentry.AspNetCore/ScopeExtensions.cs | 2 ++ 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Sentry.AspNetCore/DefaultUserFactory.cs b/src/Sentry.AspNetCore/DefaultUserFactory.cs index 1fef155e9e..df56b5baa4 100644 --- a/src/Sentry.AspNetCore/DefaultUserFactory.cs +++ b/src/Sentry.AspNetCore/DefaultUserFactory.cs @@ -2,7 +2,9 @@ namespace Sentry.AspNetCore; +#pragma warning disable CS0618 internal class DefaultUserFactory : IUserFactory, ISentryUserFactory +#pragma warning restore CS0618 { private readonly IHttpContextAccessor? _httpContextAccessor; diff --git a/src/Sentry.AspNetCore/Extensions/DependencyInjection/ServiceCollectionExtensions.cs b/src/Sentry.AspNetCore/Extensions/DependencyInjection/ServiceCollectionExtensions.cs index 6f32b884ab..98c9415c6b 100644 --- a/src/Sentry.AspNetCore/Extensions/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Sentry.AspNetCore/Extensions/DependencyInjection/ServiceCollectionExtensions.cs @@ -23,7 +23,9 @@ public static ISentryBuilder AddSentry(this IServiceCollection services) services.AddSingleton(); services.AddHttpContextAccessor(); +#pragma warning disable CS0618 services.TryAddSingleton(); +#pragma warning restore CS0618 services.TryAddSingleton(); services diff --git a/src/Sentry.AspNetCore/IUserFactory.cs b/src/Sentry.AspNetCore/IUserFactory.cs index 5501a139b6..32d5a09f9c 100644 --- a/src/Sentry.AspNetCore/IUserFactory.cs +++ b/src/Sentry.AspNetCore/IUserFactory.cs @@ -3,14 +3,9 @@ namespace Sentry.AspNetCore; /// -/// /// Sentry User Factory -/// -/// -/// Note: This interface is tightly coupled to AspNetCore and Will be removed in version 4.0.0. Please consider using -/// with instead. -/// /// +[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 { /// diff --git a/src/Sentry.AspNetCore/ScopeExtensions.cs b/src/Sentry.AspNetCore/ScopeExtensions.cs index 6f53afa8be..44eecf786c 100644 --- a/src/Sentry.AspNetCore/ScopeExtensions.cs +++ b/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(); var user = userFactory?.Create(context); +#pragma warning restore CS0618 if (user != null) { From b0462b59ea8a7a72af55e313d264c092f14b5216 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Mon, 18 Sep 2023 12:55:08 +1200 Subject: [PATCH 08/10] Added unit tests --- ...piApprovalTests.Run.DotNet6_0.verified.txt | 3 ++ ...piApprovalTests.Run.DotNet7_0.verified.txt | 3 ++ .../ApiApprovalTests.Run.Net4_8.verified.txt | 3 ++ .../DefaultUserFactoryTests.cs | 10 ++++ .../ServiceCollectionExtensionsTests.cs | 2 + .../AspNetCoreEnricherTests.cs | 48 +++++++++++++++++++ .../Sentry.OpenTelemetry.Tests.csproj | 6 +-- .../SentrySpanProcessorTests.cs | 33 ++++++++++++- 8 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 test/Sentry.OpenTelemetry.Tests/AspNetCoreEnricherTests.cs diff --git a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt index 66092784f9..b9ab302f97 100644 --- a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt +++ b/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); diff --git a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt b/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt index 66092784f9..b9ab302f97 100644 --- a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt +++ b/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); diff --git a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Net4_8.verified.txt b/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Net4_8.verified.txt index 0e05846ec0..e5a886e55f 100644 --- a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Net4_8.verified.txt +++ b/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); diff --git a/test/Sentry.AspNetCore.Tests/DefaultUserFactoryTests.cs b/test/Sentry.AspNetCore.Tests/DefaultUserFactoryTests.cs index 95269a3f5e..c2cf7b9718 100644 --- a/test/Sentry.AspNetCore.Tests/DefaultUserFactoryTests.cs +++ b/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()); + var contextAccessor = Substitute.For(); + contextAccessor.HttpContext.Returns(HttpContext); + var actual = new DefaultUserFactory(contextAccessor).Create(); + Assert.Equal(IPAddress.IPv6Loopback.ToString(), actual?.IpAddress); + } + [Fact] public void Create_ClaimNameAndIdentityDontMatch_UsernameFromIdentity() { diff --git a/test/Sentry.AspNetCore.Tests/ServiceCollectionExtensionsTests.cs b/test/Sentry.AspNetCore.Tests/ServiceCollectionExtensionsTests.cs index 4aa39e546f..255275ef70 100644 --- a/test/Sentry.AspNetCore.Tests/ServiceCollectionExtensionsTests.cs +++ b/test/Sentry.AspNetCore.Tests/ServiceCollectionExtensionsTests.cs @@ -68,6 +68,7 @@ public void AddSentry_DefaultRequestPayloadExtractor_LastRegistration() Assert.Same(typeof(DefaultRequestPayloadExtractor), last.ImplementationType); } +#pragma warning disable CS0618 [Fact] public void AddSentry_DefaultUserFactory_Registered() { @@ -75,4 +76,5 @@ public void AddSentry_DefaultUserFactory_Registered() _sut.Received().Add(Arg.Is(d => d.ServiceType == typeof(IUserFactory) && d.ImplementationType == typeof(DefaultUserFactory))); } +#pragma warning restore CS0618 } diff --git a/test/Sentry.OpenTelemetry.Tests/AspNetCoreEnricherTests.cs b/test/Sentry.OpenTelemetry.Tests/AspNetCoreEnricherTests.cs new file mode 100644 index 0000000000..fb1a1141d1 --- /dev/null +++ b/test/Sentry.OpenTelemetry.Tests/AspNetCoreEnricherTests.cs @@ -0,0 +1,48 @@ +namespace Sentry.OpenTelemetry.Tests; + +public class AspNetCoreEnricherTests +{ + [Fact] + public void Enrich_SendDefaultPii_UserOnScope() + { + // Arrange + var scope = new Scope(); + var options = new SentryOptions { SendDefaultPii = true }; + var hub = Substitute.For(); + hub.ConfigureScope(Arg.Do>(action => action(scope))); + + var user = new User{ Id = "foo" }; + var userFactory = Substitute.For(); + userFactory.Create().Returns(user); + + var enricher = new AspNetCoreEnricher(userFactory); + + // Act + enricher.Enrich(null!, null!, hub, options); + + // Assert + scope.HasUser().Should().BeTrue(); + scope.User.Should().Be(user); + } + + [Fact] + public void Enrich_SendDefaultPiiFalse_NoUserOnScope() + { + // Arrange + var scope = new Scope(); + var originalUser = scope.User; + var options = new SentryOptions { SendDefaultPii = false }; + var hub = Substitute.For(); + hub.ConfigureScope(Arg.Do>(action => action(scope))); + + var userFactory = Substitute.For(); + var enricher = new AspNetCoreEnricher(userFactory); + + // Act + enricher.Enrich(null!, null!, hub, options); + + // Assert + scope.HasUser().Should().BeFalse(); + scope.User.Should().Be(originalUser); + } +} diff --git a/test/Sentry.OpenTelemetry.Tests/Sentry.OpenTelemetry.Tests.csproj b/test/Sentry.OpenTelemetry.Tests/Sentry.OpenTelemetry.Tests.csproj index 9666d70068..1a6b390365 100644 --- a/test/Sentry.OpenTelemetry.Tests/Sentry.OpenTelemetry.Tests.csproj +++ b/test/Sentry.OpenTelemetry.Tests/Sentry.OpenTelemetry.Tests.csproj @@ -11,14 +11,12 @@ - + - + diff --git a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs index fbf20899e6..17ae164306 100644 --- a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs +++ b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs @@ -18,6 +18,8 @@ private class Fixture public ISystemClock Clock { get; set; } + public List Enrichers { get; set; } = new(); + public Fixture() { Options = new SentryOptions @@ -36,7 +38,7 @@ public Fixture() public SentrySpanProcessor GetSut() { - return new SentrySpanProcessor(GetHub()); + return new SentrySpanProcessor(GetHub(), Enrichers); } } @@ -260,6 +262,35 @@ public void OnEnd_FinishesSpan() } } + [Fact] + public void OnEnd_SpansEnriched() + { + // Arrange + _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; + var mockEnricher = Substitute.For(); + mockEnricher.Enrich(Arg.Do(s => s.SetTag("foo", "bar")), Arg.Any(), Arg.Any(), Arg.Any()); + _fixture.Enrichers.Add(mockEnricher); + var sut = _fixture.GetSut(); + + var parent = Tracer.StartActivity(name: "transaction")!; + sut.OnStart(parent); + + sut._map.TryGetValue(parent.SpanId, out var span); + + // Act + sut.OnEnd(parent); + + // Assert + if (span is not TransactionTracer transactionTracer) + { + Assert.Fail("Span is not a transaction tracer"); + return; + } + + transactionTracer.Tags.TryGetValue("foo", out var foo).Should().BeTrue(); + foo.Should().Be("bar"); + } + [Fact] public void OnEnd_FinishesTransaction() { From e138d507ebcf75e138bf530661b4d2f121527c79 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Mon, 18 Sep 2023 14:04:14 +1200 Subject: [PATCH 09/10] Update ApiApprovalTests.Run.Core3_1.verified.txt --- .../ApiApprovalTests.Run.Core3_1.verified.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Core3_1.verified.txt b/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Core3_1.verified.txt index 66092784f9..b9ab302f97 100644 --- a/test/Sentry.AspNetCore.Tests/ApiApprovalTests.Run.Core3_1.verified.txt +++ b/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); From b833430b982b938fda2738a2e1ea44af85e5d036 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 19 Sep 2023 10:32:05 +1200 Subject: [PATCH 10/10] Update CHANGELOG.md Co-authored-by: Stefan Jandl --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8905d1317e..63c17b28c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ ### Fixes -- Identifying Users with OpenTelemetry doesn't work ([#2618](https://github.com/getsentry/sentry-dotnet/pull/2618)) +- Resolved issue identifying users with OpenTelemetry ([#2618](https://github.com/getsentry/sentry-dotnet/pull/2618)) ### Dependencies