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

Merging #1215 (713af04) into main (f820e11) will decrease coverage by 1.43%.
The diff coverage is 65.21%.

@@            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     

Impacted Files	Coverage Δ
src/Sentry.AspNetCore/SentryTracingMiddleware.cs	74.39% <52.94%> (-1.93%)	⬇️
src/Sentry.AspNetCore/ScopeExtensions.cs	88.40% <100.00%> (+0.17%)	⬆️
src/Sentry/TransactionTracer.cs	94.25% <100.00%> (-0.13%)	⬇️
src/Sentry/PlatformAbstractions/FrameworkInfo.cs	0.00% <0.00%> (-100.00%)	⬇️
...ntry/PlatformAbstractions/RegistryKeyExtensions.cs	0.00% <0.00%> (-100.00%)	⬇️
...Sentry/PlatformAbstractions/FrameworkInfo.NetFx.cs	0.00% <0.00%> (-70.43%)	⬇️
...rmAbstractions/NetFxInstallationsEventProcessor.cs	4.54% <0.00%> (-68.19%)	⬇️
...ntry/PlatformAbstractions/FrameworkInstallation.cs	25.00% <0.00%> (-37.50%)	⬇️
...ntry/Integrations/NetFxInstallationsIntegration.cs	28.57% <0.00%> (-28.58%)	⬇️
src/Sentry/PlatformAbstractions/RuntimeInfo.cs	53.44% <0.00%> (-5.18%)	⬇️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f820e11...713af04. Read the comment docs.

[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 😅