Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
Tyrrrz committed Sep 22, 2020
1 parent 79c1a05 commit 21608a7
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 59 deletions.
2 changes: 1 addition & 1 deletion src/Sentry.AspNetCore/AspNetCoreEventProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal class AspNetCoreEventProcessor : ISentryEventProcessor
private readonly SentryAspNetCoreOptions _options;

public AspNetCoreEventProcessor(IOptions<SentryAspNetCoreOptions> options)
=> _options = options?.Value;
=> _options = options.Value;

public SentryEvent Process(SentryEvent @event)
{
Expand Down
8 changes: 4 additions & 4 deletions src/Sentry.AspNetCore/DefaultUserFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Sentry.AspNetCore
{
internal class DefaultUserFactory : IUserFactory
{
public User Create(HttpContext context)
public User? Create(HttpContext context)
{
Debug.Assert(context != null);

Expand All @@ -17,9 +17,9 @@ public User Create(HttpContext context)
return null;
}

string email = null;
string id = null;
string username = null;
string? email = null;
string? id = null;
string? username = null;
foreach (var claim in principal.Claims)
{
switch (claim.Type)
Expand Down
8 changes: 4 additions & 4 deletions src/Sentry.AspNetCore/HttpRequestAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ internal class HttpRequestAdapter : IHttpRequest

public HttpRequestAdapter(HttpRequest request) => _request = request;

public long? ContentLength => _request?.ContentLength;
public string ContentType => _request?.ContentType;
public Stream Body => _request?.Body;
public long? ContentLength => _request.ContentLength;
public string? ContentType => _request.ContentType;
public Stream? Body => _request.Body;

public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Form =>
_request?.Form.Select(k => new KeyValuePair<string, IEnumerable<string>>(k.Key, k.Value))
?? Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>();
}
}
}
2 changes: 1 addition & 1 deletion src/Sentry.AspNetCore/IUserFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ public interface IUserFactory
/// </summary>
/// <param name="context">The HttpContext where the user resides</param>
/// <returns>The protocol user</returns>
User Create(HttpContext context);
User? Create(HttpContext context);
}
}
40 changes: 23 additions & 17 deletions src/Sentry.AspNetCore/ScopeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,29 @@ public static class ScopeExtensions
/// </remarks>
public static void Populate(this Scope scope, HttpContext context, SentryAspNetCoreOptions options)
{
// Not to throw on code that ignores nullability warnings.
// ReSharper disable ConditionIsAlwaysTrueOrFalse
if (scope is null || context is null || options is null)
{
return;
}
// ReSharper restore ConditionIsAlwaysTrueOrFalse

// With the logger integration, a BeginScope call is made with RequestId. That ends up adding
// two tags with the same value: RequestId and TraceIdentifier
if (!scope.Tags.TryGetValue("RequestId", out var requestId) || requestId != context.TraceIdentifier)
{
scope.SetTag(nameof(context.TraceIdentifier), context.TraceIdentifier);
}

if (options?.SendDefaultPii == true && !scope.HasUser())
if (options.SendDefaultPii && !scope.HasUser())
{
var userFactory = context.RequestServices?.GetService<IUserFactory>();
if (userFactory != null)
var user = userFactory?.Create(context);

if (user != null)
{
scope.User = userFactory.Create(context);
scope.User = user;
}
}

Expand All @@ -48,8 +58,9 @@ public static void Populate(this Scope scope, HttpContext context, SentryAspNetC
}
catch (Exception e)
{
options?.DiagnosticLogger?.LogError("Failed to extract body.", e);
options.DiagnosticLogger?.LogError("Failed to extract body.", e);
}

SetEnv(scope, context, options);

// Extract the route data
Expand Down Expand Up @@ -82,7 +93,7 @@ public static void Populate(this Scope scope, HttpContext context, SentryAspNetC
{
// Suppress the error here; we expect an ArgumentNullException if httpContext.Request.RouteValues is null from GetRouteData()
// TODO: Consider adding a bool to the Sentry options to make route data extraction optional in case they don't use a routing middleware?
options?.DiagnosticLogger?.LogDebug("Failed to extract route data.", e);
options.DiagnosticLogger?.LogDebug("Failed to extract route data.", e);
}

// TODO: Get context stuff into scope
Expand All @@ -108,7 +119,7 @@ private static void SetEnv(Scope scope, HttpContext context, SentryAspNetCoreOpt
scope.Request.QueryString = context.Request.QueryString.ToString();
foreach (var requestHeader in context.Request.Headers)
{
if (options?.SendDefaultPii != true
if (!options.SendDefaultPii
// Don't add headers which might contain PII
&& (requestHeader.Key == HeaderNames.Cookie
|| requestHeader.Key == HeaderNames.Authorization))
Expand All @@ -121,7 +132,7 @@ private static void SetEnv(Scope scope, HttpContext context, SentryAspNetCoreOpt

// TODO: Hide these 'Env' behind some extension method as
// these might be reported in a non CGI, old-school way
if (options?.SendDefaultPii == true
if (options.SendDefaultPii
&& context.Connection.RemoteIpAddress?.ToString() is { } ipAddress)
{
scope.Request.Env["REMOTE_ADDR"] = ipAddress;
Expand All @@ -138,11 +149,6 @@ private static void SetEnv(Scope scope, HttpContext context, SentryAspNetCoreOpt

private static void SetBody(BaseScope scope, HttpContext context, SentryAspNetCoreOptions options)
{
if (context == null || scope == null || options == null)
{
return;
}

var extractors = context.RequestServices.GetService<IEnumerable<IRequestPayloadExtractor>>();
if (extractors == null)
{
Expand All @@ -164,10 +170,13 @@ private static void SetBody(BaseScope scope, HttpContext context, SentryAspNetCo
/// <param name="activity">The activity.</param>
public static void Populate(this Scope scope, Activity activity)
{
if (scope == null || activity == null)
// Not to throw on code that ignores nullability warnings.
// ReSharper disable ConditionIsAlwaysTrueOrFalse
if (scope is null || activity is null)
{
return;
}
// ReSharper restore ConditionIsAlwaysTrueOrFalse

//scope.ActivityId = activity.Id;

Expand All @@ -177,10 +186,7 @@ public static void Populate(this Scope scope, Activity activity)

internal static void SetWebRoot(this Scope scope, string webRoot)
{
if (webRoot != null)
{
scope.Request.Env["DOCUMENT_ROOT"] = webRoot;
}
scope.Request.Env["DOCUMENT_ROOT"] = webRoot;
}
}
}
6 changes: 6 additions & 0 deletions src/Sentry.AspNetCore/Sentry.AspNetCore.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
<AssemblyName>Sentry.AspNetCore</AssemblyName>
<RootNamespace>Sentry.AspNetCore</RootNamespace>
<Description>Official ASP.NET Core integration for Sentry - Open-source error tracking that helps developers monitor and fix crashes in real time.</Description>
<Nullable>enable</Nullable>
</PropertyGroup>

<!-- Disable nullability warnings on older frameworks because there is no nullability info for BCL -->
<PropertyGroup Condition="'$(TargetFramework)' != 'netcoreapp3.0'">
<Nullable>annotations</Nullable>
</PropertyGroup>

<ItemGroup>
Expand Down
8 changes: 4 additions & 4 deletions src/Sentry.AspNetCore/SentryAspNetCoreOptionsSetup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ public override void Configure(SentryAspNetCoreOptions options)
{
base.Configure(options);

options.Environment
= options.Environment // Don't override user defined value
?? EnvironmentLocator.Locate() // Sentry specific environment takes precedence #92
?? _hostingEnvironment?.EnvironmentName;
// Don't override user defined value
options.Environment ??=
EnvironmentLocator.Locate() // Sentry specific environment takes precedence #92
?? _hostingEnvironment.EnvironmentName;

options.AddLogEntryFilter((category, level, eventId, exception)
// https://github.com/aspnet/KestrelHttpServer/blob/0aff4a0440c2f393c0b98e9046a8e66e30a56cb0/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs#L33
Expand Down
46 changes: 25 additions & 21 deletions src/Sentry.AspNetCore/SentryMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public SentryMiddleware(
{
_next = next ?? throw new ArgumentNullException(nameof(next));
_hubAccessor = hubAccessor ?? throw new ArgumentNullException(nameof(hubAccessor));
_options = options?.Value;
_options = options.Value;
if (_options != null)
{
var hub = _hubAccessor();
Expand Down Expand Up @@ -84,20 +84,17 @@ public async Task InvokeAsync(HttpContext context)

using (hub.PushAndLockScope())
{
if (_options != null)
if (_options.MaxRequestBodySize != RequestSize.None)
{
if (_options.MaxRequestBodySize != RequestSize.None)
{
context.Request.EnableBuffering();
}
if (_options.FlushOnCompletedRequest)
context.Request.EnableBuffering();
}
if (_options.FlushOnCompletedRequest)
{
context.Response.OnCompleted(async () =>
{
context.Response.OnCompleted(async () =>
{
// Serverless environments flush the queue at the end of each request
await hub.FlushAsync(timeout: _options.FlushTimeout).ConfigureAwait(false);
});
}
// Serverless environments flush the queue at the end of each request
await hub.FlushAsync(timeout: _options.FlushTimeout).ConfigureAwait(false);
});
}

hub.ConfigureScope(scope =>
Expand Down Expand Up @@ -133,29 +130,36 @@ void CaptureException(Exception e)
{
var evt = new SentryEvent(e);

_logger?.LogTrace("Sending event '{SentryEvent}' to Sentry.", evt);
_logger.LogTrace("Sending event '{SentryEvent}' to Sentry.", evt);

var id = hub.CaptureEvent(evt);

_logger?.LogInformation("Event '{id}' queued.", id);
_logger.LogInformation("Event '{id}' queued.", id);
}
}
}

internal void PopulateScope(HttpContext context, Scope scope)
{
scope.Sdk.Name = Constants.SdkName;
scope.Sdk.Version = NameAndVersion.Version;
scope.Sdk.AddPackage(ProtocolPackageName, NameAndVersion.Version);
if (scope.Sdk is { })
{
scope.Sdk.Name = Constants.SdkName;
scope.Sdk.Version = NameAndVersion.Version;

if (NameAndVersion.Version is { } version)
{
scope.Sdk.AddPackage(ProtocolPackageName, version);
}
}

if (_hostingEnvironment != null)
if (_hostingEnvironment.WebRootPath is { } webRootPath)
{
scope.SetWebRoot(_hostingEnvironment.WebRootPath);
scope.SetWebRoot(webRootPath);
}

scope.Populate(context, _options);

if (_options?.IncludeActivityData == true)
if (_options.IncludeActivityData)
{
scope.Populate(Activity.Current);
}
Expand Down
11 changes: 4 additions & 7 deletions src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static class SentryWebHostBuilderExtensions
/// <param name="builder">The builder.</param>
/// <returns></returns>
public static IWebHostBuilder UseSentry(this IWebHostBuilder builder)
=> UseSentry(builder, (Action<SentryAspNetCoreOptions>)null);
=> UseSentry(builder, (Action<SentryAspNetCoreOptions>?)null);

/// <summary>
/// Uses Sentry integration.
Expand All @@ -40,7 +40,7 @@ public static IWebHostBuilder UseSentry(this IWebHostBuilder builder, string dsn
/// <returns></returns>
public static IWebHostBuilder UseSentry(
this IWebHostBuilder builder,
Action<SentryAspNetCoreOptions> configureOptions)
Action<SentryAspNetCoreOptions>? configureOptions)
=> builder.UseSentry((context, options) => configureOptions?.Invoke(options));

/// <summary>
Expand All @@ -51,7 +51,7 @@ public static IWebHostBuilder UseSentry(
/// <returns></returns>
public static IWebHostBuilder UseSentry(
this IWebHostBuilder builder,
Action<WebHostBuilderContext, SentryAspNetCoreOptions> configureOptions)
Action<WebHostBuilderContext, SentryAspNetCoreOptions>? configureOptions)
{
// The earliest we can hook the SDK initialization code with the framework
// Initialization happens at a later time depending if the default MEL backend is enabled or not.
Expand Down Expand Up @@ -81,10 +81,7 @@ public static IWebHostBuilder UseSentry(
_ = logging.Services.AddSentry();
});

_ = builder.ConfigureServices(c =>
{
_ = c.AddTransient<IStartupFilter, SentryStartupFilter>();
});
_ = builder.ConfigureServices(c => _ = c.AddTransient<IStartupFilter, SentryStartupFilter>());

return builder;
}
Expand Down

0 comments on commit 21608a7

Please sign in to comment.