Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support performance #633

Merged
merged 55 commits into from
Dec 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
f31adac
wip
Tyrrrz Nov 30, 2020
79d2397
wip
Tyrrrz Nov 30, 2020
d7c9974
wip
Tyrrrz Dec 1, 2020
51783e8
wip
Tyrrrz Dec 1, 2020
c99635d
wip
Tyrrrz Dec 1, 2020
7ad5ab5
wip
Tyrrrz Dec 2, 2020
96c443e
wip
Tyrrrz Dec 2, 2020
98e567d
wip
Tyrrrz Dec 2, 2020
e0eb730
wip
Tyrrrz Dec 2, 2020
748670c
wip
Tyrrrz Dec 2, 2020
dfdec31
wip
Tyrrrz Dec 2, 2020
fa64053
wip
Tyrrrz Dec 2, 2020
8338381
wip
Tyrrrz Dec 3, 2020
8fc5c2b
wip
Tyrrrz Dec 4, 2020
6a05871
wip
Tyrrrz Dec 4, 2020
6c0b928
wip
Tyrrrz Dec 4, 2020
5a47bd9
wip
Tyrrrz Dec 4, 2020
e67597e
wip
Tyrrrz Dec 7, 2020
52ec0a5
wip
Tyrrrz Dec 7, 2020
15d7001
wip
Tyrrrz Dec 7, 2020
3a705f0
wip
Tyrrrz Dec 8, 2020
c4f9371
wip
Tyrrrz Dec 8, 2020
00ef2b5
wip
Tyrrrz Dec 8, 2020
b0487cf
wip
Tyrrrz Dec 8, 2020
0a713f3
wip
Tyrrrz Dec 8, 2020
c314a66
wip
Tyrrrz Dec 8, 2020
9e2b7e1
Merge remote-tracking branch 'origin/main' into performance
Tyrrrz Dec 8, 2020
d0c5358
wip
Tyrrrz Dec 8, 2020
a4e01d8
wip
Tyrrrz Dec 8, 2020
d960d7f
wip
Tyrrrz Dec 8, 2020
a0fb93e
wip
Tyrrrz Dec 9, 2020
6c1b413
wip
Tyrrrz Dec 9, 2020
258ed5c
wip
Tyrrrz Dec 9, 2020
f2e5a8b
wip
Tyrrrz Dec 10, 2020
41bcb99
wip
Tyrrrz Dec 10, 2020
da7ee94
wip
Tyrrrz Dec 10, 2020
1371006
wip
Tyrrrz Dec 10, 2020
bafe82a
wip
Tyrrrz Dec 10, 2020
7abdb4f
wip
Tyrrrz Dec 11, 2020
a1e48f8
wip
Tyrrrz Dec 11, 2020
fd13150
wip
Tyrrrz Dec 11, 2020
134d0ec
wip
Tyrrrz Dec 14, 2020
6b92788
wip
Tyrrrz Dec 14, 2020
b6f3ee2
wip
Tyrrrz Dec 14, 2020
0e91066
wip
Tyrrrz Dec 14, 2020
ea06cfb
wip
Tyrrrz Dec 14, 2020
2cb49ef
wip
Tyrrrz Dec 14, 2020
e908cee
wip
Tyrrrz Dec 14, 2020
e8aa3cc
wip
Tyrrrz Dec 14, 2020
71c87ca
wip
Tyrrrz Dec 14, 2020
0c512b1
wip
Tyrrrz Dec 14, 2020
fbd6f17
wip
Tyrrrz Dec 14, 2020
13b88db
wip
Tyrrrz Dec 14, 2020
eb37814
wip
Tyrrrz Dec 14, 2020
61085a4
wip
Tyrrrz Dec 15, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

* Ref moved SentryId from namespace Sentry.Protocol to Sentry (#643) @lucas-zimerman
* Ref renamed `CacheFlushTimeout` to `InitCacheFlushTimeout` (#638) @lucas-zimerman
* Add support for performance. ([#633](https://github.com/getsentry/sentry-dotnet/pull/633))
* Transaction (of type `string`) on Scope and Event now is called TransactionName. ([#633](https://github.com/getsentry/sentry-dotnet/pull/633))

## 3.0.0-alpha.6

Expand All @@ -12,6 +14,7 @@
* Add `SentryScopeStateProcessor` #603
* Add net5.0 TFM to libraries #606
* Add more logging to CachingTransport #619
* Bump Microsoft.Bcl.AsyncInterfaces to 5.0.0 #618
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's on the line below (it got some ` on another PR)

Suggested change
* Bump Microsoft.Bcl.AsyncInterfaces to 5.0.0 #618

* Bump `Microsoft.Bcl.AsyncInterfaces` to 5.0.0 #618
* `DefaultTags` moved from `SentryLoggingOptions` to `SentryOptions` (#637) @PureKrome
* `Sentry.Serilog` can accept DefaultTags (#637) @PureKrome
Expand Down
29 changes: 17 additions & 12 deletions samples/Sentry.Samples.AspNetCore.Basic/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Microsoft.AspNetCore;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.DependencyInjection;

Expand All @@ -26,20 +27,24 @@ public static IWebHost BuildWebHost(string[] args) =>
// .UseSentry("dsn") or .UseSentry(o => o.Dsn = ""; o.Release = "1.0"; ...)

// The App:
.Configure(a =>
.Configure(app =>
{
// An example ASP.NET Core middleware that throws an
// exception when serving a request to path: /throw
a.Use(async (context, next) =>
app.UseRouting();
app.UseEndpoints(endpoints =>
{
var log = context.RequestServices.GetService<ILoggerFactory>()
.CreateLogger<Program>();
// Reported events will be grouped by route pattern
endpoints.MapGet("/throw/{message?}", context =>
{
var exceptionMessage = context.GetRouteValue("message") as string;

log.LogInformation("Handling some request...");
var log = context.RequestServices.GetRequiredService<ILoggerFactory>()
.CreateLogger<Program>();

if (context.Request.Path == "/throw")
{
var hub = context.RequestServices.GetService<IHub>();
log.LogInformation("Handling some request...");

var hub = context.RequestServices.GetRequiredService<IHub>();
hub.ConfigureScope(s =>
{
// More data can be added to the scope like this:
Expand All @@ -53,10 +58,10 @@ public static IWebHost BuildWebHost(string[] args) =>
// The following exception will be captured by the SDK and the event
// will include the Log messages and any custom scope modifications
// as exemplified above.
throw new Exception("An exception thrown from the ASP.NET Core pipeline");
}

await next();
throw new Exception(
exceptionMessage ?? "An exception thrown from the ASP.NET Core pipeline"
);
});
});
})
.Build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"windowsAuthentication": false,
"anonymousAuthentication": true,
"iisExpress": {
"applicationUrl": "http://localhost:59739/",
"applicationUrl": "http://localhost:59739",
"sslPort": 0
}
},
Expand All @@ -27,7 +27,7 @@
"SENTRY_RELEASE": "e386dfd",
"ASPNETCORE_ENVIRONMENT": "Development"
},
"applicationUrl": "http://localhost:59740/"
"applicationUrl": "http://localhost:59740"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>netcoreapp2.1</TargetFramework>
<TargetFramework>net5.0</TargetFramework>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore" Version="2.1.2" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\Sentry.AspNetCore\Sentry.AspNetCore.csproj" />
</ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion samples/Sentry.Samples.Console.Customized/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ await SentrySdk.ConfigureScopeAsync(async scope =>
SentrySdk.WithScope(s =>
{
s.Level = SentryLevel.Fatal;
s.Transaction = "main";
s.TransactionName = "main";
s.Environment = "SpecialEnvironment";

SentrySdk.CaptureMessage("Fatal message!");
Expand Down
43 changes: 43 additions & 0 deletions src/Sentry.AspNetCore/Extensions/HttpContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;

#if !NETSTANDARD2_0
using Microsoft.AspNetCore.Http.Features;
#endif

namespace Sentry.AspNetCore.Extensions
{
internal static class HttpContextExtensions
{
public static string? TryGetRouteTemplate(this HttpContext context)
{
#if !NETSTANDARD2_0
// Requires .UseRouting()/.UseEndpoints()
var endpoint = context.Features.Get<IEndpointFeature?>()?.Endpoint as RouteEndpoint;
var routePattern = endpoint?.RoutePattern.RawText;

if (!string.IsNullOrWhiteSpace(routePattern))
{
return routePattern;
}
#endif

// Requires legacy .UseMvc()
var routeData = context.Features.Get<IRoutingFeature?>()?.RouteData;
var controller = routeData?.Values["controller"]?.ToString();
var action = routeData?.Values["action"]?.ToString();
var area = routeData?.Values["area"]?.ToString();

if (!string.IsNullOrWhiteSpace(action))
{
return !string.IsNullOrWhiteSpace(area)
? $"{area}.{controller}.{action}"
: $"{controller}.{action}";
}

// If the handler doesn't use routing (i.e. it checks `context.Request.Path` directly),
// then there is no way for us to extract anything that resembles a route template.
return null;
}
}
}
38 changes: 19 additions & 19 deletions src/Sentry.AspNetCore/ScopeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Net.Http.Headers;
using Sentry.AspNetCore.Extensions;
using Sentry.Extensibility;
using Sentry.Protocol;

Expand Down Expand Up @@ -68,27 +69,26 @@ public static void Populate(this Scope scope, HttpContext context, SentryAspNetC
try
{
var routeData = context.GetRouteData();
if (routeData != null)
var controller = routeData.Values["controller"]?.ToString();
var action = routeData.Values["action"]?.ToString();
var area = routeData.Values["area"]?.ToString();

if (controller != null)
{
scope.SetTag("route.controller", controller);
}

if (action != null)
{
var controller = routeData.Values["controller"]?.ToString();
var action = routeData.Values["action"]?.ToString();
var area = routeData.Values["area"]?.ToString();

if (controller != null)
{
scope.SetTag("route.controller", controller);
}
if (action != null)
{
scope.SetTag("route.action", action);
}
if (area != null)
{
scope.SetTag("route.area", area);
}

scope.Transaction = area == null ? $"{controller}.{action}" : $"{area}.{controller}.{action}";
scope.SetTag("route.action", action);
}

if (area != null)
{
scope.SetTag("route.area", area);
}

scope.TransactionName = context.TryGetRouteTemplate() ?? context.Request.Path;
bruno-garcia marked this conversation as resolved.
Show resolved Hide resolved
}
catch(Exception e)
{
Expand Down
37 changes: 36 additions & 1 deletion src/Sentry.AspNetCore/SentryMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
using IHostingEnvironment = Microsoft.AspNetCore.Hosting.IWebHostEnvironment;
#endif
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Sentry.AspNetCore.Extensions;
using Sentry.Extensibility;
using Sentry.Protocol;
using Sentry.Reflection;
Expand Down Expand Up @@ -85,6 +87,7 @@ public async Task InvokeAsync(HttpContext context)
{
context.Request.EnableBuffering();
}

if (_options.FlushOnCompletedRequest)
{
context.Response.OnCompleted(async () =>
Expand All @@ -105,12 +108,19 @@ public async Task InvokeAsync(HttpContext context)

scope.OnEvaluating += (_, __) => PopulateScope(context, scope);
});

var transaction = hub.CreateTransaction(
// Try to get the route template or fallback to the request path
context.TryGetRouteTemplate() ?? context.Request.Path,
bruno-garcia marked this conversation as resolved.
Show resolved Hide resolved
"http.server"
);

try
{
await _next(context).ConfigureAwait(false);

// When an exception was handled by other component (i.e: UseExceptionHandler feature).
var exceptionFeature = context.Features.Get<IExceptionHandlerFeature>();
var exceptionFeature = context.Features.Get<IExceptionHandlerFeature?>();
if (exceptionFeature?.Error != null)
{
CaptureException(exceptionFeature.Error);
Expand All @@ -122,6 +132,12 @@ public async Task InvokeAsync(HttpContext context)

ExceptionDispatchInfo.Capture(e).Throw();
}
finally
{
transaction.Finish(
GetSpanStatusFromCode(context.Response.StatusCode)
);
}

void CaptureException(Exception e)
{
Expand Down Expand Up @@ -161,5 +177,24 @@ internal void PopulateScope(HttpContext context, Scope scope)
scope.Populate(Activity.Current);
}
}

private static SpanStatus GetSpanStatusFromCode(int statusCode) => statusCode switch
{
< 400 => SpanStatus.Ok,
400 => SpanStatus.InvalidArgument,
401 => SpanStatus.Unauthenticated,
403 => SpanStatus.PermissionDenied,
404 => SpanStatus.NotFound,
409 => SpanStatus.AlreadyExists,
429 => SpanStatus.ResourceExhausted,
499 => SpanStatus.Cancelled,
< 500 => SpanStatus.InvalidArgument,
500 => SpanStatus.InternalError,
501 => SpanStatus.Unimplemented,
503 => SpanStatus.Unavailable,
504 => SpanStatus.DeadlineExceeded,
< 600 => SpanStatus.InternalError,
_ => SpanStatus.UnknownError
};
}
}
11 changes: 5 additions & 6 deletions src/Sentry.AspNetCore/SentryStartupFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ namespace Sentry.AspNetCore
/// <inheritdoc />
internal class SentryStartupFilter : IStartupFilter
{
public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
=> e =>
{
_ = e.UseSentry();
public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next) => e =>
{
e.UseSentry();

next(e);
};
next(e);
};
}
}
19 changes: 19 additions & 0 deletions src/Sentry/Extensibility/DisabledHub.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Threading.Tasks;
using Sentry.Protocol;

namespace Sentry.Extensibility
{
Expand Down Expand Up @@ -51,6 +52,17 @@ public void WithScope(Action<Scope> scopeCallback)
{
}

/// <summary>
/// Returns a dummy transaction.
/// </summary>
public Transaction CreateTransaction(string name, string operation) =>
new Transaction(this, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have a singleton NoOp instance here instead.
Having a disabled hub means we should reduce the instrumentations overhead


/// <summary>
/// Returns null.
/// </summary>
public SentryTraceHeader? GetSentryTrace() => null;

/// <summary>
/// No-Op.
/// </summary>
Expand All @@ -63,6 +75,13 @@ public void BindClient(ISentryClient client)
/// </summary>
public SentryId CaptureEvent(SentryEvent evt, Scope? scope = null) => SentryId.Empty;

/// <summary>
/// No-Op.
/// </summary>
public void CaptureTransaction(Transaction transaction)
{
}

/// <summary>
/// No-Op.
/// </summary>
Expand Down
22 changes: 22 additions & 0 deletions src/Sentry/Extensibility/HubAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,20 @@ public IDisposable PushScope<TState>(TState state)
public void WithScope(Action<Scope> scopeCallback)
=> SentrySdk.WithScope(scopeCallback);

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
[DebuggerStepThrough]
public Transaction CreateTransaction(string name, string operation)
=> SentrySdk.CreateTransaction(name, operation);

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
[DebuggerStepThrough]
public SentryTraceHeader? GetSentryTrace()
=> SentrySdk.GetTraceHeader();

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
Expand Down Expand Up @@ -132,6 +146,14 @@ public SentryId CaptureException(Exception exception)
public SentryId CaptureEvent(SentryEvent evt, Scope? scope)
=> SentrySdk.CaptureEvent(evt, scope);

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
[DebuggerStepThrough]
[EditorBrowsable(EditorBrowsableState.Never)]
public void CaptureTransaction(Transaction transaction)
=> SentrySdk.CaptureTransaction(transaction);

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>
/// </summary>
Expand Down
Loading