sentry-dotnet
sentry-dotnet copied to clipboard
Option to link Http Request Spans to the Root Transaction?
Package
Sentry.AspNetCore
.NET Flavor
.NET Core
.NET Version
6.0.0
OS
Windows
SDK Version
3.17.1
Self-Hosted Sentry Version
No response
Steps to Reproduce
Sample project: https://github.com/lucas-zimerman/httpclient-stack-sample
-
run and once you get information on the browser an event will be sent to
Sentry SDKs
org on the .NET project. -
but it should be replicable if you call an httpclient in parallel inside of a request.
Expected Result
Looking at the code, you may expect that each span generated on the ForEach loop would have the parentId as GetAnimals
Actual Result
https://sentry.io/organizations/sentry-sdks/discover/sentry-dotnet:dc044830d1994a2685d81dd22eb3cb7c/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All+Events&query=&sort=-timestamp&statsPeriod=1h&yAxis=count%28%29
Due to the defined behavior the HttpClient will try to get the last opened span and due to the parallel behavior, you'll end up with multiple GetDog
spans inside of each other.
One idea is to set an option that affects spans not generated by the user where you can define if the parent will be the last opened Span or always the scope Transaction, (or maybe even an user defined logic?).
EDIT: A brainstorm. Spans could have a parameter that would hide it from GetSpan
, let's name it AllowChildren
. in specific cases like a http request, we could easily assume it's the last span from a stack so we set AllowChildren
as false on HttpClient spans, with that, when you call GetSpan
you'll always get the last open span that could have a children.
The same idea could be applied to the DiagnosticSource Spans or even into parallel code from users.
Given how the results are nested under each other instead of the root, I consider this more of a bug than an enhancement.
I took another look at this recently, and I have a solution. My last statement was incorrect - it is an enhancement that is required, and the SDK is currently working as designed. The good news is, the enhancement is already specified in the Sentry Unified SDK spec:
The
Scope
holds a reference to the currentSpan
orTransaction
.
Scope
IntroducesetSpan
- This can be used internally to pass a
Span
/Transaction
around so that integrations can attach children to it
In the .NET SDK, Scope
currently only has a GetSpan
method. There's no way to set the span. Adding that functionality (and making the get/set span a property), the example code from @lucas-zimerman's sample becomes this:
[HttpGet("GetAnimals")]
public async Task<IEnumerable<int>> GetDogs()
{
var dogs = Enumerable.Range(0, 200).ToList();
// Get the current span before starting the parallel operation.
var baseSpan = SentrySdk.GetSpan();
// Start the parallel operation.
var parallelOptions = new ParallelOptions {MaxDegreeOfParallelism = 6};
await Parallel.ForEachAsync(dogs, parallelOptions, async (dog, ct) =>
{
// Create a new scope for each iteration so they don't step on each other
// and we don't interfere with the parent scope.
await SentrySdk.WithScopeAsync(async scope =>
{
// Set the new scope's span to the base span we collected earlier.
// This ensures that any operation will be rooted to the starting point,
// rather than to just the latest span that happens to be open.
scope.Span = baseSpan;
// Now do the actual work.
await _httpClient.GetAsync($"{baseUri}/GetDog/{dog}", ct);
});
});
return dogs;
}
Note that WithScopeAsync
was added in 3.31.0 with #2303.
So all that's needed is to implement the Scope.Span
property setter, and then it will work.
I'm a bit embarrassed now, as it seems we reached a similar conclusion some time ago with #1845.
Fixed in #2364
I'm a bit embarrassed now, as it seems we reached a similar conclusion some time ago with #1845.
The important thing is that it got fixed :D