Fix: Avoid replacing Transaction Name on ASP.NET Core by null or empty

[lucas-zimerman]

This fix does two issues is ASP.NET Core:

• Avoid calling TryGetTransactionName if you don't have a transaction or if it Transaction.Name is already set.
• Avoid replacing the Transaction Name with null.

The second fix is important since transactions with null or Empty names get dropped.

sentry-dotnet/src/Sentry/SentryClient.cs

Lines 120 to 127 in 74312a4

	if (string.IsNullOrWhiteSpace(transaction.Name) ||
	string.IsNullOrWhiteSpace(transaction.Operation))
	{
	_options.DiagnosticLogger?.LogWarning(
	"Transaction discarded due to one or more required fields missing.");
	return;
	}

Close #1212.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 65.21739% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.13%. Comparing base (f820e11) to head (713af04).
⚠️ Report is 2101 commits behind head on main.

Files with missing lines	Patch %	Lines
src/Sentry.AspNetCore/SentryTracingMiddleware.cs	52.94%	5 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1215      +/-   ##
==========================================
- Coverage   81.57%   80.13%   -1.44%     
==========================================
  Files         212      212              
  Lines        7024     7029       +5     
  Branches     1463     1465       +2     
==========================================
- Hits         5730     5633      -97     
- Misses        846      955     +109     
+ Partials      448      441       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[lucas-zimerman]

This transaction code is Driving Me Nuts 🤪

[lucas-zimerman]

After further refactor, I made changes to account for the Event consumption process.
By doing the previous fix, it lent to another bug in regard to events having the transaction named "Unknown Route" where previously they were not inserted due to another "bug" :crazy:

In addition, I removed the scope.OnEvaluating += (_, _) => PopulateScope(context, scope); from SentryTravingMiddleWare since it's a duplicated code that is also called by SentryMiddleware

sentry-dotnet/src/Sentry.AspNetCore/SentryMiddleware.cs

Lines 98 to 108 in 0cd1584

	hub.ConfigureScope(scope =>
	{
	// At the point lots of stuff from the request are not yet filled
	// Identity for example is added later on in the pipeline
	// Subscribing to the event so that HTTP data is only read in case an event is going to be
	// sent to Sentry. This avoid the cost on error-free requests.
	// In case of event, all data made available through the HTTP Context at the time of the
	// event creation will be sent to Sentry
	scope.OnEvaluating += (_, _) => PopulateScope(context, scope);
	});

I also removed the code to get the transaction name on TryStartTransaction since not always it would return a path, and since we do the same process afterwards with a higher chance to capture the correct name, I opted to remove it.

Despite that, I left the following code as is
[SamplingExtensions.KeyForHttpRoute] = context.TryGetRouteTemplate() It's not the correct code since the route template could be null when starting and only be set afterwards, but I believe that's a task for a future PR.

And,

sentry-dotnet/src/Sentry.AspNetCore/ScopeExtensions.cs

Line 70 in 0cd1584

var routeData = context.GetRouteData();

routeData is nullable almost all the time when running Sentry.Samples.AspNetCore.MVC and it keeps throwing exceptions all the time, I left it as is since it resulted in a weird side effect of it setting the wrong Transaction Name to events .

To conclude, as requested by @bruno-garcia I inverted the TransactionTracer finish logic, by first removing itself from the scope and then Capturing it. I left the code without any try/catch block since both Hub ConfigureScope and CaptureTransactions are already covered with a try/catch block.

Sorry for the long explanation 😅